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

During Alt+Numpad composition, stash keys in case we bail out #17774

Merged
merged 2 commits into from
Aug 23, 2024
Merged

During Alt+Numpad composition, stash keys in case we bail out #17774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a541561
During Alt+Numpad composition, stash keys in case we bail out
DHowett Aug 22, 2024
35af153
All PR feedback
DHowett Aug 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 51 additions & 33 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1539,11 +1539,24 @@ namespace winrt::Microsoft::Terminal::Control::implementation
auto encoding = s.encoding;
wchar_t buf[4]{};
size_t buf_len = 0;
bool handled = true;

if (encoding == AltNumpadEncoding::Unicode)
{
// UTF-32 -> UTF-16
if (s.accumulator <= 0xffff)
if (s.accumulator == 0)
{
// If the user pressed Alt + VK_ADD, then released Alt, they probably didn't intend to insert a numpad character at all.
// Send any accumulated key events instead.
lhecker marked this conversation as resolved.
Show resolved Hide resolved
for (auto&& e : _altNumpadState.cachedKeyEvents)
{
handled = handled && _TrySendKeyEvent(e.vkey, e.scanCode, e.modifiers, e.keyDown);
}
// Send the alt keyup we are currently processing
handled = handled && _TrySendKeyEvent(vkey, scanCode, modifiers, keyDown);
// do not accumulate into the buffer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh we should probably return h somewhere...

}
else if (s.accumulator <= 0xffff)
{
buf[buf_len++] = static_cast<uint16_t>(s.accumulator);
}
Expand Down Expand Up @@ -1587,7 +1600,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

s = {};
return true;
return handled;
}
// As a continuation of the above, this handles the key-down case.
if (modifiers.IsAltPressed())
Expand All @@ -1601,53 +1614,58 @@ namespace winrt::Microsoft::Terminal::Control::implementation
SCROLLLOCK_ON |
CAPSLOCK_ON;

if (keyDown && (modifiers.Value() & ~permittedModifiers) == 0)
if ((modifiers.Value() & ~permittedModifiers) == 0)
{
auto& s = _altNumpadState;

if (vkey == VK_ADD)
{
// Alt '+' <number> is used to input Unicode code points.
// Every time you press + it resets the entire state
// in the original OS implementation as well.
s.encoding = AltNumpadEncoding::Unicode;
s.accumulator = 0;
s.active = true;
}
else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0)
{
// Alt '0' <number> is used to input ANSI code points.
// Otherwise, they're OEM codepoints.
s.encoding = AltNumpadEncoding::ANSI;
s.active = true;
}
else
if (keyDown)
{
// Otherwise, append the pressed key to the accumulator.
const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10;
uint32_t add = 0xffffff;

if (vkey >= VK_NUMPAD0 && vkey <= VK_NUMPAD9)
if (vkey == VK_ADD)
{
add = vkey - VK_NUMPAD0;
// Alt '+' <number> is used to input Unicode code points.
// Every time you press + it resets the entire state
// in the original OS implementation as well.
s.encoding = AltNumpadEncoding::Unicode;
s.accumulator = 0;
s.active = true;
}
else if (vkey >= 'A' && vkey <= 'F')
else if (vkey == VK_NUMPAD0 && s.encoding == AltNumpadEncoding::OEM && s.accumulator == 0)
{
add = vkey - 'A' + 10;
// Alt '0' <number> is used to input ANSI code points.
// Otherwise, they're OEM codepoints.
s.encoding = AltNumpadEncoding::ANSI;
s.active = true;
}

// Pressing Alt + <not a number> should not activate the Alt+Numpad input, however.
if (add < base)
else
{
s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu);
s.active = true;
// Otherwise, append the pressed key to the accumulator.
const uint32_t base = s.encoding == AltNumpadEncoding::Unicode ? 16 : 10;
uint32_t add = 0xffffff;

if (vkey >= VK_NUMPAD0 && vkey <= VK_NUMPAD9)
{
add = vkey - VK_NUMPAD0;
}
else if (vkey >= 'A' && vkey <= 'F')
{
add = vkey - 'A' + 10;
}

// Pressing Alt + <not a number> should not activate the Alt+Numpad input, however.
if (add < base)
{
s.accumulator = std::min(s.accumulator * base + add, 0x10FFFFu);
s.active = true;
}
}
}

// If someone pressed Alt + <not a number>, we'll skip the early
// return and send the Alt key combination as per usual.
if (s.active)
{
// Cache it in case we have to emit it after alt is released
_altNumpadState.cachedKeyEvents.emplace_back(vkey, scanCode, modifiers, keyDown);
return true;
}

Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/TerminalControl/TermControl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
};
struct AltNumpadState
{
struct CachedKey
{
WORD vkey;
WORD scanCode;
::Microsoft::Terminal::Core::ControlKeyStates modifiers;
bool keyDown;
};
AltNumpadEncoding encoding = AltNumpadEncoding::OEM;
uint32_t accumulator = 0;
// Checking for accumulator != 0 to see if we have an ongoing Alt+Numpad composition is insufficient.
// The state can be active, while the accumulator is 0, if the user pressed Alt+Numpad0 which enabled
// the OEM encoding mode (= active), and then pressed Numpad0 again (= accumulator is still 0).
bool active = false;
til::small_vector<CachedKey, 4> cachedKeyEvents;
};
AltNumpadState _altNumpadState;

Expand Down
Loading