Repository: microsoft/terminal
File: src/cascadia/TerminalSettingsModel/ActionMap.cpp (C++)

Code before:
            }
            // the ID is an empty string, these keys are explicitly unbound
            return nullptr;
        }

        return std::nullopt;
    }

    // Method Description:
    // - Retrieves the key chord for the provided action
    // Arguments:
    // - cmdID: the ID of the command we're looking for
    // Return Value:
    // - the key chord that executes the given action
    // - nullptr if the action is not bound to a key chord
    Control::KeyChord ActionMap::GetKeyBindingForAction(const winrt::hstring& cmdID)
    {
        if (!_ResolvedKeyActionMapCache)
        {
            _RefreshKeyBindingCaches();
        }

        // I dislike that we have to do an O(n) lookup every time we want to get the keybinding for an action -
        // an alternative is having the key->action map be a bi-map (would require a dependency), or store another map that is just
        // the reverse direction (action->key) which would be mean storing the same data twice but getting faster lookup
        for (const auto [key, action] : _ResolvedKeyActionMapCache)
        {
            if (action.ID() == cmdID)
            {
                // if there are multiple keys bound to this action, we will just return the first one we find
                return key;
            }
        }

        // This key binding does not exist
        return nullptr;
    }

    // Method Description:
    // - Rebinds a key binding to a new key chord
    // Arguments:
    // - oldKeys: the key binding that we are rebinding
    // - newKeys: the new key chord that is being used to replace oldKeys
    // Return Value:
    // - true, if successful. False, otherwise.
    bool ActionMap::RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys)
    {
        const auto cmd{ GetActionByKeyChord(oldKeys) };
        if (!cmd)
        {
            // oldKeys must be bound. Otherwise, we don't know what action to bind.

Review comment (performance):
fwiw, if these collections like _ResolvedKeyActionMapCache are NOT USED on the WinRT or IDL surface of this API, _they do not need to be winrt collection types_. They will be smaller and faster.

This one, for example, sounds like an std::unordered_map from keychord to command. Or, if we enumerate the entire thing, an std::unordered_set.

Code after:
            {
                // there is an ID associated with these keys, find the command
                if (const auto foundCmd = _GetActionByID(*actionIDOptional))
                {
                    return foundCmd;
                }
            }
            // the ID is an empty string, these keys are explicitly unbound
            return nullptr;
        }

        return std::nullopt;
    }

    // Method Description:
    // - Retrieves the key chord for the provided action
    // Arguments:
    // - cmdID: the ID of the command we're looking for
    // Return Value:
    // - the key chord that executes the given action
    // - nullptr if the action is not bound to a key chord
    Control::KeyChord ActionMap::GetKeyBindingForAction(const winrt::hstring& cmdID)
    {
        if (!_ResolvedKeyToActionMapCache)
        {
            _RefreshKeyBindingCaches();
        }

        if (_CumulativeActionToKeyMapCache.contains(cmdID))
        {
            return _CumulativeActionToKeyMapCache.at(cmdID);
        }

        // This key binding does not exist
        return nullptr;
    }

    // Method Description:
    // - Rebinds a key binding to a new key chord
    // Arguments:
    // - oldKeys: the key binding that we are rebinding
    // - newKeys: the new key chord that is being used to replace oldKeys
    // Return Value:
    // - true, if successful. False, otherwise.
    bool ActionMap::RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys)
    {
        const auto cmd{ GetActionByKeyChord(oldKeys) };
        if (!cmd)
        {
            // oldKeys must be bound. Otherwise, we don't know what action to bind.
            return false;