Skip to content

Commit

Permalink
Fix crash and empty action in SUI Actions Page (#11427)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Fixes two issues related to SUI's Actions page:
1. Crash when adding an action and setting key chord to one that is already taken
   - **Cause**: the new key binding that was introduced with the "Add new" button appears in `_KeyBindingList` that we're iterating over. This has no `CurrentKeys()`, resulting in a null pointer exception.
   - **Fix**: null-check it
2. There's an action that appears as being nameless in the dropdown
   - **Cause**: The culprit seems to be `MultipleActions`. We would register it, but it wouldn't have a name, so it would appear as a nameless option.
   - **Fix**: if it has no name, don't register it. This is also future-proof in that any new nameless actions won't be automatically added.

Closes #10981
Part of #11353
  • Loading branch information
carlos-zamora authored Oct 6, 2021
1 parent f7b5b5c commit 14d068f
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsEditor/Actions.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "pch.h"
Expand Down Expand Up @@ -369,7 +369,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
const auto kbdVM{ get_self<KeyBindingViewModel>(_KeyBindingList.GetAt(i)) };
const auto& otherKeys{ kbdVM->CurrentKeys() };
if (keys.Modifiers() == otherKeys.Modifiers() && keys.Vkey() == otherKeys.Vkey())
if (otherKeys && keys.Modifiers() == otherKeys.Modifiers() && keys.Vkey() == otherKeys.Vkey())
{
return i;
}
Expand Down
13 changes: 6 additions & 7 deletions src/cascadia/TerminalSettingsModel/ActionMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
static void RegisterShortcutAction(ShortcutAction shortcutAction, std::unordered_map<hstring, Model::ActionAndArgs>& list, std::unordered_set<InternalActionID>& visited)
{
const auto actionAndArgs{ make_self<ActionAndArgs>(shortcutAction) };
if (actionAndArgs->Action() != ShortcutAction::Invalid)
/*We have a valid action.*/
/*Check if the action was already added.*/
if (visited.find(Hash(*actionAndArgs)) == visited.end())
{
/*We have a valid action.*/
/*Check if the action was already added.*/
if (visited.find(Hash(*actionAndArgs)) == visited.end())
/*This is an action that wasn't added!*/
/*Let's add it if it has a name.*/
if (const auto name{ actionAndArgs->GenerateName() }; !name.empty())
{
/*This is an action that wasn't added!*/
/*Let's add it.*/
const auto name{ actionAndArgs->GenerateName() };
list.insert({ name, *actionAndArgs });
}
}
Expand Down

0 comments on commit 14d068f

Please sign in to comment.