From 901efa0d5c25c29b98f18281d7e169b5f0bff056 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 06:16:34 -0500 Subject: [PATCH 1/6] Add smart copy/paste and update docs --- docs/source/user/terminal.rst | 25 ++++++++-------- .../terminal-extension/schema/plugin.json | 9 ++++++ packages/terminal/src/tokens.ts | 10 +++++-- packages/terminal/src/widget.ts | 30 +++++++++++++++++-- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/docs/source/user/terminal.rst b/docs/source/user/terminal.rst index 98ce39893ec8..80a16907dab8 100644 --- a/docs/source/user/terminal.rst +++ b/docs/source/user/terminal.rst @@ -47,20 +47,19 @@ re-open it using the Running tab in the left sidebar: Copy/Paste ~~~~~~~~~~~~ -For both Windows and Mac users, press ``shift`` + ``right-click`` to get to the right-click menu from inside Jupyterlab terminal. Or by using shortcut keys as follows: +For all platforms, JupyterLab will interpret ``Ctrl+C`` as a copy if there is text selected. +In addition, ``Ctrl+V`` will be interpreted as a paste command unless the ``pasteWithCtrlV`` +setting is disabled. One may want to disable ``pasteWithCtrlV`` if the shortcut is needed +for something else such as the vi editor. -* **For Mac users:** - - **Copy** : ``Cmd`` + ``c`` - - **Paste**: ``Cmd`` + ``v`` - - -* **For Windows users (Power shell):** - - **Copy** : ``Ctrl`` + ``Insert`` - - **Paste** : ``Shift`` + ``Insert`` +To use the native browser Copy/Paste menu, hold ``Shift`` and right click to bring up the +context menu. +For MacOS users, ``Cmd+C`` and ``Cmd+V`` work as usual. +For Windows users using ``PowerShell``, ``Ctrl+Insert`` and ``Shift+Insert`` work as usual. +For anyone using a *nix shell, the default ``Ctrl+Shift+C`` conflicts with the default +shortcut for toggling the command palette (``apputils:activate-command-palette``). +If desired, that shortcut can be changed by editing the keyboard shortcuts in setttings. +Using ``Ctrl+Shift+V`` for paste works as usual. diff --git a/packages/terminal-extension/schema/plugin.json b/packages/terminal-extension/schema/plugin.json index 730c9db7be2e..c47e5463b0a6 100644 --- a/packages/terminal-extension/schema/plugin.json +++ b/packages/terminal-extension/schema/plugin.json @@ -21,6 +21,9 @@ }, "scrollback": { "type": "number" + }, + "pasteWithCtrlV": { + "type": "boolean" } }, "properties": { @@ -59,6 +62,12 @@ "description": "Whether to shut down or not the session when closing the terminal.", "type": "boolean", "default": false + }, + "pasteWithCtrlV": { + "title": "Paste with Ctrl+V", + "description": "Whether to enable pasting with Ctrl+V. This can be disabled to use Ctrl+V in the vi editor, for instance", + "type": "boolean", + "default": true } }, "additionalProperties": false, diff --git a/packages/terminal/src/tokens.ts b/packages/terminal/src/tokens.ts index cb0017ce1567..3d1f594fa0cf 100644 --- a/packages/terminal/src/tokens.ts +++ b/packages/terminal/src/tokens.ts @@ -96,11 +96,16 @@ export namespace ITerminal { initialCommand: string; /** - * Wether to enable screen reader support. + * Whether to enable screen reader support. * * Set to false if you run into performance problems from DOM overhead */ screenReaderMode: boolean; + + /** + * Whether to enable using Ctrl+V to paste. + */ + pasteWithCtrlV: boolean; } /** @@ -115,7 +120,8 @@ export namespace ITerminal { shutdownOnClose: false, cursorBlink: true, initialCommand: '', - screenReaderMode: true + screenReaderMode: true, + pasteWithCtrlV: true }; /** diff --git a/packages/terminal/src/widget.ts b/packages/terminal/src/widget.ts index 4970c2981ef9..e9a68c890db6 100644 --- a/packages/terminal/src/widget.ts +++ b/packages/terminal/src/widget.ts @@ -237,7 +237,8 @@ export class Terminal extends Widget implements ITerminal.ITerminal { * Initialize the terminal object. */ private _initializeTerm(): void { - this._term.on('data', (data: string) => { + const term = this._term; + term.on('data', (data: string) => { if (this.isDisposed) { return; } @@ -247,9 +248,32 @@ export class Terminal extends Widget implements ITerminal.ITerminal { }); }); - this._term.on('title', (title: string) => { + term.on('title', (title: string) => { this.title.label = title; }); + + term.attachCustomKeyEventHandler(event => { + if ( + (event.ctrlKey || event.metaKey) && + event.key === 'c' && + term.hasSelection() + ) { + // Return so that the usual OS copy happens + // instead of interrupt signal. + return false; + } + + if ( + (event.ctrlKey || event.metaKey) && + event.key === 'v' && + this._options.pasteWithCtrlV + ) { + // Return so that the usual paste happens. + return false; + } + + return true; + }); } /** @@ -303,7 +327,7 @@ export class Terminal extends Widget implements ITerminal.ITerminal { } } - private _term: Xterm; + private readonly _term: Xterm; private _needsResize = true; private _termOpened = false; private _offsetWidth = -1; From 1bfae50d2504d9242b40d1ae30f60821fd51d5ff Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 07:45:30 -0500 Subject: [PATCH 2/6] Use Cmd+C/Cmd+V always on macOS --- docs/source/user/terminal.rst | 16 ++++++++-------- packages/terminal-extension/schema/plugin.json | 2 +- packages/terminal/src/tokens.ts | 2 ++ packages/terminal/src/widget.ts | 8 ++++++++ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/source/user/terminal.rst b/docs/source/user/terminal.rst index 80a16907dab8..01bf80447f3a 100644 --- a/docs/source/user/terminal.rst +++ b/docs/source/user/terminal.rst @@ -47,17 +47,17 @@ re-open it using the Running tab in the left sidebar: Copy/Paste ~~~~~~~~~~~~ -For all platforms, JupyterLab will interpret ``Ctrl+C`` as a copy if there is text selected. -In addition, ``Ctrl+V`` will be interpreted as a paste command unless the ``pasteWithCtrlV`` -setting is disabled. One may want to disable ``pasteWithCtrlV`` if the shortcut is needed -for something else such as the vi editor. +For macOS users, ``Cmd+C`` and ``Cmd+V`` work as usual. -To use the native browser Copy/Paste menu, hold ``Shift`` and right click to bring up the -context menu. +For Windows users using ``PowerShell``, ``Ctrl+Insert`` and ``Shift+Insert`` work as usual. -For MacOS users, ``Cmd+C`` and ``Cmd+V`` work as usual. +To use the native browser Copy/Paste menu, hold ``Shift`` and right click to bring up the +context menu (note: this may not work in all browsers). -For Windows users using ``PowerShell``, ``Ctrl+Insert`` and ``Shift+Insert`` work as usual. +For non-macOS users, JupyterLab will interpret ``Ctrl+C`` as a copy if there is text selected. +In addition, ``Ctrl+V`` will be interpreted as a paste command unless the ``pasteWithCtrlV`` +setting is disabled. One may want to disable ``pasteWithCtrlV`` if the shortcut is needed +for something else such as the vi editor. For anyone using a *nix shell, the default ``Ctrl+Shift+C`` conflicts with the default shortcut for toggling the command palette (``apputils:activate-command-palette``). diff --git a/packages/terminal-extension/schema/plugin.json b/packages/terminal-extension/schema/plugin.json index c47e5463b0a6..bac2ba28b1fa 100644 --- a/packages/terminal-extension/schema/plugin.json +++ b/packages/terminal-extension/schema/plugin.json @@ -65,7 +65,7 @@ }, "pasteWithCtrlV": { "title": "Paste with Ctrl+V", - "description": "Whether to enable pasting with Ctrl+V. This can be disabled to use Ctrl+V in the vi editor, for instance", + "description": "Whether to enable pasting with Ctrl+V. This can be disabled to use Ctrl+V in the vi editor, for instance. This setting has no effect on macOS, where Cmd+V is available", "type": "boolean", "default": true } diff --git a/packages/terminal/src/tokens.ts b/packages/terminal/src/tokens.ts index 3d1f594fa0cf..2349d344cd68 100644 --- a/packages/terminal/src/tokens.ts +++ b/packages/terminal/src/tokens.ts @@ -104,6 +104,8 @@ export namespace ITerminal { /** * Whether to enable using Ctrl+V to paste. + * + * This setting has no effect on macOS, where Cmd+V is available. */ pasteWithCtrlV: boolean; } diff --git a/packages/terminal/src/widget.ts b/packages/terminal/src/widget.ts index e9a68c890db6..ed0b98e5ef11 100644 --- a/packages/terminal/src/widget.ts +++ b/packages/terminal/src/widget.ts @@ -3,6 +3,8 @@ import { TerminalSession } from '@jupyterlab/services'; +import { IS_MAC } from '@phosphor/domutils'; + import { Message, MessageLoop } from '@phosphor/messaging'; import { Widget } from '@phosphor/widgets'; @@ -252,6 +254,12 @@ export class Terminal extends Widget implements ITerminal.ITerminal { this.title.label = title; }); + // Do not add any Ctrl+C/Ctrl+V handling on macOS, + // where Cmd+C/Cmd+V works as intended. + if (IS_MAC) { + return; + } + term.attachCustomKeyEventHandler(event => { if ( (event.ctrlKey || event.metaKey) && From 456549f7504128c14c6bfc218d7d1643315f881f Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 07:51:45 -0500 Subject: [PATCH 3/6] integrity --- packages/terminal/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/terminal/package.json b/packages/terminal/package.json index f5ce712cb6b5..6d5f8cc0bbd6 100644 --- a/packages/terminal/package.json +++ b/packages/terminal/package.json @@ -34,6 +34,7 @@ "@jupyterlab/apputils": "^1.0.0-alpha.6", "@jupyterlab/services": "^4.0.0-alpha.6", "@phosphor/coreutils": "^1.3.0", + "@phosphor/domutils": "^1.1.2", "@phosphor/messaging": "^1.2.2", "@phosphor/widgets": "^1.6.0", "xterm": "~3.10.1" From 3132d850931cbec29cdb2a78fddde6bd801dc6b5 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 07:53:03 -0500 Subject: [PATCH 4/6] update docs --- docs/source/user/terminal.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/user/terminal.rst b/docs/source/user/terminal.rst index 01bf80447f3a..47335e9ebba8 100644 --- a/docs/source/user/terminal.rst +++ b/docs/source/user/terminal.rst @@ -59,7 +59,7 @@ In addition, ``Ctrl+V`` will be interpreted as a paste command unless the ``past setting is disabled. One may want to disable ``pasteWithCtrlV`` if the shortcut is needed for something else such as the vi editor. -For anyone using a *nix shell, the default ``Ctrl+Shift+C`` conflicts with the default +For anyone using a \*nix shell, the default ``Ctrl+Shift+C`` conflicts with the default shortcut for toggling the command palette (``apputils:activate-command-palette``). If desired, that shortcut can be changed by editing the keyboard shortcuts in setttings. Using ``Ctrl+Shift+V`` for paste works as usual. From c6b1ac23768544cd9e574ac870063d81b0b73d6a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 09:16:40 -0500 Subject: [PATCH 5/6] Fix import --- packages/terminal/src/widget.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/terminal/src/widget.ts b/packages/terminal/src/widget.ts index ed0b98e5ef11..3cd91aced014 100644 --- a/packages/terminal/src/widget.ts +++ b/packages/terminal/src/widget.ts @@ -3,7 +3,7 @@ import { TerminalSession } from '@jupyterlab/services'; -import { IS_MAC } from '@phosphor/domutils'; +import { Platform } from '@phosphor/domutils'; import { Message, MessageLoop } from '@phosphor/messaging'; @@ -256,7 +256,7 @@ export class Terminal extends Widget implements ITerminal.ITerminal { // Do not add any Ctrl+C/Ctrl+V handling on macOS, // where Cmd+C/Cmd+V works as intended. - if (IS_MAC) { + if (Platform.IS_MAC) { return; } From 08a9808225b7b7106c7ce772eae9de540a72f133 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 10:17:44 -0500 Subject: [PATCH 6/6] simplify logic --- packages/terminal/src/widget.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/terminal/src/widget.ts b/packages/terminal/src/widget.ts index 3cd91aced014..f6a897385d3f 100644 --- a/packages/terminal/src/widget.ts +++ b/packages/terminal/src/widget.ts @@ -261,21 +261,13 @@ export class Terminal extends Widget implements ITerminal.ITerminal { } term.attachCustomKeyEventHandler(event => { - if ( - (event.ctrlKey || event.metaKey) && - event.key === 'c' && - term.hasSelection() - ) { + if (event.ctrlKey && event.key === 'c' && term.hasSelection()) { // Return so that the usual OS copy happens // instead of interrupt signal. return false; } - if ( - (event.ctrlKey || event.metaKey) && - event.key === 'v' && - this._options.pasteWithCtrlV - ) { + if (event.ctrlKey && event.key === 'v' && this._options.pasteWithCtrlV) { // Return so that the usual paste happens. return false; }