Skip to content

Commit

Permalink
conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
@PankajBhojwani
PankajBhojwani committed Jun 3, 2024
2 parents d4d216c + 406312f commit 253dedf
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 171 deletions.
122 changes: 52 additions & 70 deletions src/cascadia/TerminalSettingsModel/ActionMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,72 +66,66 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void ActionMap::_FinalizeInheritance()
{
// first, gather the inbox actions from the relevant parent
std::unordered_map<InternalActionID, Model::Command> InboxActions;
std::unordered_map<InternalActionID, Model::Command> inboxActions;
winrt::com_ptr<implementation::ActionMap> foundParent{ nullptr };
for (const auto parent : _parents)
for (const auto& parent : _parents)
{
for (const auto [_, cmd] : parent->_ActionMap)
const auto parentMap = parent->_ActionMap;
if (parentMap.begin() != parentMap.end() && parentMap.begin()->second.Origin() == OriginTag::InBox)
{
if (cmd.Origin() != OriginTag::InBox)
{
// only one parent contains all the inbox actions and that parent contains only inbox actions,
// so if we found a non-inbox action we can just skip to the next parent
break;
}
// only one parent contains all the inbox actions and that parent contains only inbox actions,
// so if we found an inbox action we know this is the parent we are looking for
foundParent = parent;
break;
}
}

if (foundParent)
{
for (const auto [_, cmd] : foundParent->_ActionMap)
for (const auto& [_, cmd] : foundParent->_ActionMap)
{
InboxActions.emplace(Hash(cmd.ActionAndArgs()), cmd);
inboxActions.emplace(Hash(cmd.ActionAndArgs()), cmd);
}
}

std::unordered_map<KeyChord, winrt::hstring, KeyChordHash, KeyChordEquality> keysToReassign;

// now, look through our _ActionMap for commands that
// - had an ID generated for them
// - do not have a name/icon path
// - have a hash that matches a command in the inbox actions
std::unordered_set<winrt::hstring> IdsToRemove;
for (const auto [userID, userCmd] : _ActionMap)
{
const auto userCmdImpl{ get_self<Command>(userCmd) };

// Note we don't need to explicitly check for the origin tag here since we only generate IDs for user actions,
// so if we ID was generated it means this is a user action
if (userCmdImpl->IdWasGenerated() && !userCmdImpl->HasName() && userCmd.IconPath().empty())
std::erase_if(_ActionMap, [&](const auto& pair) {
const auto userCmdImpl{ get_self<Command>(pair.second) };
if (userCmdImpl->IDWasGenerated() && !userCmdImpl->HasName() && userCmdImpl->IconPath().empty())
{
const auto userActionHash = Hash(userCmd.ActionAndArgs());
if (const auto inboxCmd = InboxActions.find(userActionHash); inboxCmd != InboxActions.end())
const auto userActionHash = Hash(userCmdImpl->ActionAndArgs());
if (const auto inboxCmd = inboxActions.find(userActionHash); inboxCmd != inboxActions.end())
{
for (auto [key, cmdID] : _KeyMap)
for (const auto& [key, cmdID] : _KeyMap)
{
// for any of our keys that point to the user action, point them to the inbox action instead
if (cmdID == userID)
if (cmdID == pair.first)
{
_KeyMap.insert_or_assign(key, inboxCmd->second.ID());
keysToReassign.insert_or_assign(key, inboxCmd->second.ID());
}
}

// add this ID to our set of IDs to remove
IdsToRemove.insert(userID);
// remove this pair
return true;
}
}
}
return false;
});

// now, remove the commands with the IDs we found
for (const auto id : IdsToRemove)
for (const auto [key, cmdID] : keysToReassign)
{
_ActionMap.erase(id);
_KeyMap.insert_or_assign(key, cmdID);
}
}

bool ActionMap::FixUpsAppliedDuringLoad() const
bool ActionMap::FixupsAppliedDuringLoad() const
{
return _fixUpsAppliedDuringLoad;
return _fixupsAppliedDuringLoad;
}

// Method Description:
Expand All @@ -141,7 +135,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// - actionID: the internal ID associated with a Command
// Return Value:
// - The command if it exists in this layer, otherwise nullptr
Model::Command ActionMap::_GetActionByID(const winrt::hstring actionID) const
Model::Command ActionMap::_GetActionByID(const winrt::hstring& actionID) const
{
// Check current layer
const auto actionMapPair{ _ActionMap.find(actionID) };
Expand All @@ -155,7 +149,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
return cmd;
}

for (const auto parent : _parents)
for (const auto& parent : _parents)
{
if (const auto inheritedCmd = parent->_GetActionByID(actionID))
{
Expand Down Expand Up @@ -211,9 +205,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
// Only populate AvailableActions with actions that haven't been visited already.
const auto actionID = Hash(cmd.ActionAndArgs());
if (visitedActionIDs.find(actionID) == visitedActionIDs.end())
if (!visitedActionIDs.contains(actionID))
{
const auto& name{ cmd.Name() };
const auto name{ cmd.Name() };
if (!name.empty())
{
// Update AvailableActions.
Expand Down Expand Up @@ -308,7 +302,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// there might be a collision here, where there could be 2 different commands with the same name
// in this case, prioritize the user's action
// TODO GH #17166: we should no longer use Command.Name to identify commands anywhere
if (nameMap.find(name) == nameMap.end() || cmd.Origin() == OriginTag::User)
if (!nameMap.contains(name) || cmd.Origin() == OriginTag::User)
{
// either a command with this name does not exist, or this is a user-defined command with a name
// in either case, update the name map with the command (if this is a user-defined command with
Expand All @@ -321,37 +315,39 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

// Method Description:
// - Recursively populate keyBindingsMap with ours and our parents' key -> id pairs
// - This is a bottom-up approach, ensuring that the keybindings of the parents are overridden by the children
// - This is a bottom-up approach
// - Keybindings of the parents are overridden by the children
void ActionMap::_PopulateCumulativeKeyMap(std::unordered_map<Control::KeyChord, winrt::hstring, KeyChordHash, KeyChordEquality>& keyBindingsMap)
{
for (const auto& [keys, cmdID] : _KeyMap)
{
if (keyBindingsMap.find(keys) == keyBindingsMap.end())
if (!keyBindingsMap.contains(keys))
{
keyBindingsMap.emplace(keys, cmdID);
}
}

for (const auto parent : _parents)
for (const auto& parent : _parents)
{
parent->_PopulateCumulativeKeyMap(keyBindingsMap);
}
}

// Method Description:
// - Recursively populate actionMap with ours and our parents' id -> command pairs
// - This is a bottom-up approach, ensuring that the actions of the parents are overridden by the children
// - This is a bottom-up approach
// - Actions of the parents are overridden by the children
void ActionMap::_PopulateCumulativeActionMap(std::unordered_map<hstring, Model::Command>& actionMap)
{
for (const auto& [cmdID, cmd] : _ActionMap)
{
if (actionMap.find(cmdID) == actionMap.end())
if (!actionMap.contains(cmdID))
{
actionMap.emplace(cmdID, cmd);
}
}

for (const auto parent : _parents)
for (const auto& parent : _parents)
{
parent->_PopulateCumulativeActionMap(actionMap);
}
Expand Down Expand Up @@ -385,7 +381,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
_PopulateCumulativeKeyMap(accumulatedKeybindingsMap);
_PopulateCumulativeActionMap(accumulatedActionsMap);

for (const auto [keys, cmdID] : accumulatedKeybindingsMap)
for (const auto& [keys, cmdID] : accumulatedKeybindingsMap)
{
if (const auto idCmdPair = accumulatedActionsMap.find(cmdID); idCmdPair != accumulatedActionsMap.end())
{
Expand Down Expand Up @@ -506,7 +502,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
}

// only add to the _ActionMap if there is an ID
if (auto cmdID = cmd.ID(); !cmdID.empty() && cmd.ActionAndArgs().Action() != ShortcutAction::Invalid)
if (auto cmdID = cmd.ID(); !cmdID.empty())
{
// in the legacy scenario, a user might have several of the same action but only one of them has defined an icon or a name
// eg. { "command": "paste", "name": "myPaste", "keys":"ctrl+a" }
Expand All @@ -521,7 +517,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// if so, we check if there already exists a command with that generated ID, and if there is we port over any name/icon there might be
// (this may cause us to overwrite in scenarios where the user has an existing command that has the same generated ID but
// performs a different action or has different args, but that falls under "play stupid games")
if (cmdImpl->IdWasGenerated())
if (cmdImpl->IDWasGenerated())
{
if (const auto foundCmd{ _GetActionByID(cmdID) })
{
Expand Down Expand Up @@ -562,14 +558,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// i.e. they provided an ID for a null command (which they really shouldn't, there's no purpose)
// in this case, we do _not_ want to use the id they provided, we want to use an empty id
// (empty id in the _KeyMap indicates the keychord was explicitly unbound)
if (cmd.ActionAndArgs().Action() == ShortcutAction::Invalid)
{
_KeyMap.insert_or_assign(keys, L"");
}
else
{
_KeyMap.insert_or_assign(keys, cmd.ID());
}
const auto action = cmd.ActionAndArgs().Action();
const auto id = action == ShortcutAction::Invalid ? hstring{} : cmd.ID();
_KeyMap.insert_or_assign(keys, id);
}

// Method Description:
Expand Down Expand Up @@ -612,15 +603,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
if (const auto keyIDPair = _KeyMap.find(keys); keyIDPair != _KeyMap.end())
{
if (const auto cmdID = keyIDPair->second; !cmdID.empty())
{
return cmdID;
}
else
{
// the keychord is defined in this layer, but points to an empty string - explicitly unbound
return L"";
}
// the keychord is defined in this layer, return the ID
return keyIDPair->second;
}

// search through our parents
Expand All @@ -632,7 +616,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
}
}

// we did not find the keychord anywhere, its not bound and not explicity unbound either
// we did not find the keychord anywhere, it's not bound and not explicitly unbound either
return std::nullopt;
}

Expand All @@ -649,18 +633,16 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
if (const auto actionIDOptional = _GetActionIdByKeyChordInternal(keys))
{
if (!(*actionIDOptional).empty())
if (!actionIDOptional->empty())
{
// 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;
}
// the ID is an empty string, these keys are explicitly unbound
return nullptr;
}

return std::nullopt;
Expand All @@ -673,7 +655,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// 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(winrt::hstring cmdID)
Control::KeyChord ActionMap::GetKeyBindingForAction(const winrt::hstring& cmdID)
{
if (!_ResolvedKeyActionMapCache)
{
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalSettingsModel/ActionMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
// queries
Model::Command GetActionByKeyChord(const Control::KeyChord& keys) const;
bool IsKeyChordExplicitlyUnbound(const Control::KeyChord& keys) const;
Control::KeyChord GetKeyBindingForAction(winrt::hstring cmdID);
Control::KeyChord GetKeyBindingForAction(const winrt::hstring& cmdID);

// population
void AddAction(const Model::Command& cmd, const Control::KeyChord& keys);
Expand All @@ -71,7 +71,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
std::vector<SettingsLoadWarnings> LayerJson(const Json::Value& json, const OriginTag origin, const bool withKeybindings = true);
Json::Value ToJson() const;
Json::Value KeyBindingsToJson() const;
bool FixUpsAppliedDuringLoad() const;
bool FixupsAppliedDuringLoad() const;

// modification
bool RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys);
Expand All @@ -85,7 +85,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
winrt::Windows::Foundation::Collections::IVector<Model::Command> FilterToSendInput(winrt::hstring currentCommandline);

private:
Model::Command _GetActionByID(const winrt::hstring actionID) const;
Model::Command _GetActionByID(const winrt::hstring& actionID) const;
std::optional<winrt::hstring> _GetActionIdByKeyChordInternal(const Control::KeyChord& keys) const;
std::optional<Model::Command> _GetActionByKeyChordInternal(const Control::KeyChord& keys) const;

Expand All @@ -109,7 +109,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
std::unordered_map<winrt::hstring, Model::Command> _NestedCommands;
std::vector<Model::Command> _IterableCommands;

bool _fixUpsAppliedDuringLoad;
bool _fixupsAppliedDuringLoad{ false };

// _KeyMap is the map of key chords -> action IDs defined in this layer
// _ActionMap is the map of action IDs -> commands defined in this layer
Expand Down
16 changes: 6 additions & 10 deletions src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
// there are keys in this command block meaning this is the legacy style -
// inform the loader that fixups are needed
_fixUpsAppliedDuringLoad = true;
_fixupsAppliedDuringLoad = true;
}

if (jsonBlock.isMember(JsonKey(ActionKey)) && !jsonBlock.isMember(JsonKey(IterateOnKey)) && origin == OriginTag::User && !jsonBlock.isMember(JsonKey(IDKey)))
{
// for non-nested non-iterable commands,
// if there's no ID in the command block we will generate one for the user -
// inform the loader that the ID needs to be written into the json
_fixUpsAppliedDuringLoad = true;
_fixupsAppliedDuringLoad = true;
}
}
else if (keys)
Expand Down Expand Up @@ -145,17 +145,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
Json::Value keybindingsList{ Json::ValueType::arrayValue };

auto toJson = [&keybindingsList](const KeyChord kc, const winrt::hstring cmdID) {
Json::Value keyIDPair{ Json::ValueType::objectValue };
JsonUtils::SetValueForKey(keyIDPair, KeysKey, kc);
JsonUtils::SetValueForKey(keyIDPair, IDKey, cmdID);
keybindingsList.append(keyIDPair);
};

// Serialize all standard keybinding objects in the current layer
for (const auto& [keys, cmdID] : _KeyMap)
{
toJson(keys, cmdID);
Json::Value keyIDPair{ Json::ValueType::objectValue };
JsonUtils::SetValueForKey(keyIDPair, KeysKey, keys);
JsonUtils::SetValueForKey(keyIDPair, IDKey, cmdID);
keybindingsList.append(keyIDPair);
}

return keybindingsList;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ bool SettingsLoader::FixupUserSettings()
};

auto fixedUp = userSettings.fixupsAppliedDuringLoad;
fixedUp = userSettings.globals->FixUpsAppliedDuringLoad() || fixedUp;
fixedUp = userSettings.globals->FixupsAppliedDuringLoad() || fixedUp;

fixedUp = RemapColorSchemeForProfile(userSettings.baseLayerProfile) || fixedUp;
for (const auto& profile : userSettings.profiles)
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalSettingsModel/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
if (const auto generatedID = actionAndArgsImpl->GenerateID(); !generatedID.empty())
{
_ID = generatedID;
_IdWasGenerated = true;
_IDWasGenerated = true;
}
}
}

bool Command::IdWasGenerated()
bool Command::IDWasGenerated()
{
return _IdWasGenerated;
return _IDWasGenerated;
}

void Command::Name(const hstring& value)
Expand Down
Loading

0 comments on commit 253dedf

Please sign in to comment.