Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add smart copy/paste in terminal and update docs #6391

Merged
merged 6 commits into from May 23, 2019
Merged

Add smart copy/paste in terminal and update docs #6391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
901efa0
Add smart copy/paste and update docs
blink1073 May 23, 2019
1bfae50
Use Cmd+C/Cmd+V always on macOS
blink1073 May 23, 2019
456549f
integrity
blink1073 May 23, 2019
3132d85
update docs
blink1073 May 23, 2019
c6b1ac2
Fix import
blink1073 May 23, 2019
08a9808
simplify logic
blink1073 May 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 12 additions & 13 deletions docs/source/user/terminal.rst
View file
Open in desktop
Expand Up @@ -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 macOS users, ``Cmd+C`` and ``Cmd+V`` work as usual.

* **For Mac users:**

**Copy** : ``Cmd`` + ``c``

**Paste**: ``Cmd`` + ``v``


* **For Windows users (Power shell):**

**Copy** : ``Ctrl`` + ``Insert``

**Paste** : ``Shift`` + ``Insert``
For Windows users using ``PowerShell``, ``Ctrl+Insert`` and ``Shift+Insert`` 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 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``).
If desired, that shortcut can be changed by editing the keyboard shortcuts in setttings.
Using ``Ctrl+Shift+V`` for paste works as usual.
9 changes: 9 additions & 0 deletions packages/terminal-extension/schema/plugin.json
View file
Open in desktop
Expand Up @@ -21,6 +21,9 @@
},
"scrollback": {
"type": "number"
},
"pasteWithCtrlV": {
"type": "boolean"
}
},
"properties": {
Expand Down Expand Up @@ -59,6 +62,12 @@
"description": "Whether to shut down or not the session when closing the terminal.",
"type": "boolean",
"default": false
},
"pasteWithCtrlV": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pasteWithCtrlVNotMac? Ctrl+V won't paste on a mac, so I think the option should say something about only applying on non-Mac platforms, and the actual code also check to see if it is a mac platform.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in Ctrl+V doesn't paste on a mac with this PR? If it does, I don't see a reason to make a distinction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, what we could do is set the default to false if on a mac and add a separate setting for copyWithCtrlC that is also false on a mac, I think that would be coherent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in Ctrl+V doesn't paste on a mac with this PR? If it does, I don't see a reason to make a distinction.

Standard browser behavior is that Ctrl+V doesn't paste on the mac.

Actually, what we could do is set the default to false if on a mac and add a separate setting for copyWithCtrlC that is also false on a mac, I think that would be coherent.

Can we change defaults according to platform? I thought the json schema didn't allow changing defaults based on platform.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, shoot, I was thinking of the terminal options themselves, not the schema. I think the setting should be ignored on a Mac and we just don't add the handler at all there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With documentation to that effect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could be more explicit: browserDefaultForCtrlVNotMac or something. Instead of assuming that the browser will be pasting, just mentioning exactly what this option does, which is ignore ctrl+v in xterm.js.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that doesn't tell you anything about what Ctrl+C does. That name is not read-able at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me update the PR and see what you think.

"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. This setting has no effect on macOS, where Cmd+V is available",
"type": "boolean",
"default": true
}
},
"additionalProperties": false,
Expand Down
1 change: 1 addition & 0 deletions packages/terminal/package.json
View file
Open in desktop
Expand Up @@ -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"
Expand Down
12 changes: 10 additions & 2 deletions packages/terminal/src/tokens.ts
View file
Open in desktop
Expand Up @@ -96,11 +96,18 @@ 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.
*
* This setting has no effect on macOS, where Cmd+V is available.
*/
pasteWithCtrlV: boolean;
}

/**
Expand All @@ -115,7 +122,8 @@ export namespace ITerminal {
shutdownOnClose: false,
cursorBlink: true,
initialCommand: '',
screenReaderMode: true
screenReaderMode: true,
pasteWithCtrlV: true
};

/**
Expand Down
30 changes: 27 additions & 3 deletions packages/terminal/src/widget.ts
View file
Open in desktop
Expand Up @@ -3,6 +3,8 @@

import { TerminalSession } from '@jupyterlab/services';

import { Platform } from '@phosphor/domutils';

import { Message, MessageLoop } from '@phosphor/messaging';

import { Widget } from '@phosphor/widgets';
Expand Down Expand Up @@ -237,7 +239,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;
}
Expand All @@ -247,9 +250,30 @@ export class Terminal extends Widget implements ITerminal.ITerminal {
});
});

this._term.on('title', (title: string) => {
term.on('title', (title: string) => {
this.title.label = title;
});

// Do not add any Ctrl+C/Ctrl+V handling on macOS,
// where Cmd+C/Cmd+V works as intended.
if (Platform.IS_MAC) {
return;
}

term.attachCustomKeyEventHandler(event => {
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.key === 'v' && this._options.pasteWithCtrlV) {
// Return so that the usual paste happens.
return false;
}

return true;
});
}

/**
Expand Down Expand Up @@ -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;
Expand Down