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

Ctrl+Insert does not copy the selected text from Command Palette #9520

Open
KalleOlaviNiemitalo opened this issue Mar 17, 2021 · 10 comments
Open
Labels
Area-CmdPal Command Palette issues and features good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Milestone

Comments

@KalleOlaviNiemitalo
Copy link

Environment

Windows build number: 10.0.19042.867
Windows Terminal version (if applicable): 1.6.10571.0

Steps to reproduce

  1. Delete any saved Windows Terminal settings.
  2. Start Windows Terminal.
  3. Use the mouse to select some text in the pane.
  4. Press Ctrl+Shift+P to open the command palette.
  5. Type abc.
  6. Press Ctrl+A to select the text abc.
  7. Press Ctrl+Insert.

Expected behavior

The text abc should be copied to the clipboard, and the command palette should remain open.

Actual behavior

The command palette closes, and the text that you selected in the pane is copied to the clipboard.

Notes

If you copy by pressing Ctrl+C instead of Ctrl+Insert, then it works as expected.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 17, 2021
@KalleOlaviNiemitalo
Copy link
Author

I see, #9056 added special handling for Ctrl+C and Ctrl+V in the command palette, but not for Ctrl+Insert and Shift+Insert.

@zadjii-msft
Copy link
Member

Wait holy what now - Does ctrl+Ins just work (pretty much) everywhere in the OS? I did not know that. TIL! That sems like an easy enough fix.

@zadjii-msft zadjii-msft added Area-CmdPal Command Palette issues and features good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Mar 17, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Mar 17, 2021
@KalleOlaviNiemitalo
Copy link
Author

Wikipedia lists Ctrl+Insert copying as being part of the IBM Common User Access standard.

@DHowett
Copy link
Member

DHowett commented Mar 17, 2021

We have got to figure out what to do about key bindings and how they impact or do not impact certain scopes.

@DHowett
Copy link
Member

DHowett commented Mar 17, 2021

I'm inclined to reject a pull request at this point that adds a fourth band-aid to the command palette (AND THE SEARCH BOX) by introducing another "did the user press THIS key?" check.

@KalleOlaviNiemitalo
Copy link
Author

Maybe the precedence could depend on the action to which the key is bound.

@Don-Vito
Copy link
Contributor

What a headache. Everything starts with us not getting some keys in KeyDown handler (as they get handled by inner controls). As a result we register to PreviewKeyDown, that propagates top down. And this gives the key bindings a priority over the controls (e.g., ctrl+c key binding would get priority over ctrl+c in search box). To address this we have added special flows in the PreviewKeyDown handler, covering ctr+c, but not ctrl+insert (which I would expect to work if it was not bound).

As Dustin mentioned we need to find a good approach.

I really don't understand the decision not to bubble up some keys.

@KalleOlaviNiemitalo
Copy link
Author

If I am renaming a tab in Windows Terminal Preview 1.7.572.0 and press the Left arrow key when the insertion point is already at the beginning of the edit box, then it closes the edit box and commits the change as if I had pressed Enter. Is that a consequence of keys being bubbled up and the edit box deciding not to handle the key? If so, I hope this behavior won't spread to the command palette and the find box.

@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 26, 2021
@Don-Vito
Copy link
Contributor

Don-Vito commented Mar 26, 2021

If I am renaming a tab in Windows Terminal Preview 1.7.572.0 and press the Left arrow key when the insertion point is already at the beginning of the edit box, then it closes the edit box and commits the change as if I had pressed Enter. Is that a consequence of keys being bubbled up and the edit box deciding not to handle the key? If so, I hope this behavior won't spread to the command palette and the find box.

@KalleOlaviNiemitalo - not sure how you find all these 😄. I think this one is unrelated, the Left key switches to another tab (I guess the TabView catches it and decides to switch to an adjacent), causing the renamer box to lose focus. If we want to fix this, we should probably open a separate ticket

@eleadufresne
Copy link

eleadufresne commented Nov 14, 2024

Hello everyone! I'd like to address this issue by adding a conditional check for CTRL with either Insert or C for the copy action, similar to the approach used in PR #9056.

I noticed that Insert is just a virtual key (rather than a modifier), which means it doesn't trigger the same way on "KeyDown." So, while this doesn't solve the underlying issues mentioned in the comments, it should fix the copy behaviour in the command palette with minimal changes.

Please let me know if this direction works for you! If this sounds good, I can create a new branch and a draft PR. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CmdPal Command Palette issues and features good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

5 participants