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.

Sign up for GitHub

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

Jump to bottom

Default bound key combinations are not working as expected on international keyboard layouts #7539

Closed
pstaag opened this issue Sep 5, 2020 · 5 comments · Fixed by #10666
Closed

Default bound key combinations are not working as expected on international keyboard layouts #7539

pstaag opened this issue Sep 5, 2020 · 5 comments · Fixed by #10666
Assignees
@lhecker
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v2.0

Comments

@pstaag
Copy link

pstaag commented Sep 5, 2020

Environment

Windows build number: Microsoft Windows [Version 10.0.19041.450]
Windows Terminal version (if applicable): 1.2.2381.0
  • Swedish keyboard layout

Steps to reproduce

invoke the Font Size Increase command ctrl+= on a Swedish keyboard layout.

(hint: that would be ctrl+shift+0)

Expected behaviour

Font size increases one step.

Actual behaviour

Nothing happens.

Reflections

  • The new Command Palette (that I am already a huge fan of, despite it's flaws) insists that Increase Font Size is bound to ctrl+shift+0 (bonus bug?)

  • Referring to the Modifier keys section in http://docs.microsoft.com/en-us/windows/terminal/customize-settings/key-bindings#key-binding-properties it appears that all the characters that are shift:ed on an American keyboard layout are invalid bindings. This theory is also widely supported by the fact that Windows Terminal binds ctrl+shift+numeral to newTab and ctrl+alt+numeral to switchToTab.

  • International keyboards often move the special characters around on the keyboard. (In the case with the Swedish keyboard layout, to fit in the three extra vowels in our 29 letter alphabet.)

  • It appears that the problem is how Terminal deals with characters that are bound to the numeric keys.
@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 Sep 5, 2020
@pstaag pstaag mentioned this issue Sep 5, 2020
Closed
@MiHondo2020
Copy link

It appears there are at least two conflicting visions of what MS terminal should be. One camp seems to be about making it something new and fixing historical issues (accepting that all of the world don't use English keyboards for instance). The other is about supporting the historical features of VT100+ that are widely used in the Linux world still. These are unlikely to be reconciled with a single configuration. For instance the VT* keyboard has to have separate key handlings, regardless of the desires consistency or non-English keyboards. So it seems there needs to be a VT100/200/etc mode , Xterm , and maybe a "MSTerm (ugh?)" mode which is targeted for non-terminal emulated applications.

@DHowett
Copy link
Member

DHowett commented Jul 2, 2021

@lhecker you might be interested in this one.

These are unlikely to be reconciled with a single configuration.

Yeah... you're definitely struck at the heart of the issue.

As an aside: sorry -- this fell out of our triage queue and we just found it again. I apologize!

@DHowett DHowett added Area-Settings Issues related to settings and customizability, for console or terminal Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Jul 2, 2021
@DHowett DHowett added this to the Terminal v2.0 milestone Jul 2, 2021
@DHowett
Copy link
Member

DHowett commented Jul 2, 2021

@lhecker remove the triage tag if you agree with the characterization and the milestone being set to 2.0, ok?

@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 2, 2021
@lhecker lhecker self-assigned this Jul 2, 2021
@lhecker
Copy link
Member

lhecker commented Jul 2, 2021 •
edited
Loading

@DHowett I believe setting it as a goal for v2.0 is a good idea and I'll gladly tackle it.
This issue is "basically" a sister-issue of #10203 and both can be solved simultaneously.

We need to introduce sc(N) and vk(N) to our shortcut sequences, which respectively stand for specific scan codes and virtual keys that are to be bound to an action. For instance Win+sc(29) would always bind to the key below the escape key, as virtually every keyboard in the world assigns the scan code 29 to the key below the escape key. 🙂

Now instead of binding zoom in/out to Ctrl+= and Ctrl+-, we need to bind it to Ctrl+vk(187) and Ctrl+vk(189) respectively. This is because for instance = on an US keyboard has the exact same virtual key as + on almost all international keyboards. Of course this doesn't work for all keyboards, but it should work for a significant fraction of them as far as I'm aware.

@lhecker lhecker mentioned this issue Jul 9, 2021
Merged
2 tasks
ghost pushed a commit that referenced this issue Jul 13, 2021
@lhecker
Enable /Zc:preprocessor (#10593)
32fbd4c 
This commit is a preparation for upcoming changes to KeyChordSerialization for #7539 and #10203.
In order to support variadic macros, /Zc:preprocessor was enabled, which required changing unrelated parts of the project.

## PR Checklist
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

* Project still compiles ✔️
@lhecker lhecker mentioned this issue Jul 14, 2021
Merged
ghost pushed a commit that referenced this issue Jul 14, 2021
@lhecker
Clean up KeyChordSerialization (#10654)
fca87b2 
This commit is a preparation for upcoming changes to
KeyChordSerialization for #7539 and #10203.  It introduces several
string helpers to simplify key chord parsing and get rid of our implicit
dependency on locale sensitive functions, which are known to behave
erratically.

Additionally key chord serialization used to depend on iteration order
of a hashmap which caused different strings to be returned for the same
key chord. This commit fixes the iteration order and will always return
the same string.

## Validation Steps Performed

* Key bindings are correctly parsed ✔️
* Key bindings are correctly serialized ❔
@lhecker lhecker mentioned this issue Jul 15, 2021
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Jul 15, 2021
@ghost ghost closed this as completed in #10666 Jul 20, 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 Jul 20, 2021
ghost pushed a commit that referenced this issue Jul 20, 2021
@lhecker
Introduce vk() and sc() key chord specifiers (#10666)
10b12ac 
This commit introduces an alternative to specifying key bindings as a combination of key modifiers and a character. It allows you to specify an explicit virtual key as `vk(nnn)`.
Additionally this commit makes it possible to bind actions to scan codes. As scan code 41 appears to be the button below the Escape key on virtually all keyboards, we'll be able to bind the quake mode hotkey to `win+sc(41)` and have it work consistently across most if not all keyboard layouts.

## PR Checklist
* [x] Closes #7539, Closes #10203
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

The following was tested both on US and DE keyboard layouts:
* Ctrl+, opens settings ✔️
* Win+` opens quake mode window ✔️
* Ctrl+plus/minus increase/decrease font size ✔️
Rosefield pushed a commit to Rosefield/terminal that referenced this issue Jul 22, 2021
@lhecker @Rosefield
Introduce vk() and sc() key chord specifiers (microsoft#10666)
2b468c7 
This commit introduces an alternative to specifying key bindings as a combination of key modifiers and a character. It allows you to specify an explicit virtual key as `vk(nnn)`.
Additionally this commit makes it possible to bind actions to scan codes. As scan code 41 appears to be the button below the Escape key on virtually all keyboards, we'll be able to bind the quake mode hotkey to `win+sc(41)` and have it work consistently across most if not all keyboard layouts.

## PR Checklist
* [x] Closes microsoft#7539, Closes microsoft#10203
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

The following was tested both on US and DE keyboard layouts:
* Ctrl+, opens settings ✔️
* Win+` opens quake mode window ✔️
* Ctrl+plus/minus increase/decrease font size ✔️
DHowett pushed a commit that referenced this issue Aug 25, 2021
@lhecker @DHowett
Enable /Zc:preprocessor (#10593)
f4de1e4 
This commit is a preparation for upcoming changes to KeyChordSerialization for #7539 and #10203.
In order to support variadic macros, /Zc:preprocessor was enabled, which required changing unrelated parts of the project.

* [x] I work here
* [x] Tests added/passed

* Project still compiles ✔️

(cherry picked from commit 32fbd4c)
@ghost
Copy link

ghost commented Aug 31, 2021

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

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lhecker lhecker

Labels
Area-Settings Issues related to settings and customizability, for console or terminal Impact-Correctness It be wrong. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting 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
Milestone
Terminal v2.0
Development

Successfully merging a pull request may close this issue.

Introduce vk() and sc() key chord specifiers microsoft/terminal
4 participants
@DHowett @lhecker @pstaag @MiHondo2020