Skip to content

Commit

Permalink
Allow the mapping of OEM keys ({}|\<>/_-=+) in key bindings
Browse files Browse the repository at this point in the history 
This commit introduces support for key bindings containing keys
traditionally classified as "OEM" keys. It uses VkKeyScanW and
MapVirtualKeyW, and translates the modifiers that come out of
VkKeyScanW to key chord modifiers.

The net result of this is that you can use bindings like "ctrl+|" in
your settings. That one in particular will be reserialized (and
displayed in any menus) as "ctrl+shift+\". Admittedly, this is not
clear, but it _is_ the truest representation of the key.

This commit also moves the Xaml key chord name override generator into
App as a static function, *AND* it forces its use for all modifier
names. This will present a localization issue, which will be helped in
part by #1972. This is required to work around
microsoft/microsoft-ui-xaml#708. I've kept the original code around
guarded by a puzzling ifdef, because it absolutely has value.

Fixes #1212.
  • Loading branch information
@DHowett
DHowett committed Jul 23, 2019
1 parent dca0ffe commit 5531286
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 45 deletions.
37 changes: 35 additions & 2 deletions src/cascadia/TerminalApp/App.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1395,13 +1395,41 @@ namespace winrt::TerminalApp::implementation
});
}

// Method Description:
// - Handles the special case of providing a text override for the UI shortcut due to VK_OEM issue.
// Looks at the flags from the KeyChord modifiers and provides a concatenated string value of all
// in the same order that XAML would put them as well.
// Return Value:
// - a WinRT hstring representation of the key modifiers for the shortcut
//NOTE: This needs to be localized with https://github.com/microsoft/terminal/issues/794 if XAML framework issue not resolved before then
static std::wstring _FormatOverrideShortcutText(Settings::KeyModifiers modifiers)
{
std::wstring buffer{ L"" };

if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Ctrl))
{
buffer += L"Ctrl+";
}
if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Shift))
{
buffer += L"Shift+";
}
if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Alt))
{
buffer += L"Alt+";
}

return buffer;
}

// Method Description:
// - Takes a MenuFlyoutItem and a corresponding KeyChord value and creates the accelerator for UI display.
// Takes into account a special case for an error condition for a comma
// Arguments:
// - MenuFlyoutItem that will be displayed, and a KeyChord to map an accelerator
void App::_SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::Settings::KeyChord& keyChord)
{
#ifdef DEP_MICROSOFT_UI_XAML_708_FIXED
// work around https://github.com/microsoft/microsoft-ui-xaml/issues/708 in case of VK_OEM_COMMA
if (keyChord.Vkey() != VK_OEM_COMMA)
{
Expand All @@ -1421,10 +1449,15 @@ namespace winrt::TerminalApp::implementation
menuItem.KeyboardAccelerators().Append(menuShortcut);
}
else // we've got a comma, so need to just use the alternate method
#endif
{
// extract the modifier and key to a nice format
auto overrideString = AppKeyBindings::FormatOverrideShortcutText(keyChord.Modifiers());
menuItem.KeyboardAcceleratorTextOverride(overrideString + L" ,");
auto overrideString = _FormatOverrideShortcutText(keyChord.Modifiers());
auto mappedCh = MapVirtualKeyW(keyChord.Vkey(), MAPVK_VK_TO_CHAR);
if (mappedCh != 0)
{
menuItem.KeyboardAcceleratorTextOverride(overrideString + gsl::narrow_cast<wchar_t>(mappedCh));
}
}
}

Expand Down
27 changes: 0 additions & 27 deletions src/cascadia/TerminalApp/AppKeyBindings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,33 +215,6 @@ namespace winrt::TerminalApp::implementation
return keyModifiers;
}

// Method Description:
// - Handles the special case of providing a text override for the UI shortcut due to VK_OEM_COMMA issue.
// Looks at the flags from the KeyChord modifiers and provides a concatenated string value of all
// in the same order that XAML would put them as well.
// Return Value:
// - a WinRT hstring representation of the key modifiers for the shortcut
//NOTE: This needs to be localized with https://github.com/microsoft/terminal/issues/794 if XAML framework issue not resolved before then
winrt::hstring AppKeyBindings::FormatOverrideShortcutText(Settings::KeyModifiers modifiers)
{
std::wstring buffer{ L"" };

if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Ctrl))
{
buffer += L"Ctrl+";
}
if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Shift))
{
buffer += L"Shift+";
}
if (WI_IsFlagSet(modifiers, Settings::KeyModifiers::Alt))
{
buffer += L"Alt+";
}

