Skip to content

Commit

Permalink
edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
PankajBhojwani committed Apr 24, 2024
1 parent 22ab936 commit 0a3e17e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
47 changes: 38 additions & 9 deletions src/cascadia/TerminalSettingsModel/ActionMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -584,19 +584,19 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// If the actionID is empty, this keybinding is explicitly unbound
if (!actionID.empty())
{
const auto cmd{ _GetActionByID2(actionID).value() };
if (cmd)
const auto cmd{ _GetActionByID2(actionID) };
if (cmd.has_value())
{
// iterate over all of the action's bound keys
const auto cmdImpl{ get_self<Command>(cmd) };
const auto cmdImpl{ get_self<Command>(cmd.value()) };
for (const auto& keys : cmdImpl->KeyMappings())
{
// Only populate KeyBindingsMap with actions that...
// (1) haven't been visited already
// (2) aren't explicitly unbound
if (keyBindingsMap.find(keys) == keyBindingsMap.end() && unboundKeys.find(keys) == unboundKeys.end())
{
keyBindingsMap.emplace(keys, cmd);
keyBindingsMap.emplace(keys, cmd.value());
}
}
}
Expand Down Expand Up @@ -1081,31 +1081,60 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// - nullopt if it was not bound in this layer
std::optional<Model::Command> ActionMap::_GetActionByKeyChordInternal2(const Control::KeyChord& keys) const
{
// todo: stage 3 - why does this function search through parents but _GetActionByID2 doesn't? even the original implementation was similar with some recursive and some non-recursive functions
if (const auto keyIDPair = _KeyMap2.find(keys); keyIDPair != _KeyMap2.end())
{
if (const auto cmdID = keyIDPair->second; !cmdID.empty())
{
return _GetActionByID2(cmdID);
if (const auto cmd = _GetActionByID2(cmdID); cmd.has_value())
{
// standard case: both the keys and the ID are defined in this layer
return cmd;
}
else
{
for (const auto parent : _parents)
{
if (const auto inheritedCmd = parent->_GetActionByID2(cmdID); inheritedCmd.has_value())
{
// edge case 1: the keys are bound to an ID in this layer, but the ID is defined in one of our parents
return inheritedCmd;
}
}
}
}
else
{
// the keychord is in our map, but points to an empty string - explicitly unbound
// the keychord is defined in this layer, but points to an empty string - explicitly unbound
return nullptr;
}
}

// the command was not bound in this layer,
// ask my parents
// search through our parents
for (const auto& parent : _parents)
{
if (const auto parentKeyIDPair = parent->_KeyMap2.find(keys); parentKeyIDPair != parent->_KeyMap2.end())
{
if (const auto cmdID = parentKeyIDPair->second; !cmdID.empty())
{
if (const auto cmd = _GetActionByID2(cmdID); cmd.has_value())
{
// edge case 2: the keychord maps to an ID in one of our parents, but a command with that ID exists in this layer
// use the command from this layer
return cmd;
}
}
}

// we've checked for the standard case and the 2 edge cases, now we can recurse
const auto& inheritedCmd{ parent->_GetActionByKeyChordInternal2(keys) };
if (inheritedCmd)
{
return *inheritedCmd;
}
}

// we did not find the keychord in our map, its not bound and not explicity unbound either
// we did not find the keychord anywhere, its not bound and not explicity unbound either
return std::nullopt;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
winrt::hstring idJson;
if (JsonUtils::GetValueForKey(json, "keys", keys))
{
// even if the "id" field doesn't exist in the json, idJson will be an empty string which is fine
// if the "id" field doesn't exist in the json, idJson will be an empty string which is fine
JsonUtils::GetValueForKey(json, "id", idJson);

// any existing keybinding with the same keychord in this layer will get overwritten
Expand Down

0 comments on commit 0a3e17e

Please sign in to comment.