From 4455bcfc8f199e5b971aa40d2580c33a403c15c2 Mon Sep 17 00:00:00 2001 From: nmadhum Date: Mon, 27 Nov 2017 05:34:08 -0500 Subject: [PATCH 01/13] Allow programmatic copy to clipboard --- notebook/static/notebook/js/clipboard.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/notebook/static/notebook/js/clipboard.js b/notebook/static/notebook/js/clipboard.js index b5248dd23..a15bae0f8 100644 --- a/notebook/static/notebook/js/clipboard.js +++ b/notebook/static/notebook/js/clipboard.js @@ -34,10 +34,18 @@ function load_json(clipboard) { return JSON.parse(s.slice(pix + jcbprefix.length, six)); } +function isProgrammaticCopy(event) { + return (typeof(event.target.selectionStart) !== 'undefined' + && typeof(event.target.selectionEnd) !== 'undefined' + && ((event.target.selectionEnd - event.target.selectionStart) > 0)); +} + function copy(event) { if ((Jupyter.notebook.mode !== 'command') || // window.getSelection checks if text is selected, e.g. in output - !window.getSelection().isCollapsed) { + !window.getSelection().isCollapsed || + // Allow programmatic copy + isProgrammaticCopy(event)) { return; } var selecn = Jupyter.notebook.get_selected_cells().map( From 7b8759fafb5b4e6c98b28e9fd379a76894421c1a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 30 Nov 2017 23:33:50 -0600 Subject: [PATCH 02/13] Add support for terminals on Windows (#3087) * Add support for terminals on Windows * Bump terminado requirement * Fix handling of default shell * Fix appveyor syntax * Fix requires syntax * Fix version target * Clean up handling of default shell and update version check * Always require terminado * Clean up appveyor test * Make the terminado warning uniform * Default to powershell on Windows * Clean up terminado verison --- appveyor.yml | 2 +- notebook/notebookapp.py | 3 +-- notebook/terminal/__init__.py | 12 +++++++++--- setup.py | 2 +- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 68bf1bc94..3b817395a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -23,4 +23,4 @@ install: - cmd: pip install .[test] test_script: - - nosetests --exclude-dir notebook\terminal -v notebook + - nosetests -v notebook diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 4baf4f256..c8e204fb9 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -1328,8 +1328,7 @@ class NotebookApp(JupyterApp): initialize(self.web_app, self.notebook_dir, self.connection_url, self.terminado_settings) self.web_app.settings['terminals_available'] = True except ImportError as e: - log = self.log.debug if sys.platform == 'win32' else self.log.warning - log(_("Terminals not available (error was %s)"), e) + self.log.warning(_("Terminals not available (error was %s)"), e) def init_signal(self): if not sys.platform.startswith('win') and sys.stdin and sys.stdin.isatty(): diff --git a/notebook/terminal/__init__.py b/notebook/terminal/__init__.py index 90df83073..e165ef747 100644 --- a/notebook/terminal/__init__.py +++ b/notebook/terminal/__init__.py @@ -3,9 +3,10 @@ import os import terminado from ..utils import check_version -if not check_version(terminado.__version__, '0.3.3'): - raise ImportError("terminado >= 0.3.3 required, found %s" % terminado.__version__) +if not check_version(terminado.__version__, '0.8.1'): + raise ImportError("terminado >= 0.8.1 required, found %s" % terminado.__version__) +from ipython_genutils.py3compat import which from terminado import NamedTermManager from tornado.log import app_log from notebook.utils import url_path_join as ujoin @@ -13,7 +14,12 @@ from .handlers import TerminalHandler, TermSocket from . import api_handlers def initialize(webapp, notebook_dir, connection_url, settings): - shell = settings.get('shell_command', [os.environ.get('SHELL') or 'sh']) + default_shell = which('sh') + if not default_shell and os.name == 'nt': + default_shell = 'powershell.exe' + shell = settings.get('shell_command', + [os.environ.get('SHELL') or default_shell] + ) terminal_manager = webapp.settings['terminal_manager'] = NamedTermManager( shell_command=shell, extra_env={'JUPYTER_SERVER_ROOT': notebook_dir, diff --git a/setup.py b/setup.py index c90c95314..ace0ac4f4 100755 --- a/setup.py +++ b/setup.py @@ -153,9 +153,9 @@ install_requires = [ 'nbconvert', 'ipykernel', # bless IPython kernel for now 'Send2Trash', + 'terminado>=0.8.1' ] extras_require = { - ':sys_platform != "win32"': ['terminado>=0.3.3'], 'test:python_version == "2.7"': ['mock'], 'test': ['nose', 'coverage', 'requests', 'nose_warnings_filters', 'nbval'], 'test:sys_platform == "win32"': ['nose-exclude'], From cbf7db74497271b7c5cf2e2201b445fb31661fe2 Mon Sep 17 00:00:00 2001 From: Josh Barnes Date: Fri, 1 Dec 2017 05:39:42 +0000 Subject: [PATCH 03/13] Compare non-specific language code for arabic numerals (#3055) Otherwise, country-specific locales like `ar-sa` (Arabic, Saudi Arabia), end up using the default numerals --- notebook/static/bidi/bidi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notebook/static/bidi/bidi.js b/notebook/static/bidi/bidi.js index 9ace652e0..4998dc80d 100644 --- a/notebook/static/bidi/bidi.js +++ b/notebook/static/bidi/bidi.js @@ -19,7 +19,7 @@ define(['bidi/numericshaping'], function(numericshaping) { console.log('Loaded moment locale', moment.locale(_uiLang())); }); - shaperType = _uiLang() == 'ar' ? 'national' : 'defaultNumeral'; + shaperType = _uiLang().split('-')[0] == 'ar' ? 'national' : 'defaultNumeral'; }; var _isMirroringEnabled = function() { From bcc343d9b295ada7006cfbc5a3bbcef78b261d9d Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 04:14:51 +0530 Subject: [PATCH 04/13] Allowing non empty dirs to be deleted --- notebook/services/contents/filemanager.py | 13 ++----------- .../services/contents/tests/test_contents_api.py | 3 ++- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index ef397d4e3..bd586ad1a 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -489,17 +489,8 @@ class FileContentsManager(FileManagerMixin, ContentsManager): path = path.strip('/') os_path = self._get_os_path(path) rm = os.unlink - if os.path.isdir(os_path): - listing = os.listdir(os_path) - # Don't delete non-empty directories. - # A directory containing only leftover checkpoints is - # considered empty. - cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) - for entry in listing: - if entry != cp_dir: - raise web.HTTPError(400, u'Directory %s not empty' % os_path) - elif not os.path.isfile(os_path): - raise web.HTTPError(404, u'File does not exist: %s' % os_path) + if not os.path.exists(os_path): + raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) if self.delete_to_trash: self.log.debug("Sending %s to trash", os_path) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 990b6ca51..9b880f539 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -518,7 +518,8 @@ class APITest(NotebookTestBase): for name in sorted(self.dirs + ['/'], key=len, reverse=True): listing = self.api.list(name).json()['content'] for model in listing: - self.api.delete(model['path']) + if os.path.exists(model['path']): + self.api.delete(model['path']) listing = self.api.list('/').json()['content'] self.assertEqual(listing, []) From f24d3d72651c9be19379e6b4a26b4461bdd42e50 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 04:30:54 +0530 Subject: [PATCH 05/13] Not allowing permanent deletion of non empty dirs --- notebook/services/contents/filemanager.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index bd586ad1a..7b9dcb786 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -493,6 +493,15 @@ class FileContentsManager(FileManagerMixin, ContentsManager): raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) if self.delete_to_trash: + if os.path.isdir(os_path): + listing = os.listdir(os_path) + # Don't send non-empty directories to trash. + # A directory containing only leftover checkpoints is + # considered empty. + cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) + for entry in listing: + if entry != cp_dir: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Sending %s to trash", os_path) # Looking at the code in send2trash, I don't think the errors it # raises let us distinguish permission errors from other errors in From a307af9b3531500f7135e7630bae280867410e14 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 04:40:28 +0530 Subject: [PATCH 06/13] Minor bug fix --- notebook/services/contents/filemanager.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index 7b9dcb786..19cdc8f4f 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -495,13 +495,13 @@ class FileContentsManager(FileManagerMixin, ContentsManager): if self.delete_to_trash: if os.path.isdir(os_path): listing = os.listdir(os_path) - # Don't send non-empty directories to trash. - # A directory containing only leftover checkpoints is - # considered empty. - cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) - for entry in listing: - if entry != cp_dir: - raise web.HTTPError(400, u'Directory %s not empty' % os_path) + # Don't send non-empty directories to trash. + # A directory containing only leftover checkpoints is + # considered empty. + cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) + for entry in listing: + if entry != cp_dir: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Sending %s to trash", os_path) # Looking at the code in send2trash, I don't think the errors it # raises let us distinguish permission errors from other errors in From 03d5d900bc935dbfaa352c092ebdb9463a8118b5 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 16:18:06 +0530 Subject: [PATCH 07/13] Changed order of checks in delete_file --- notebook/services/contents/filemanager.py | 17 ++++++++--------- .../contents/tests/test_contents_api.py | 3 +-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index 19cdc8f4f..7d0c7be5d 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -493,15 +493,6 @@ class FileContentsManager(FileManagerMixin, ContentsManager): raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) if self.delete_to_trash: - if os.path.isdir(os_path): - listing = os.listdir(os_path) - # Don't send non-empty directories to trash. - # A directory containing only leftover checkpoints is - # considered empty. - cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) - for entry in listing: - if entry != cp_dir: - raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Sending %s to trash", os_path) # Looking at the code in send2trash, I don't think the errors it # raises let us distinguish permission errors from other errors in @@ -510,6 +501,14 @@ class FileContentsManager(FileManagerMixin, ContentsManager): return if os.path.isdir(os_path): + listing = os.listdir(os_path) + # Don't permanently delete non-empty directories. + # A directory containing only leftover checkpoints is + # considered empty. + cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) + for entry in listing: + if entry != cp_dir: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Removing directory %s", os_path) with self.perm_to_403(): shutil.rmtree(os_path) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 9b880f539..990b6ca51 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -518,8 +518,7 @@ class APITest(NotebookTestBase): for name in sorted(self.dirs + ['/'], key=len, reverse=True): listing = self.api.list(name).json()['content'] for model in listing: - if os.path.exists(model['path']): - self.api.delete(model['path']) + self.api.delete(model['path']) listing = self.api.list('/').json()['content'] self.assertEqual(listing, []) From 7f23ad65a62f704b5573b4b81e83d04c696303f8 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 16:30:10 +0530 Subject: [PATCH 08/13] modified test for deleting non empty directory --- notebook/services/contents/tests/test_contents_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 990b6ca51..12049fc4b 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -525,7 +525,8 @@ class APITest(NotebookTestBase): def test_delete_non_empty_dir(self): """delete non-empty dir raises 400""" with assert_http_error(400): - self.api.delete(u'å b') + shutil.rmtree((u'å b', ignore_errors=True) + # self.api.delete(u'å b') def test_rename(self): resp = self.api.rename('foo/a.ipynb', 'foo/z.ipynb') From 773e55cb1329f38b29235dbf514e1aa5256568b6 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 16:30:13 +0530 Subject: [PATCH 09/13] modified test for deleting non empty directory --- notebook/services/contents/tests/test_contents_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 12049fc4b..a37b99d13 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -523,7 +523,7 @@ class APITest(NotebookTestBase): self.assertEqual(listing, []) def test_delete_non_empty_dir(self): - """delete non-empty dir raises 400""" + """permanentaly deleting non-empty dir raises 400""" with assert_http_error(400): shutil.rmtree((u'å b', ignore_errors=True) # self.api.delete(u'å b') From 081dcd577461112ab06e8e82a430ed8f66f95fca Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 16:50:40 +0530 Subject: [PATCH 10/13] Added test for permanent delete --- notebook/services/contents/filemanager.py | 17 ++++++++--------- .../contents/tests/test_contents_api.py | 10 +++++----- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/notebook/services/contents/filemanager.py b/notebook/services/contents/filemanager.py index 19cdc8f4f..7d0c7be5d 100644 --- a/notebook/services/contents/filemanager.py +++ b/notebook/services/contents/filemanager.py @@ -493,15 +493,6 @@ class FileContentsManager(FileManagerMixin, ContentsManager): raise web.HTTPError(404, u'File or directory does not exist: %s' % os_path) if self.delete_to_trash: - if os.path.isdir(os_path): - listing = os.listdir(os_path) - # Don't send non-empty directories to trash. - # A directory containing only leftover checkpoints is - # considered empty. - cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) - for entry in listing: - if entry != cp_dir: - raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Sending %s to trash", os_path) # Looking at the code in send2trash, I don't think the errors it # raises let us distinguish permission errors from other errors in @@ -510,6 +501,14 @@ class FileContentsManager(FileManagerMixin, ContentsManager): return if os.path.isdir(os_path): + listing = os.listdir(os_path) + # Don't permanently delete non-empty directories. + # A directory containing only leftover checkpoints is + # considered empty. + cp_dir = getattr(self.checkpoints, 'checkpoint_dir', None) + for entry in listing: + if entry != cp_dir: + raise web.HTTPError(400, u'Directory %s not empty' % os_path) self.log.debug("Removing directory %s", os_path) with self.perm_to_403(): shutil.rmtree(os_path) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 9b880f539..51284d99e 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -185,6 +185,8 @@ class APITest(NotebookTestBase): """Delete a directory at api_path, removing any contents.""" os_path = self.to_os_path(api_path) shutil.rmtree(os_path, ignore_errors=True) + with assert_http_error(400): + shutil.rmtree(u'å b') def delete_file(self, api_path): """Delete a file at the given path if it exists.""" @@ -518,15 +520,13 @@ class APITest(NotebookTestBase): for name in sorted(self.dirs + ['/'], key=len, reverse=True): listing = self.api.list(name).json()['content'] for model in listing: - if os.path.exists(model['path']): - self.api.delete(model['path']) + self.api.delete(model['path']) listing = self.api.list('/').json()['content'] self.assertEqual(listing, []) def test_delete_non_empty_dir(self): - """delete non-empty dir raises 400""" - with assert_http_error(400): - self.api.delete(u'å b') + """deleting non-empty dir is allowed""" + self.api.delete(u'å b') def test_rename(self): resp = self.api.rename('foo/a.ipynb', 'foo/z.ipynb') From 31a6625e23c2d2ec4fc25d5f95e3ceeb58ef19c1 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 16:58:44 +0530 Subject: [PATCH 11/13] Fixed test --- notebook/services/contents/tests/test_contents_api.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 0a330cacc..60eadcae7 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -185,8 +185,6 @@ class APITest(NotebookTestBase): """Delete a directory at api_path, removing any contents.""" os_path = self.to_os_path(api_path) shutil.rmtree(os_path, ignore_errors=True) - with assert_http_error(400): - shutil.rmtree(u'å b') def delete_file(self, api_path): """Delete a file at the given path if it exists.""" From d136c12c3be5da1375d997ca70f6a52cbc3ec06d Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 18:57:17 +0530 Subject: [PATCH 12/13] Added test for deleting non empty dirs --- notebook/services/contents/tests/test_contents_api.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 60eadcae7..46402323c 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -522,6 +522,12 @@ class APITest(NotebookTestBase): listing = self.api.list('/').json()['content'] self.assertEqual(listing, []) + def test_delete_non_empty_dir(self): + # Test that non empty directory can be deleted + self.api.delete(u'å b') + # Assertion will pass only if self.api.delete does not throw and error + assert True + def test_rename(self): resp = self.api.rename('foo/a.ipynb', 'foo/z.ipynb') self.assertEqual(resp.headers['Location'].split('/')[-1], 'z.ipynb') From 98b8e2ecdac315fdcdb09a8896996bd34e333f66 Mon Sep 17 00:00:00 2001 From: Kirit Thadaka Date: Tue, 5 Dec 2017 19:07:29 +0530 Subject: [PATCH 13/13] Made test for thorough --- notebook/services/contents/tests/test_contents_api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/notebook/services/contents/tests/test_contents_api.py b/notebook/services/contents/tests/test_contents_api.py index 46402323c..cfea8cb5f 100644 --- a/notebook/services/contents/tests/test_contents_api.py +++ b/notebook/services/contents/tests/test_contents_api.py @@ -525,8 +525,9 @@ class APITest(NotebookTestBase): def test_delete_non_empty_dir(self): # Test that non empty directory can be deleted self.api.delete(u'å b') - # Assertion will pass only if self.api.delete does not throw and error - assert True + # Check if directory has actually been deleted + with assert_http_error(404): + self.api.list(u'å b') def test_rename(self): resp = self.api.rename('foo/a.ipynb', 'foo/z.ipynb')