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

Pressing escape in the terminal should clear the selection #46728

Closed
kieferrm opened this issue Mar 27, 2018 · 7 comments
Closed

Pressing escape in the terminal should clear the selection #46728

kieferrm opened this issue Mar 27, 2018 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities terminal General terminal issues that don't fall under another label verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@kieferrm
Copy link
Member

Testing #46594.

  1. Select command output that was previously scrolled out of the viewport
  2. Realize you were off by one and selected the wrong output
  3. Press ESC
    -> The output is deselected and the terminal scroll to the prompt. If the command was several dozen commands before, I have to go all the way back to correct an off-by-one mistake.

I'm unsure what the right thing is. I'd expect the first ESC to remove the selection and the second to jump back. But I'm not sure how many flows would break with that.

@kieferrm kieferrm added the terminal General terminal issues that don't fall under another label label Mar 27, 2018
@Tyriar
Copy link
Member

Tyriar commented Mar 27, 2018

The behavior you're after seems reasonable, I think to do that we would need a terminalHasSelection context key (like editorHasSelection) and then add a terminal.integrated.clearSelection to ESC. Open to PRs.

@Tyriar Tyriar added help wanted Issues identified as good community contribution opportunities feature-request Request for new features or functionality good first issue Issues identified as good for first-time contributors labels Mar 27, 2018
@Tyriar Tyriar changed the title Unselecting scrolls terminal Pressing escape in the terminal should clear the selection Mar 27, 2018
@ddruker
Copy link
Contributor

ddruker commented Mar 28, 2018

@Tyriar I can jump on this one. I'd like to complete a few of these "good first issue" issues before moving on. I already have a PR for another one. How does that sound?

@Tyriar
Copy link
Member

Tyriar commented Mar 28, 2018

@ddruker sounds good! Also I just realized that we already have such a context key terminalTextSelected, so it will be easier than I expected 😃

@ddruker
Copy link
Contributor

ddruker commented Mar 31, 2018

@Tyriar Just wondering. I have this working by adding it to terminal.integrated.commandsToSkipShell and works well but would you prefer this as a separate configuration called terminal.integrated.clearSelection? We do need to control the keybinding here. Please see the working UX below.

The user can select and when escape is pressed, it will clear the selection. When there is no selection, it will default to normal behavior.

esc

@Tyriar
Copy link
Member

Tyriar commented Mar 31, 2018

@ddruker looks good! We shouldn't have a setting for this as people can unbind the keybinding. For commandsToSkipShell you want to add the command to the default values for that setting:

https://github.com/Microsoft/vscode/blob/b66f0ba3c70b08ffc046a9f1a6f56e0260129540/src/vs/workbench/parts/terminal/electron-browser/terminal.contribution.ts#L207

@ddruker
Copy link
Contributor

ddruker commented Mar 31, 2018

Thanks @Tyriar ! I look forward to the feedback.

@Tyriar Tyriar added this to the April 2018 milestone Apr 2, 2018
@Tyriar
Copy link
Member

Tyriar commented Apr 2, 2018

Fixed in #47042, thanks @ddruker 👍

@Tyriar Tyriar closed this as completed Apr 2, 2018
@Tyriar Tyriar added the verification-needed Verification of issue is requested label Apr 23, 2018
@isidorn isidorn added the verified Verification succeeded label Apr 24, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 17, 2018
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 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities 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

4 participants