-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove command's knowledge of its keys #17215
Changes from 80 commits
90627b3
9dff28f
8bcbd0b
052d063
8cc82de
642d0ab
66fe08f
2bb1b6c
be193b2
db528c9
7c907fe
b43191d
2093660
6c32539
eccd87f
44510dc
10d1fc8
71bf90f
d57c7a1
5c2307c
9fc6972
dca7df5
dd25ed7
6e293a5
af2d22f
bdf42c2
12f3aa9
aa49212
5ee630e
360b92e
5e70911
ca3eb87
85933e2
c134402
22ab936
0a3e17e
e28d478
f425746
d0938e2
12a61c5
f1633e0
ddfac90
3e7ab38
ae16a5e
dc874c3
936afd6
b3e9c26
5a1b822
c51558f
754bf04
2f1d8d2
2b4aeb2
e62dfa2
e725f1e
db00b90
3d92f27
428821b
6437b9f
4c744e6
ca4015f
45cfcd6
3c6015d
c2c75c8
3e601f5
cdb907d
f35bf20
2b16acd
193e573
0480d65
02a1e37
80fc299
ebc03e9
ccf1cc9
7793c5c
abef25d
4d35c14
3e31bda
14d83b5
5e48a45
ba375ec
7d00b25
d4d216c
6c6dd46
b88a8c5
9703815
a80316d
625753c
406312f
253dedf
5a00d5f
96d8d1f
327858b
258c6eb
6ea25e4
589a1e0
cc837b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -950,21 +950,27 @@ | |
void CommandPalette::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) | ||
{ | ||
_actionMap = actionMap; | ||
_setCommands(); | ||
} | ||
|
||
void CommandPalette::SetCommands(const Collections::IVector<Command>& actions) | ||
void CommandPalette::_setCommands() | ||
{ | ||
_allCommands.Clear(); | ||
for (const auto& action : actions) | ||
if (_actionMap) | ||
{ | ||
auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) }; | ||
auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) }; | ||
_allCommands.Append(filteredCommand); | ||
} | ||
_allCommands.Clear(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this has a behavior difference - if you call Is this important? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this was ever being called with |
||
const auto expandedCommands{ _actionMap.ExpandedCommands() }; | ||
for (const auto& action : expandedCommands) | ||
{ | ||
const auto keyChordText{ KeyChordSerialization::ToString(_actionMap.GetKeyBindingForAction(action.ID())) }; | ||
auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, keyChordText) }; | ||
auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) }; | ||
_allCommands.Append(filteredCommand); | ||
} | ||
|
||
if (Visibility() == Visibility::Visible && _currentMode == CommandPaletteMode::ActionMode) | ||
{ | ||
_updateFilteredActions(); | ||
if (Visibility() == Visibility::Visible && _currentMode == CommandPaletteMode::ActionMode) | ||
{ | ||
_updateFilteredActions(); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -1178,7 +1184,8 @@ | |
for (const auto& nameAndCommand : parentCommand.NestedCommands()) | ||
{ | ||
const auto action = nameAndCommand.Value(); | ||
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) }; | ||
// nested commands cannot have keybinds bound to them, so just pass in the command and no keys | ||
|
||
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, L"") }; | ||
PankajBhojwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) }; | ||
_currentNestedCommands.Append(nestedFilteredCommand); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -508,7 +508,7 @@ namespace winrt::TerminalApp::implementation | |
automationPeer.RaiseNotificationEvent( | ||
Automation::Peers::AutomationNotificationKind::ItemAdded, | ||
Automation::Peers::AutomationNotificationProcessing::MostRecent, | ||
paletteItem.Name() + L" " + paletteItem.KeyChordText(), | ||
paletteItem.Name(), | ||
PankajBhojwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
L"SuggestionsControlSelectedItemChanged" /* unique name for this notification category */); | ||
} | ||
} | ||
|
@@ -751,17 +751,13 @@ namespace winrt::TerminalApp::implementation | |
return _filteredActions; | ||
} | ||
|
||
void SuggestionsControl::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was never being called anywhere |
||
{ | ||
_actionMap = actionMap; | ||
} | ||
|
||
void SuggestionsControl::SetCommands(const Collections::IVector<Command>& actions) | ||
{ | ||
_allCommands.Clear(); | ||
for (const auto& action : actions) | ||
{ | ||
auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) }; | ||
// key chords aren't relevant in the suggestions control, so make the palette item with just the command and no keys | ||
auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, L"") }; | ||
PankajBhojwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) }; | ||
_allCommands.Append(filteredCommand); | ||
} | ||
|
@@ -915,7 +911,7 @@ namespace winrt::TerminalApp::implementation | |
for (const auto& nameAndCommand : parentCommand.NestedCommands()) | ||
{ | ||
const auto action = nameAndCommand.Value(); | ||
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) }; | ||
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, L"") }; | ||
PankajBhojwani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) }; | ||
_currentNestedCommands.Append(nestedFilteredCommand); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SetCommands
was only ever being called in conjunction withSetActionMap
, so I figured it'd make more sense for this to just be a private helper thatSetActionMap
calls