Repository: microsoft/terminal
File: src/cascadia/TerminalSettingsEditor/KeyChordListener.cpp (C++)

Code before:
        }
    }

    void KeyChordListener::_OnKeysChanged(DependencyObject const& d, DependencyPropertyChangedEventArgs const& e)
    {
        if (auto control{ d.try_as<Editor::KeyChordListener>() })
        {
            auto controlImpl{ get_self<KeyChordListener>(control) };
            TextBox tb{ controlImpl->FindName(L"KeyChordTextBox").as<TextBox>() };
            tb.Text(Model::KeyChordSerialization::ToString(unbox_value<Control::KeyChord>(e.NewValue())));
            if (auto automationPeer{ Automation::Peers::FrameworkElementAutomationPeer::FromElement(tb) })
            {
                automationPeer.RaiseNotificationEvent(
                    Automation::Peers::AutomationNotificationKind::ActionCompleted,
                    Automation::Peers::AutomationNotificationProcessing::MostRecent,
                    tb.Text(),
                    L"KeyChordListenerText");
            }
        }
    }

    void KeyChordListener::KeyChordTextBox_KeyDown(IInspectable const& /*sender*/, KeyRoutedEventArgs const& e)
    {
        const auto key{ e.OriginalKey() };
        for (const auto mod : ModifierKeys)
        {
            if (key == mod)
            {
                // Ignore modifier keys
                return;
            }
        }

        const auto modifiers{ _GetModifiers() };
        if (key == VirtualKey::Tab && (modifiers == VirtualKeyModifiers::None || modifiers == VirtualKeyModifiers::Shift))
        {
            // [Shift]+[Tab] && [Tab] are needed for keyboard navigation
            return;
        }

        // Permitted key events are used to update _Keys
        Keys({ modifiers, static_cast<int32_t>(key) });
        e.Handled(true);
    }
}


Review comment (suggestion):
I don't understand the need for `_GetModifiers`.
If you keep track over how many keys are simultaneously pressed you can use the key down events directly to assemble the modifiers bitfield. This removes the need for the majority of the `_GetModifiers` implementation.
(You'd also need to reset the modifiers when the control looses focus of course.)

Code after:
            TextBox tb{ controlImpl->FindName(L"KeyChordTextBox").as<TextBox>() };
            tb.Text(Model::KeyChordSerialization::ToString(unbox_value<Control::KeyChord>(e.NewValue())));
            if (auto automationPeer{ Automation::Peers::FrameworkElementAutomationPeer::FromElement(tb) })
            {
                automationPeer.RaiseNotificationEvent(
                    Automation::Peers::AutomationNotificationKind::ActionCompleted,
                    Automation::Peers::AutomationNotificationProcessing::MostRecent,
                    tb.Text(),
                    L"KeyChordListenerText");
            }
        }
    }

    void KeyChordListener::KeyChordTextBox_KeyDown(IInspectable const& /*sender*/, KeyRoutedEventArgs const& e)
    {
        const auto key{ e.OriginalKey() };
        for (const auto mod : ModifierKeys)
        {
            if (key == mod)
            {
                // Ignore modifier keys
                return;
            }
        }

        const auto modifiers{ _GetModifiers() };
        if (key == VirtualKey::Tab && (modifiers == VirtualKeyModifiers::None || modifiers == VirtualKeyModifiers::Shift))
        {
            // [Shift]+[Tab] && [Tab] are needed for keyboard navigation
            return;
        }

        // Permitted key events are used to update _Keys
        Keys({ modifiers, static_cast<int32_t>(key) });
        e.Handled(true);
    }
}