Repository: microsoft/terminal (language: cpp)

Issue:
Ctrl+Insert does not copy the selected text from Command Palette
# Environment

```none
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.


Discussion / hints:
I see, <https://github.com/microsoft/terminal/pull/9056> added special handling for Ctrl+C and Ctrl+V in the command palette, but not for Ctrl+Insert and Shift+Insert.
Wait holy _what now_ - Does <kbd>ctrl+Ins</kbd> just work (pretty much) everywhere in the OS? I did _not_ know that. TIL! That sems like an easy enough fix.
Wikipedia lists Ctrl+Insert copying as being part of the [IBM Common User Access](https://en.wikipedia.org/wiki/IBM_Common_User_Access) standard.
We have _got_ to figure out what to do about key bindings and how they impact or do not impact certain scopes.
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.
Maybe the precedence could depend on the action to which the key is bound.
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.
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.
> 
> 
> 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 :smile:. 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

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](https://github.com/microsoft/terminal/pull/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. 😊
I would also like to take this up.

Fix patch:
diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp
index 4817cd52424..bcee25c72dd 100644
--- a/src/cascadia/TerminalApp/CommandPalette.cpp
+++ b/src/cascadia/TerminalApp/CommandPalette.cpp
@@ -359,7 +359,7 @@ namespace winrt::TerminalApp::implementation
             _switchToMode(CommandPaletteMode::CommandlineMode);
             e.Handled(true);
         }
-        else if (key == VirtualKey::C && ctrlDown)
+        else if ((key == VirtualKey::C || key == VirtualKey::Insert) && ctrlDown)
         {
             _searchBox().CopySelectionToClipboard();
             e.Handled(true);


Test patch: