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

Add conditional ctrl+c/v to copy on integrated terminal similar to cmd #8818

Closed
Tyriar opened this issue Jul 6, 2016 · 13 comments
Closed

Add conditional ctrl+c/v to copy on integrated terminal similar to cmd #8818

Tyriar opened this issue Jul 6, 2016 · 13 comments
Assignees
Labels
feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jul 6, 2016

From @bldesign in #6451 (comment)

Maybe integrated terminal can present same feature as native Windows terminal, on Windows.

In the property options there are edition options with the ability to active ctrl shortcuts. It's just what we are talking about. It kills the process only if there is no selection. Cool no? VS Code can do it.

Sorry my preview is in french, but yours certainly looks the same.
terminalpropertiesoptions

With all options enabled. You can:

  1. Copy paste ctrl + c / v even when a process is running (kills only if no selection)
  2. Always paste ctrl + v (after process ended)
  3. Select text with mouse
  4. Select text with keyboard shift + /

You can copy, paste and select text only with keyboard on Windows.
(Both terminal, integrated and not are Microsoft Windows [version 10.0.10586])

Can you expose these options to user preferences?

@Tyriar Tyriar added feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label labels Jul 6, 2016
@Tyriar Tyriar added this to the Backlog milestone Jul 6, 2016
@Tyriar Tyriar self-assigned this Jul 6, 2016
@warpdesign
Copy link
Contributor

+1 that would be nice!

@vstoykov
Copy link

vstoykov commented Aug 1, 2016

Terminal shortcuts on Windows, Linux and Mac are different and probably need to be the same for the OS you are using.

  • Mac - Cmd+C/Cmd+V
  • Linux - Ctrl+Shift+C/Ctrl+Shift+V
  • Windows - Ctrl+C/Ctrl+V (I never new that you can use this in windows terminal)

Actually currently I'm unable to copy/paste anything to/from VS Code terminal. Right click menu with Copy/Paste options will be helpful.

P.S. Probably the problem that I have is related only to Linux (Ubuntu 14.04)

@Tyriar
Copy link
Member Author

Tyriar commented Aug 1, 2016

@vstoykov Insiders currently allows ctrl+shift+c/v on Windows and Linux, this is coming to Stable in a week or so. It also contains a context menu 😃

For Linux you can also use middle click to paste selection into the terminal, use this if you're sticking to stable for the time being.

@vstoykov
Copy link

vstoykov commented Aug 1, 2016

@Tyriar thanks for the info. I will wait for now until new stable version is ready.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 8, 2016

A change was just made that allows rebinding the terminal commands to anything and they should skip the terminal and work. This means that ctrl+c/v can be bound to copy and paste now, a terminalTextSelected state or similar needs to be implemented before conditional keybindings can work though.

You can try this in Insiders tomorrow.

@wclr
Copy link

wclr commented Aug 25, 2016

@Tyriar
Yes this is needed if terminalTextSelected ctr+C should copy the text.
I tried in insiders: If I use this config:

{
        "key": "ctrl+c",
        "command": "workbench.action.terminal.copySelection",
        "when": "terminalTextSelected"
    }
]

I see that keyboard combination for copy changed in the menu
image

But actually it doesn't work, maybe some other condition not terminalTextSelected?

@Tyriar
Copy link
Member Author

Tyriar commented Aug 25, 2016

@whitecolor the work in this issue is essentially to add terminalTextSelected.

@Tyriar
Copy link
Member Author

Tyriar commented Oct 23, 2016

Hey @alexandrudima, from what I can see all information around a command's context keys are currently private. Is it possible to expose this so that I can use IContextKeyService.contextMatchesRules to ensure I'm only allow keybindings that have valid when clauses to skip the terminal?

So given a set of command IDs (string[]), I need to get their keybindings (I'm currently using IKeybindingService.lookupKeybindings) in addition to their associated ContextKeyExpr (all hidden from what I can see).

See cf28f6d#diff-8c4484557359fefa25fe7396aa970c56R119

@alexdima
Copy link
Member

@Tyriar I've read through this issue, but I'm not fully understanding what you mean.

  • By default xterm interprets and eats up (stop propagation?) all keydown events
  • You have a hook from xterm in the form of attachCustomKeydownHandler.
  • If you return false for a keydown event, then xterm will not interpret and not eat up the event.
  • In consequence, if you return false, we get to interpret and dispatch the event possibly to a command in our world.
  • So you would like to "find out" ahead of time what the keydown event would do and if it would end up executing a command in a certain set, but without actually executing it now, then you'd return false, and the command would end up being executed by the keybindingsService eventually.

Is this what you mean?

@Tyriar
Copy link
Member Author

Tyriar commented Oct 27, 2016

@alexandrudima basically yeah, currently if the keybinding pressed matches one of the commands in some list it will pass it through. The problem is when implementing this is that I only want to have ctrl+c skip xterm only if the when property of the command is fulfilled. It's very important this only happens when the when property (terminalTextSelected) because ctrl+c is the keybinding the terminal uses to send SIGINT to the terminal.

@alexdima
Copy link
Member

I've extracted #14603 so we don't forget about it.

@wclr
Copy link

wclr commented Dec 4, 2016

Didn't forget?

@Tyriar
Copy link
Member Author

Tyriar commented Dec 4, 2016

I actually forgot about the branch I was working on for this, just merged into master!

@Tyriar Tyriar closed this as completed Dec 4, 2016
@Tyriar Tyriar modified the milestones: January 2017, Backlog Dec 4, 2016
@Tyriar Tyriar added the verification-needed Verification of issue is requested label Dec 4, 2016
@Tyriar Tyriar modified the milestones: November 2016, January 2017 Dec 4, 2016
@roblourens roblourens added the verified Verification succeeded label Dec 8, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants