Conversation
ghost
added
Area-SettingsUI
| @@ -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() }; | |||
There was a problem hiding this comment.
The & isn't necessarily needed here.
| if (actionAndArgs->Action() != ShortcutAction::Invalid) | ||
| /*We have a valid action.*/ | ||
| /*Check if the action was already added.*/ | ||
| if (visited.find(Hash(*actionAndArgs)) == visited.end()) |
There was a problem hiding this comment.
You can use the count() method instead of find() if you'd like.
The C-style comments could also be replaced with modern ones.
There was a problem hiding this comment.
I might have blocked over the two of these nits combined -- the comments don't follow our style at all (even spacing-wise), for example..?
There was a problem hiding this comment.
I didn't add the comments though...
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
🎉 Handy links: |
ghost
mentioned this pull request
4 participants
Summary of the Pull Request
Fixes two issues related to SUI's Actions page:
_KeyBindingListthat we're iterating over. This has noCurrentKeys(), resulting in a null pointer exception.MultipleActions. We would register it, but it wouldn't have a name, so it would appear as a nameless option.Closes #10981
Part of #11353