return winrt::hstring{ buffer };
}

// -------------------------------- Events ---------------------------------
// clang-format off
DEFINE_EVENT(AppKeyBindings, CopyText, _CopyTextHandlers, TerminalApp::CopyTextEventArgs);
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/AppKeyBindings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::Settings::KeyChord GetKeyBinding(TerminalApp::ShortcutAction const& action);

static Windows::System::VirtualKeyModifiers ConvertVKModifiers(winrt::Microsoft::Terminal::Settings::KeyModifiers modifiers);
static winrt::hstring FormatOverrideShortcutText(winrt::Microsoft::Terminal::Settings::KeyModifiers modifiers);

// clang-format off
DECLARE_EVENT(CopyText, _CopyTextHandlers, TerminalApp::CopyTextEventArgs);
Expand Down
45 changes: 30 additions & 15 deletions src/cascadia/TerminalApp/KeyChordSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,7 @@ static const std::unordered_map<int32_t, std::wstring_view> vkeyNamePairs {
{ VK_F22 , L"f22" },
{ VK_F23 , L"f23" },
{ VK_F24 , L"f24" },
{ VK_OEM_PLUS , L"plus" },
{ VK_OEM_COMMA , L"," },
{ VK_OEM_MINUS , L"-" },
{ VK_OEM_PERIOD , L"." }
// TODO:
// These all look like they'd be good keybindings, but change based on keyboard
// layout. How do we deal with that?
// #define VK_OEM_NEC_EQUAL 0x92 // '=' key on numpad
// #define VK_OEM_1 0xBA // ';:' for US
// #define VK_OEM_2 0xBF // '/?' for US
// #define VK_OEM_3 0xC0 // '`~' for US
// #define VK_OEM_4 0xDB // '[{' for US
// #define VK_OEM_5 0xDC // '\|' for US
// #define VK_OEM_6 0xDD // ']}' for US
// #define VK_OEM_7 0xDE // ''"' for US
{ VK_OEM_PLUS , L"plus" }
};
// clang-format on

Expand Down Expand Up @@ -178,6 +164,25 @@ winrt::Microsoft::Terminal::Settings::KeyChord KeyChordSerialization::FromString
}
}

// If we haven't found a key, attempt a keyboard mapping
if (!foundKey && part.size() == 1)
{
auto oemVk = VkKeyScanW(part[0]);
if (oemVk != -1)
{
vkey = oemVk & 0xFF;
auto oemModifiers = (oemVk & 0xFF00) >> 8;
// We're using WI_SetFlagIf instead of WI_UpdateFlag because we want to be strictly additive
// to the user's specified modifiers. ctrl+| should be the same as ctrl+shift+\,
// but if we used WI_UpdateFlag, ctrl+shift+\ would turn _off_ Shift because \ doesn't
// require it.
WI_SetFlagIf(modifiers, KeyModifiers::Shift, WI_IsFlagSet(oemModifiers, 1U));
WI_SetFlagIf(modifiers, KeyModifiers::Ctrl, WI_IsFlagSet(oemModifiers, 2U));
WI_SetFlagIf(modifiers, KeyModifiers::Alt, WI_IsFlagSet(oemModifiers, 4U));
foundKey = true;
}
}

// If we weren't able to find a match, throw an exception.
if (!foundKey)
{
Expand Down Expand Up @@ -240,6 +245,16 @@ winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord)
buffer += vkeyNamePairs.at(vkey);
serializedSuccessfully = true;
}
else
{
auto mappedChar = MapVirtualKeyW(vkey, MAPVK_VK_TO_CHAR);
if (mappedChar != 0)
{
wchar_t mappedWch = gsl::narrow_cast<wchar_t>(mappedChar);
buffer += std::wstring_view{ &mappedWch, 1 };
serializedSuccessfully = true;
}
}
}

if (!serializedSuccessfully)
Expand Down

0 comments on commit 5531286

Please sign in to comment.