Improve copy/paste behavior in Connect terminal #44871

gzdunek · 2024-07-31T12:41:42Z

Closes #37650 - on Windows and Linux now we use Ctrl+Shift+C/V
Closes #22828 - text can be copied/pasted with a right click (config item terminal.rightClick, enabled by default on Windows)
Closes #24451 - text can be copied by selection (config item terminal.copyOnSelect, disabled by default)

The config options were inspired by Tabby and VS Code.

TODO: Add the new config options to docs.

changelog: Improved copy and paste behavior in the terminal in Teleport Connect. On Windows and Linux, Ctrl+Shift+C/V now copies and pastes text (these shortcuts can be changed with keymap.terminalCopy/keymap.terminalPaste). A mouse right click (terminal.rightClick) can copy/paste text too (enabled by default on Windows).
Optionally, you can also enable auto-copy on select (terminal.copyOnSelect).

gzdunek · 2024-07-31T12:44:43Z

web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts

+      //  the same time on Linux causes flickering.
+      e.preventDefault();
+
+      if (this.configService.get('terminal.rightClick').value === 'menu') {


Initially, I wanted to pass terminalRightClick as another property in options, but maybe it makes more sense to pass the entire service? It is easy to mock in tests anyway.

Another thing is that this allows us to dynamically read the config, so when one day we make the config panel, new values will be applied without an app restart or opening a new terminal.

…set reasonable defaults for Windows and Linux

web/packages/teleterm/src/services/config/appConfigSchema.ts

web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts

ravicious · 2024-08-01T15:25:57Z

web/packages/teleterm/src/ui/DocumentTerminal/Terminal/ctrl.ts

@@ -79,12 +84,85 @@ export default class TtyTerminal {
      },
    });

+    this.term.onSelectionChange(() => {
+      if (
+        this.configService.get('terminal.copyOnSelect').value &&


copyOnSelect behaves a bit weird in combination with rightClick: 'copyPaste'.

When I select something and then press right click, the whole "word" gets selected and copied.

Comparison videos

{ rightClick: 'copyPaste', copyOnSelect: true }

rightClick-copyPaste-copyOnSelect-true.mov

{ rightClick: 'copyPaste', copyOnSelect: false }

rightClick-copyPaste-copyOnSelect-false.mov

I think what's happening is that even with copyOnSelect: false, the right click seems to highlight the whole word but copies only the previously selected fragment. With copyOnSelect: true, the right click copies the selected text but it also changes the selection to the full word, which in turn copies the whole word.

idk if there's something we need to do about it. Maybe the config options should look a little bit different? Setting { rightClick: 'copyPaste', copyOnSelect: true } at the same time is "legal" but idk if it makes much sense from the UX standpoint. Perhaps a smaller number of available choices would work well enough. I don't really want to dictate a solution here as I don't intuitively understand all requirements here.

repro.mov

Good finding.

To solve this problem, we could use the xterm.js's option rightClickSelectsWord.
For example, vscode sets this option to false when their terminal.rightClick is selectWord. When this option is active, right click selects a word and then opens the context menu. I don't think we need option too, instead we could set rightClickSelectsWord to false when terminal.rightClick is menu. What do you think?

I don't think we need option too, instead we could set rightClickSelectsWord to false when terminal.rightClick is menu. What do you think?

App Shows context menu Highlights word Does copy/paste

cmd.exe ❌ ❌ ✅

Powershell ❌ ❌ ✅

Windows Terminal ❌ ❌ ✅

Terminal.app ✅ ✅ ❌

iTerm ✅ ✅ ❌

Terminal on Ubuntu ✅ ❌ ❌

Terminal on Fedora ✅ ❌ ❌

Assuming we don't need to stick to differences between macOS and Linux, what do you say we do rightClickSelectsWord: false if rightClick is paste or copyPaste and rightClickSelectsWord: true if rightClick is menu?

We could consider rightClickSelectsWord: false when rightClick: 'menu', copyOnSelect: true, but I don't know what people would actually want here. We could leave it as is, I think copyOnSelect is something you care about mostly when rightClick is set to pase or copyPaste.

Assuming we don't need to stick to differences between macOS and Linux, what do you say we do rightClickSelectsWord: false if rightClick is paste or copyPaste and rightClickSelectsWord: true if rightClick is menu?

I agree, this makes sense. Actually, I wanted to propose the same in the comment above, but I totally mixed it up 🤦
I wanted to say:

vscode sets this option to true when their terminal.rightClick is selectWord

we could set rightClickSelectsWord to true when terminal.rightClick is menu

And thanks for the table. It's really helpful, I appreciate the effort.
I applied the change, and made the config to be read only when terminal starts. Otherwise, some parts of ctrl.ts would read the config dynamically (in event listeners), and some (rightClickSelectsWord) only in the constructor.

We could consider rightClickSelectsWord: false when rightClick: 'menu', copyOnSelect: true, but I don't know what people would actually want here. We could leave it as is, I think copyOnSelect is something you care about mostly when rightClick is set to pase or copyPaste.

Agree 👍

ravicious · 2024-08-01T15:52:47Z

web/packages/teleterm/src/services/config/appConfigSchema.ts

@@ -182,6 +204,8 @@ const getDefaultKeymap = (
        openConnections: 'Ctrl+Shift+P',
        openClusters: 'Ctrl+Shift+E',
        openProfiles: 'Ctrl+Shift+I',
+        terminalCopy: 'Ctrl+Shift+C',
+        terminalPaste: 'Ctrl+Shift+V',


How are people on Linux and Windows going to know that these are the shortcuts for copy and paste inside a terminal? We could include them in the context menu on right click, but I think that would only work on Linux, right? idk if there's a solution on Windows, but I guess people are used to using a mouse for that anyway.

We could include them in the context menu on right click, but I think that would only work on Linux, right? idk if there's a solution on Windows, but I guess people are used to using a mouse for that anyway.

Good news, we can add an accelerator (keyboard shortcut) to menu items. It will be shown on all platforms.
The only problem I see, is that by default we set terminal.rightClick on Windows to copyPaste. Perhaps, we should revert it to the context menu? So users can learn these new shortcuts.

The accelerator added to menu items is fantastic, wow.

Since the equivalent of rightClick: 'copyPaste' is the default on all the other three terminals on Windows, I think we should stick with it there. Windows users can learn the shortcuts from docs I suppose if they absolutely need to, assuming they won't just use right click.

Perhaps we could also modify the tab with keyboard shortcuts to show "Copy in terminal" and "Paste in terminal" if the platform is different than macOS? I think we don't need to show it on macOS since 99% of users are not going to ever change the default shortcut. It'd be mostly shown just for Windows and Linux users.

Another very cool thing we could add is "Show keyboard shortcuts" below "Open config file" in additional actions menu, but I think we could leave this for another time.

Since the equivalent of rightClick: 'copyPaste' is the default on all the other three terminals on Windows, I think we should stick with it there. Windows users can learn the shortcuts from docs I suppose if they absolutely need to, assuming they won't just use right click.

👍

Perhaps we could also modify the tab with keyboard shortcuts to show "Copy in terminal" and "Paste in terminal" if the platform is different than macOS?

This a really good idea, done.

Another very cool thing we could add is "Show keyboard shortcuts" below "Open config file" in additional actions menu, but I think we could leave this for another time.

👍

…ted in callsites, rename `copy` to `copySelection`

…ShortcutsPanel`

…enu`

ravicious

It's perfect now. 👌

… `true`

github-actions · 2024-08-08T13:32:22Z

🤖 Vercel preview here: https://docs-f8guyba33-goteleport.vercel.app/docs/ver/preview

github-actions · 2024-08-08T13:32:30Z

🤖 Vercel preview here: https://docs-cnftd2nsq-goteleport.vercel.app/docs/ver/preview

github-actions · 2024-08-08T13:42:36Z

🤖 Vercel preview here: https://docs-q02j4ix76-goteleport.vercel.app/docs/ver/preview

github-actions · 2024-08-08T13:55:40Z

🤖 Vercel preview here: https://docs-3gz84bcga-goteleport.vercel.app/docs/ver/preview

public-teleport-github-review-bot · 2024-08-08T15:05:53Z

@gzdunek See the table below for backport results.

Branch	Result
branch/v14	Failed
branch/v15	Failed
branch/v16	Create PR

* Allow reading clipboard content in renderer process * Make keyboard shortcuts to copy/paste text in terminal configurable, set reasonable defaults for Windows and Linux * Allow copying text in terminal with selection and a right click * Remove check for `selection.trim()`, check if there's something selected in callsites, rename `copy` to `copySelection` * Show copy/paste keyboard shortcuts in the context menu * Remove redundant `to` * Add `Copy in Terminal` and `Paste in Terminal` shortcuts to `KeyboardShortcutsPanel` * Set `rightClickSelectsWord: true` only if `terminal.rightClick` is `menu` * Add docs * Patched jsdom `global.matchMedia` should return `matches: false`, not `true` * Small docs correction (cherry picked from commit ee1a5ba)

gzdunek · 2024-08-08T15:22:09Z

I abandoned the backport to v14, too many conflicts.

* Allow reading clipboard content in renderer process * Make keyboard shortcuts to copy/paste text in terminal configurable, set reasonable defaults for Windows and Linux * Allow copying text in terminal with selection and a right click * Remove check for `selection.trim()`, check if there's something selected in callsites, rename `copy` to `copySelection` * Show copy/paste keyboard shortcuts in the context menu * Remove redundant `to` * Add `Copy in Terminal` and `Paste in Terminal` shortcuts to `KeyboardShortcutsPanel` * Set `rightClickSelectsWord: true` only if `terminal.rightClick` is `menu` * Add docs * Patched jsdom `global.matchMedia` should return `matches: false`, not `true` * Small docs correction (cherry picked from commit ee1a5ba)

Allow reading clipboard content in renderer process

17e572c

gzdunek added teleport-connect Issues related to Teleport Connect. backport/branch/v14 backport/branch/v15 backport/branch/v16 labels Jul 31, 2024

gzdunek commented Jul 31, 2024

View reviewed changes

gzdunek marked this pull request as ready for review July 31, 2024 12:45

gzdunek requested a review from ravicious July 31, 2024 12:45

github-actions bot requested review from avatus and rudream July 31, 2024 12:46

github-actions bot added size/md ui labels Jul 31, 2024

gzdunek removed the request for review from rudream July 31, 2024 12:46

Make keyboard shortcuts to copy/paste text in terminal configurable, …

8d734ab

…set reasonable defaults for Windows and Linux

gzdunek force-pushed the gzdunek/terminal-copy-paste branch from eeef7ed to 7a5dd36 Compare July 31, 2024 12:49

Allow copying text in terminal with selection and a right click

124874d

gzdunek force-pushed the gzdunek/terminal-copy-paste branch from 7a5dd36 to 124874d Compare July 31, 2024 13:07

avatus approved these changes Jul 31, 2024

View reviewed changes

gzdunek changed the title ~~Improve copy/paste behavior in terminal~~ Improve copy/paste behavior in Connect terminal Jul 31, 2024

ravicious reviewed Aug 1, 2024

View reviewed changes

gzdunek added 3 commits August 2, 2024 17:25

Remove check for selection.trim(), check if there's something selec…

8007a5f

…ted in callsites, rename `copy` to `copySelection`

Show copy/paste keyboard shortcuts in the context menu

5802eb0

Remove redundant to

b4ef2f5

ravicious self-requested a review August 6, 2024 14:35

gzdunek added 2 commits August 8, 2024 10:30

Add Copy in Terminal and Paste in Terminal shortcuts to `Keyboard…

3feba5e

…ShortcutsPanel`

Set rightClickSelectsWord: true only if terminal.rightClick is `m…

6139657

…enu`

ravicious approved these changes Aug 8, 2024

View reviewed changes

Add docs

787f68a

gzdunek temporarily deployed to vercel August 8, 2024 13:22 — with GitHub Actions Inactive

Merge branch 'master' into gzdunek/terminal-copy-paste

2b313e0

gzdunek temporarily deployed to vercel August 8, 2024 13:22 — with GitHub Actions Inactive

Patched jsdom global.matchMedia should return matches: false, not…

500750c

… `true`

gzdunek temporarily deployed to vercel August 8, 2024 13:32 — with GitHub Actions Inactive

Small docs correction

fa9ed77

gzdunek temporarily deployed to vercel August 8, 2024 13:45 — with GitHub Actions Inactive

gzdunek added this pull request to the merge queue Aug 8, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 8, 2024

gzdunek added this pull request to the merge queue Aug 8, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 8, 2024

gzdunek added this pull request to the merge queue Aug 8, 2024

Merged via the queue into master with commit ee1a5ba Aug 8, 2024
38 checks passed

gzdunek deleted the gzdunek/terminal-copy-paste branch August 8, 2024 15:03

This was referenced Aug 8, 2024

[v16] Improve copy/paste behavior in Connect terminal #45265

Merged

[v15] Improve copy/paste behavior in Connect terminal #45266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve copy/paste behavior in Connect terminal #44871

Improve copy/paste behavior in Connect terminal #44871

gzdunek commented Jul 31, 2024 •

edited

Loading

gzdunek Jul 31, 2024

ravicious Aug 1, 2024 •

edited

Loading

gzdunek Aug 2, 2024

ravicious Aug 7, 2024

gzdunek Aug 8, 2024

ravicious Aug 1, 2024

gzdunek Aug 2, 2024

ravicious Aug 7, 2024

gzdunek Aug 8, 2024

ravicious left a comment

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

public-teleport-github-review-bot bot commented Aug 8, 2024

gzdunek commented Aug 8, 2024

App	Shows context menu	Highlights word	Does copy/paste
cmd.exe	❌	❌	✅
Powershell	❌	❌	✅
Windows Terminal	❌	❌	✅
Terminal.app	✅	✅	❌
iTerm	✅	✅	❌
Terminal on Ubuntu	✅	❌	❌
Terminal on Fedora	✅	❌	❌

Improve copy/paste behavior in Connect terminal #44871

Improve copy/paste behavior in Connect terminal #44871

Conversation

gzdunek commented Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

ravicious Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravicious left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

public-teleport-github-review-bot bot commented Aug 8, 2024

gzdunek commented Aug 8, 2024

gzdunek commented Jul 31, 2024 •

edited

Loading

ravicious Aug 1, 2024 •

edited

Loading