Skip to content

Commit

Permalink
Improve handling of Space key combinations (#16645)
Browse files Browse the repository at this point in the history
This fixes two issues where the `Space` key wasn't being handled
correctly:

* Keyboards with an `AltGr`+`Space` mapping were not generating the
  expected character.
* Pressing a dead key followed by `Space` is supposed to generate the
  accent character associated with that key, but it wasn't doing so.

## References and Relevant Issues

These were both regressions from the keyboard refactor in PR #16511.

## Detailed Description of the Pull Request / Additional comments

The problem was that we were treating `VK_SPACE` as a "functional" key,
which means it gets hardcoded VT mappings which take precedence over
whatever is in the keyboard layout. This was deemed necessary to deal
with the fact that many keyboards incorrectly map `Ctrl`+`Space` as a
`SP` character, when it's expected to be `NUL`.

I've now dropped `VK_SPACE` from the functional mapping table and allow
it be handled by the default mapping algorithm for "graphic" keys.
However, I've also introduced a special case check for `Ctrl`+`Space`
(and other modifier variants), so we can bypass any incorrect keyboard
layouts for those combinations.

## Validation Steps Performed

I couldn't test with a French-BEPO keyboard layout directly, because the
MS Keyboard Layout Creator wouldn't accept a `Space` key mapping that
wasn't whitespace. However, if I remapped the `AltGr`+`Space` combo to
`LF`, I could confirm that we are now generating that correctly.

I've also tested the dead key `Space` combination on various keyboard
layouts and confirmed that that is now working correctly, and checked
that the `Ctrl`+`Space` combinations are still working too.

Closes #16641
Closes #16642
  • Loading branch information
j4james committed Feb 6, 2024
1 parent 5dda507 commit ec91be5
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/terminal/input/terminalInput.cpp
Expand Up @@ -230,8 +230,12 @@ TerminalInput::OutputType TerminalInput::HandleKey(const INPUT_RECORD& event)
return keyMatch->second;
}

// If it's not in the key map, we'll use the UnicodeChar, if provided.
if (unicodeChar != 0)
// If it's not in the key map, we'll use the UnicodeChar, if provided,
// except in the case of Ctrl+Space, which is often mapped incorrectly as
// a space character when it's expected to be mapped to NUL. We need to
// let that fall through to the standard mapping algorithm below.
const auto ctrlSpaceKey = ctrlIsReallyPressed && virtualKeyCode == VK_SPACE;
if (unicodeChar != 0 && !ctrlSpaceKey)
{
// In the case of an AltGr key, we may still need to apply a Ctrl
// modifier to the char, either because both Ctrl keys were pressed,
Expand Down Expand Up @@ -389,13 +393,6 @@ try
defineKeyWithAltModifier(Ctrl + Enhanced + VK_RETURN, L"\n"s);
defineKeyWithAltModifier(Ctrl + Shift + Enhanced + VK_RETURN, L"\n"s);

// SPACE maps to SP, and Ctrl+SPACE to NUL. The Shift modifier as no effect.
// The Alt modifier adds an ESC prefix (not standard).
defineKeyWithAltModifier(VK_SPACE, L" "s);
defineKeyWithAltModifier(Shift + VK_SPACE, L" "s);
defineKeyWithAltModifier(Ctrl + VK_SPACE, L"\0"s);
defineKeyWithAltModifier(Ctrl + Shift + VK_SPACE, L"\0"s);

if (_inputMode.test(Mode::Ansi))
{
// F1 to F4 map to the VT keypad function keys, which are SS3 sequences.
Expand Down Expand Up @@ -581,6 +578,10 @@ wchar_t TerminalInput::_makeCtrlChar(const wchar_t ch)
{
return ch & 0b11111;
}
if (ch == L' ')
{
return 0x00;
}
if (ch == L'/')
{
return 0x1F;
Expand Down

0 comments on commit ec91be5

Please sign in to comment.