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.

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

fix #84080 added cntl-shift-c and cntl-shift-v to terminal #84438

Merged
merged 4 commits into from Nov 17, 2019

Conversation

@Grommers00
Copy link
Contributor

Grommers00 commented Nov 11, 2019

Added functionality to the "secondary:" to include CNTL-SHIFT+C and CNTL-SHIFT+V so both are capable of being used. You can test this out in the terminal to make sure they are functional. I checked keyboard mapping and they appear there as well.

This PR fixes #84080

Grommers00 added 2 commits Nov 11, 2019
@Tyriar Tyriar self-assigned this Nov 11, 2019
@Grommers00

This comment has been minimized.

Copy link
Contributor Author

Grommers00 commented Nov 12, 2019

Will look into this! Thanks for the feedback!

@Grommers00

This comment has been minimized.

Copy link
Contributor Author

Grommers00 commented Nov 12, 2019

Hi @Tyriar - not sure if I'm doing something wrong.
if (BrowserFeatures.clipboard.readText) { actionRegistry.registerWorkbenchAction(new SyncActionDescriptor(TerminalPasteAction, TerminalPasteAction.ID, TerminalPasteAction.LABEL, { mac: { primary: KeyMod.CtrlCmd | KeyCode.KEY_V }, win: { primary: KeyCode.Ctrl | KeyCode.KEY_V, secondary: [KeyCode.Ctrl | KeyCode.Shift | KeyCode.KEY_V] }, linux: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KEY_V } }, KEYBINDING_CONTEXT_TERMINAL_FOCUS), 'Terminal: Paste into Active Terminal', category); },

But I assume this is what I'm trying to add. When I load "yarn watch"
and run vscode from ".\scripts\code.bat"

The loaded visual studio code doesn't seem to reflect any of my changes...is this because I'm using a windows machine and running through command prompt as per "Building and debugging via the Windows subsystem for Linux (WSL) is currently not supported."

Sorry to bug you! Thanks for all your help!

@Tyriar

This comment has been minimized.

Copy link
Member

Tyriar commented Nov 12, 2019

@Grommers00 the problem is probably because there is no longer a primary one, try removing mac and just making that the "primary" one:

primary: ...,
win: { primary: ..., secondary: ... },
linux: { primary: ... }
Grommers00 added 2 commits Nov 14, 2019
@Grommers00 Grommers00 requested a review from Tyriar Nov 14, 2019
if (BrowserFeatures.clipboard.writeText) {
actionRegistry.registerWorkbenchAction(new SyncActionDescriptor(CopyTerminalSelectionAction, CopyTerminalSelectionAction.ID, CopyTerminalSelectionAction.LABEL, {
primary: KeyMod.CtrlCmd | KeyCode.KEY_C,
win: { primary: KeyCode.Ctrl | KeyCode.KEY_C, secondary: [KeyCode.Ctrl | KeyCode.Shift | KeyCode.KEY_C] },

This comment has been minimized.

Copy link
@tracker1

tracker1 Nov 15, 2019

Would this work?

primary: KeyCode.CtrlCmd | KeyCode.Shift | KeyCode.KEY_C,
win: { secondary: KeyCode.Ctrl | KeyCode.KEY_C },
mac: { secondary: KeyCode.Cmd | KeyCode.KEY_C }

This comment has been minimized.

Copy link
@Grommers00

Grommers00 Nov 16, 2019

Author Contributor

Yes, I think that should work as well.

This comment has been minimized.

Copy link
@Tyriar

Tyriar Nov 17, 2019

Member

I'm not sure that works but if it did we wouldn't want cmd+shift+c to be bound on mac. I think they all need a primary.

@Tyriar
Tyriar approved these changes Nov 17, 2019
Copy link
Member

Tyriar left a comment

Thanks, looks great!

@Tyriar Tyriar added this to the November 2019 milestone Nov 17, 2019
@Tyriar Tyriar merged commit 720d085 into microsoft:master Nov 17, 2019
5 checks passed
5 checks passed
linux
Details
windows
Details
darwin
Details
VS Code #20191114.59 succeeded
Details
license/cla All CLA requirements met.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.