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

Sequence CTRL+\ is not detected in some keyboard layouts #8458

Closed
ojroques opened this issue Dec 2, 2020 · 15 comments · Fixed by #8870
Closed

Sequence CTRL+\ is not detected in some keyboard layouts #8458

ojroques opened this issue Dec 2, 2020 · 15 comments · Fixed by #8870
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@ojroques
Copy link

ojroques commented Dec 2, 2020

Environment

Windows build number: 10.0.19041.0
Windows Terminal version (if applicable): Windows Terminal Preview v1.5.3242.0

Edit: I've updated the description.

Here is the old one.

Steps to reproduce

  1. Try to use a mapping containing CTRL+\ in an application: for instance CTRL+\ CTRL+n in Neovim to quit the built-in terminal or by adding a new mapping in tmux.

Actual behavior

Nothing happens. It seems the CTRL+\ sequence is completely ignored by Windows Terminal. I'm using WSL1 with Ubuntu 20.04 but I've tried in Powershell and same issue. I've also tried with another terminal emulator (kitty) and CTRL+\ works fine there.

I found maybe related issues: #1288, #521 but they are marked resolved and I'm still getting the bug. Any idea why?

Steps to reproduce

  1. Change your keyboard layout to the UK extended layout
  2. Launch the Terminal and run showkey -a (I'm using Ubuntu 20.04 in WSL)
  3. Press CTRL-\

Actual behavior

Nothing happens. The CTRl-\ sequence goes undetected by the terminal. I've tried different layouts (US and French) and it works fine, so this issue seems tied to the choice of the keyboard layout.

Expected behavior

CTRL-\ is correctly detected. Maybe related issues: #1288, #521.

@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 Dec 2, 2020
@zadjii-msft
Copy link
Member

What keyboard layout do you use? Is it one that has an AltGr key?

Can you share the output of running showkey -a, then pressing ctrl+\? It should look something like:
image

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 2, 2020
@ojroques
Copy link
Author

ojroques commented Dec 2, 2020

I'm using the UK extended layout so yes it has an AltGr key. I tried showkey -a but Ctrl-\ outputs nothing. Other Ctrl sequences works fine though.
Edit: I've tried with different layouts (US and UK) and it works. So it seems it is related to the UK extended layout in particular.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 2, 2020
@sashkachan
Copy link

I am having the same problem with third party Colemak layout. Can't figure out why it's not passing through. In my case, I am trying Ctrl+_ (undo in Emacs). showkey also shows no input.

@ojroques
Copy link
Author

I'm aware that this isn't a high priority issue since very few users should be affected. But it's quite annoying to me because it prevents me to use some features of my editor of choice (the built-in terminal of Neovim).

I would like to take a look at the code and maybe try to find the issue myself. @zadjii-msft could you tell me where I should start looking?

@ojroques ojroques changed the title Unable to use any mapping containing 'CTRL+\' in apps Sequence CTRL+\ is not detected in some keyboard layouts Dec 13, 2020
@DHowett
Copy link
Member

DHowett commented Dec 17, 2020

The best place to start looking will be in Terminal.cpp's SendKeyEvent and SendCharEvent.

If you'd like to give us a quick debug trace, would you mind following the "input log" trace steps in this issue comment? Type out a couple Ctrl+\ sequences and send a screenshot (so we can see which records are red and which are not)

Thanks!

@DHowett DHowett added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Dec 17, 2020
@ojroques
Copy link
Author

Thanks for the info I'll look into the files your mentioned.

Here's the trace of me doing CTRL+\ using the UK extended keyboard layout:
image

There seems to be no trace of the release of \. When I change my keyboard layout to US, where the sequence works fine, I get this:
image

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 17, 2020
@ojroques
Copy link
Author

I've checked the source and it seems it's due to this particular line:

const auto result = ToUnicodeEx(vkey, scanCode, keyState.data(), buffer.data(), gsl::narrow_cast<int>(buffer.size()), 0b101, nullptr);
(related doc: https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-tounicodeex).

That function returns 0 meaning that the given vkey (here 220) has no associated Unicode character. And this is what we see in the screenshot when we use UK extended layout:
image

However what I don't understand is that when I switch to the standard UK layout, I see the same vkey but a different value for the unicode character:
image

I'd expect that for the same vkey + scan code we would get the same Unicode character... what is happening there?

@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Jan 21, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 21, 2021
@zadjii-msft
Copy link
Member

You know, that's a good question. I don't know what's happening there. Maybe @lhecker might?

@lhecker
Copy link
Member

lhecker commented Jan 22, 2021

I'll look into it. 👍

@lhecker lhecker self-assigned this Jan 22, 2021
@lhecker
Copy link
Member

lhecker commented Jan 24, 2021

@ojroques We use ToUnicodeEx to determine whether a given key event will likely produce a character (like "a") as opposed to a control character or similar (like pressing an arrow key). This is important as the operating system knows more about the current state of the keyboard than our application and will help us to support dead keys (´+e = é for instance).

@zadjii-msft This issue occurs, because some non-US keyboard layouts apparently do not contain Ctrl-key mappings at all:
wt-issue-8458

In fact it turns out that the affected keyboards layouts don't contain any mappings for Ctrl-key combinations. It appears as if Windows simply defaults a combination of Ctrl and any A-Z key to ^A-^Z, if the current keyboard layout has no mappings itself.
AFAICS the solution is to Ctrl+key combinations in TerminalInput::HandleKey even if GetCharData is zero. PR incoming.

@ghost ghost added the In-PR This issue has a related PR label Jan 24, 2021
@DHowett DHowett removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jan 28, 2021
@DHowett DHowett added this to the Terminal v1.7 milestone Jan 28, 2021
@DHowett
Copy link
Member

DHowett commented Jan 28, 2021

Triaged into 1.7 since we've got our best people looking at it. 😄

@ghost ghost closed this as completed in #8870 Feb 2, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 2, 2021
ghost pushed a commit that referenced this issue Feb 2, 2021
Pressing Ctrl+\ produces `^\` using the US keyboard layout, thanks to Ctrl-key mappings inside the keyboard layout, whereas some layouts, like the UK extended layout, don't contain those. This causes the character value to be zero and previously caused no VT sequence to be generated under these situations. This PR employs `MapVirtualKeyW` to infer the missing characters.
As a side effect this PR effectively causes _all_ major keys on the keyboard to produce Ctrl+combinations now.

## PR Checklist
* [x] Closes #8458
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests passed

## Validation Steps Performed

Compared all major keys in combination with Ctrl with the app store version of Terminal using `showkey` in WSL. All keys that previously worked still appear to continue to work.
DHowett added a commit that referenced this issue Feb 11, 2021
Dustin L. Howett (3)
* Move CharToKeyEvents (and friends) into InteractivityBase (GH-9106)
* Update Cascadia Code to 2102.03 (GH-9088)
* verison: bump to 1.7 on main

Josh Soref (1)
* ci: update to Spell check to 0.0.17a (CC-9014)

Leonard Hecker (3)
* Fixed GH-5205: Ctrl+Alt+2 doesn't send ^[^@ (CC-5272)
* Fix issues in tests.xml and OpenConsole.psm1 (CC-9011)
* Fix GH-8458: Handle all Ctrl-key combinations (CC-8870)

Mike Griese (1)
* Add support for running a commandline in another WT window (GH-8898)

Michael Niksa (1)
* Teach the renderer to keep thread alive if engine requests it (GH-9091)

Lachlan Picking (1)
* Fix shader time input (CC-8994)

PankajBhojwani (1)
* Separate runtime TerminalSettings from profile-TerminalSettings (CC-8602)

Chester Liu (2)
* Add support for paste filtering and bracketed paste mode (CC-9034)
* Add support for chaining OSC 10-12 (CC-8999)

Related work items: MSFT-31692939
@ghost
Copy link

ghost commented Mar 1, 2021

🎉This issue was addressed in #8870, which has now been successfully released as Windows Terminal Preview v1.7.572.0.:tada:

Handy links:

@XL64
Copy link

XL64 commented Feb 29, 2024

I have similar issue with french layout and ctrl+_ not correctly passed to the linux applications but as it can be reproduced within any other windows terminals I tried, i guess it's more a wsl issue. I opened an issue on WSL github repo : microsoft/WSL#11041

@j4james
Copy link
Collaborator

j4james commented Mar 1, 2024

@XL64 That sounds like #11194, which should have been fixed in the v1.20 preview release. I've just done a test with a French keyboard layout in WSL, and Ctrl+_ is definitely working as expected for me. If you can still reproduce the issue on version 1.20, it's best you open another issue and provide more details.

@XL64
Copy link

XL64 commented Jun 13, 2024

Indeed, it is fixed. what is strange is that other native windows terminal are affected too.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants