Skip to content

Commit

Permalink
generate here instead
Browse files Browse the repository at this point in the history
  • Loading branch information
@PankajBhojwani
PankajBhojwani committed May 6, 2024
1 parent 4d35c14 commit 3e31bda
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 38 deletions.
69 changes: 37 additions & 32 deletions src/cascadia/TerminalSettingsModel/ActionMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,49 +419,54 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
}

// Method Description:
// - Try to add the new command to _ActionMap.
// - If the command was added previously in this layer, populate oldCmd.
// - If the command was added previously in another layer, populate maskingCmd.
// - Try to add the new command to _ActionMap
// Arguments:
// - cmd: the action we're trying to register
// - oldCmd: the action found in _ActionMap, if one already exists
// - maskingAction: the action found in a parent layer, if one already exists
void ActionMap::_TryUpdateActionMap(const Model::Command& cmd)
{
// only add to the _ActionMap if there is an ID and the shortcut action is valid
// (if the shortcut action is invalid, then this is for unbinding and _TryUpdateKeyChord will handle that)
if (auto cmdID = cmd.ID(); !cmdID.empty() && cmd.ActionAndArgs().Action() != ShortcutAction::Invalid)
{
// 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" }
// { "command": "paste", "keys": "ctrl+b" }
// once they port over to the new implementation, we will reduce it to just one Command object with a generated ID
// but several key binding entries, like so
// { "command": "newTab", "id": "User.paste" } -> in the actions map
// { "keys": "ctrl+a", "id": "User.paste" } -> in the keybindings map
// { "keys": "ctrl+b", "id": "User.paste" } -> in the keybindings map
// however, we have to make sure that we preserve the icon/name that might have been there in one of the command objects
// to do that, we check if this command we're adding had an ID that was generated
// 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 the shortcut action is invalid, then this is for unbinding and _TryUpdateKeyChord will handle that
if (cmd.ActionAndArgs().Action() != ShortcutAction::Invalid)
{
const auto cmdImpl{ get_self<implementation::Command>(cmd) };
if (cmdImpl->IdWasGenerated())
if (cmd.Origin() == OriginTag::User && cmd.ID().empty())
{
// the user did not define an ID for their non-nested, non-iterable, valid command - generate one for them
cmdImpl->GenerateID();
}

// only add to the _ActionMap if there is an ID
if (auto cmdID = cmd.ID(); !cmdID.empty() && cmd.ActionAndArgs().Action() != ShortcutAction::Invalid)
{
if (const auto foundCmd{ _GetActionByID(cmdID) })
// 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" }
// { "command": "paste", "keys": "ctrl+b" }
// once they port over to the new implementation, we will reduce it to just one Command object with a generated ID
// but several key binding entries, like so
// { "command": "newTab", "id": "User.paste" } -> in the actions map
// { "keys": "ctrl+a", "id": "User.paste" } -> in the keybindings map
// { "keys": "ctrl+b", "id": "User.paste" } -> in the keybindings map
// however, we have to make sure that we preserve the icon/name that might have been there in one of the command objects
// to do that, we check if this command we're adding had an ID that was generated
// 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())
{
const auto foundCmdImpl{ get_self<implementation::Command>(foundCmd) };
if (foundCmdImpl->HasName() && !cmdImpl->HasName())
{
cmdImpl->Name(foundCmdImpl->Name());
}
if (!foundCmdImpl->IconPath().empty() && cmdImpl->IconPath().empty())
if (const auto foundCmd{ _GetActionByID(cmdID) })
{
cmdImpl->IconPath(foundCmdImpl->IconPath());
const auto foundCmdImpl{ get_self<implementation::Command>(foundCmd) };
if (foundCmdImpl->HasName() && !cmdImpl->HasName())
{
cmdImpl->Name(foundCmdImpl->Name());
}
if (!foundCmdImpl->IconPath().empty() && cmdImpl->IconPath().empty())
{
cmdImpl->IconPath(foundCmdImpl->IconPath());
}
}
}
_ActionMap.insert_or_assign(cmdID, cmd);
}
_ActionMap.insert_or_assign(cmdID, cmd);
}
}

Expand Down
6 changes: 0 additions & 6 deletions src/cascadia/TerminalSettingsModel/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
if (const auto actionJson{ json[JsonKey(ActionKey)] })
{
result->_ActionAndArgs = *ActionAndArgs::FromJson(actionJson, warnings);

// if this is a user-defined, non-iterable, valid command and they did not provide an id, generate one for them
if (result->_ID.empty() && result->_IterateOn == ExpandCommandType::None && result->_ActionAndArgs.Action() != ShortcutAction::Invalid && origin == OriginTag::User)
{
result->GenerateID();
}
}
else
{
Expand Down

0 comments on commit 3e31bda

Please sign in to comment.