Skip to content
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

Merged
merged 96 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
90627b3
add origin tag
PankajBhojwani Mar 5, 2024
9dff28f
update calls in tests
PankajBhojwani Mar 5, 2024
8bcbd0b
fix tests
PankajBhojwani Mar 5, 2024
052d063
ah one of the tests uses this
PankajBhojwani Mar 5, 2024
8cc82de
generated
PankajBhojwani Mar 5, 2024
642d0ab
inbox makes more sense
PankajBhojwani Mar 5, 2024
66fe08f
default ids
PankajBhojwani Mar 7, 2024
2bb1b6c
conflict
PankajBhojwani Mar 19, 2024
be193b2
merge origin
PankajBhojwani Mar 19, 2024
db528c9
generate IDs for user commands
PankajBhojwani Mar 26, 2024
7c907fe
nits
PankajBhojwani Mar 26, 2024
b43191d
spacing
PankajBhojwani Mar 26, 2024
2093660
line
PankajBhojwani Mar 26, 2024
6c32539
string of numbers is unsightly but it works
PankajBhojwani Mar 27, 2024
eccd87f
update comment
PankajBhojwani Mar 27, 2024
44510dc
move id generation to fixupusersettings
PankajBhojwani Mar 28, 2024
10d1fc8
this way is better
PankajBhojwani Mar 28, 2024
71bf90f
even better, also get the ID from json
PankajBhojwani Mar 28, 2024
d57c7a1
move this
PankajBhojwani Mar 28, 2024
5c2307c
fix test
PankajBhojwani Mar 28, 2024
9fc6972
add tests
PankajBhojwani Mar 29, 2024
dca7df5
excess line
PankajBhojwani Mar 29, 2024
dd25ed7
change tests
PankajBhojwani Apr 1, 2024
6e293a5
Everytime
PankajBhojwani Apr 1, 2024
af2d22f
defaults conflict
PankajBhojwani Apr 11, 2024
bdf42c2
first round of nits
PankajBhojwani Apr 11, 2024
12f3aa9
truncate and hex, debug assert
PankajBhojwani Apr 12, 2024
aa49212
null check
PankajBhojwani Apr 12, 2024
5ee630e
fmt is smart
PankajBhojwani Apr 12, 2024
360b92e
fmt_compile, fix test
PankajBhojwani Apr 17, 2024
5e70911
remove 0
PankajBhojwani Apr 17, 2024
ca3eb87
rename and comment
PankajBhojwani Apr 17, 2024
85933e2
midpoint
PankajBhojwani Apr 23, 2024
c134402
about to test stage 1
PankajBhojwani Apr 24, 2024
22ab936
works??
PankajBhojwani Apr 24, 2024
0a3e17e
edge cases
PankajBhojwani Apr 24, 2024
e28d478
some todos for later
PankajBhojwani Apr 24, 2024
f425746
remove keysmap
PankajBhojwani Apr 24, 2024
d0938e2
ugly way to make sure we fixup
PankajBhojwani Apr 25, 2024
12a61c5
shows up in sui and all keybindings work
PankajBhojwani Apr 26, 2024
f1633e0
overwritten IDs and overwritten keychords show up properly in the SUI
PankajBhojwani Apr 26, 2024
ddfac90
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani Apr 26, 2024
3e7ab38
sui works?
PankajBhojwani Apr 26, 2024
ae16a5e
started stage 3
PankajBhojwani Apr 26, 2024
dc874c3
rename to special/standard
PankajBhojwani Apr 27, 2024
936afd6
_getactionbyid no longer returns optional
PankajBhojwani Apr 27, 2024
b3e9c26
remove check for invalid
PankajBhojwani Apr 27, 2024
5a1b822
reimplement populating all known keybindings
PankajBhojwani Apr 30, 2024
c51558f
unmark these
PankajBhojwani Apr 30, 2024
754bf04
mark gh todo
PankajBhojwani Apr 30, 2024
2f1d8d2
update defaults
PankajBhojwani Apr 30, 2024
2b4aeb2
don't check for special in standard
PankajBhojwani Apr 30, 2024
e62dfa2
some comments
PankajBhojwani Apr 30, 2024
e725f1e
resolve conflict
PankajBhojwani Apr 30, 2024
db00b90
spelling things
PankajBhojwani Apr 30, 2024
3d92f27
format
PankajBhojwani Apr 30, 2024
428821b
remove _idwasgenerated
PankajBhojwani Apr 30, 2024
6437b9f
fix user defaults file
PankajBhojwani Apr 30, 2024
4c744e6
misc
PankajBhojwani Apr 30, 2024
ca4015f
only one tojson
PankajBhojwani Apr 30, 2024
45cfcd6
just add duplicate pane auto to defaults
PankajBhojwani Apr 30, 2024
3c6015d
remove _getcumulativeactions
PankajBhojwani Apr 30, 2024
c2c75c8
bandaid temporary fix for name
PankajBhojwani May 1, 2024
3e601f5
better if
PankajBhojwani May 1, 2024
cdb907d
mark todo
PankajBhojwani May 1, 2024
f35bf20
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani May 1, 2024
2b16acd
check for name, fix some tests
PankajBhojwani May 1, 2024
193e573
fix remaining tests
PankajBhojwani May 2, 2024
0480d65
this is better
PankajBhojwani May 2, 2024
02a1e37
correct GH todo
PankajBhojwani May 2, 2024
80fc299
some new tests
PankajBhojwani May 3, 2024
ebc03e9
another test
PankajBhojwani May 3, 2024
ccf1cc9
nits
PankajBhojwani May 3, 2024
7793c5c
schema
PankajBhojwani May 3, 2024
abef25d
move this to header
PankajBhojwani May 3, 2024
4d35c14
schema conflict
PankajBhojwani May 3, 2024
3e31bda
generate here instead
PankajBhojwani May 6, 2024
14d83b5
delete user actions that are identical to inbox actions
PankajBhojwani May 6, 2024
5e48a45
update add action
PankajBhojwani May 7, 2024
ba375ec
remove keys from command
PankajBhojwani May 8, 2024
7d00b25
spelling
PankajBhojwani May 8, 2024
d4d216c
don't need helper anymore
PankajBhojwani May 8, 2024
6c6dd46
leonard comments
PankajBhojwani May 20, 2024
b88a8c5
eraseif
PankajBhojwani May 30, 2024
9703815
nits n fixes
PankajBhojwani May 31, 2024
a80316d
1 more test
PankajBhojwani May 31, 2024
625753c
x86 hash
PankajBhojwani Jun 1, 2024
406312f
fix loops
PankajBhojwani Jun 1, 2024
253dedf
conflict
PankajBhojwani Jun 3, 2024
5a00d5f
nits and changes
PankajBhojwani Jun 3, 2024
96d8d1f
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani Jun 4, 2024
327858b
Merge branch 'dev/pabhoj/action_refactor' into dev/pabhoj/command_keys
PankajBhojwani Jun 4, 2024
258c6eb
O(1), rename maps for clarity
PankajBhojwani Jun 4, 2024
6ea25e4
ID
PankajBhojwani Jun 4, 2024
589a1e0
IDTo
PankajBhojwani Jun 4, 2024
cc837b6
fix test
PankajBhojwani Jun 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/allow/allow.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ hyperlink
hyperlinking
hyperlinks
iconify
ID
img
inlined
issuetitle
Expand Down
3 changes: 2 additions & 1 deletion .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ idllib
IDOK
IDR
idth
IDTo
IDXGI
IEnd
IEnum
Expand Down Expand Up @@ -1870,7 +1871,7 @@ unk
unknwn
UNORM
unparseable
unregistering
Unregistering
untextured
UPDATEDISPLAY
UPDOWN
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/ActionPaletteItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;

namespace winrt::TerminalApp::implementation
{
ActionPaletteItem::ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command) :
ActionPaletteItem::ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command, const winrt::hstring keyChordText) :
_Command(command)
{
Name(command.Name());
KeyChordText(command.KeyChordText());
KeyChordText(keyChordText);
Icon(command.IconPath());
}
}
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/ActionPaletteItem.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace winrt::TerminalApp::implementation
struct ActionPaletteItem : ActionPaletteItemT<ActionPaletteItem, PaletteItem>
{
ActionPaletteItem() = default;
ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command);
ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command, const winrt::hstring keyChordText);

WINRT_PROPERTY(Microsoft::Terminal::Settings::Model::Command, Command, nullptr);

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/ActionPaletteItem.idl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace TerminalApp
{
[default_interface] runtimeclass ActionPaletteItem : PaletteItem
{
ActionPaletteItem(Microsoft.Terminal.Settings.Model.Command command);
ActionPaletteItem(Microsoft.Terminal.Settings.Model.Command command, String keyChordText);

Microsoft.Terminal.Settings.Model.Command Command { get; };
}
Expand Down
27 changes: 17 additions & 10 deletions src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -950,21 +950,27 @@ namespace winrt::TerminalApp::implementation
void CommandPalette::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap)
{
_actionMap = actionMap;
_populateCommands();
}

void CommandPalette::SetCommands(const Collections::IVector<Command>& actions)
Copy link
Contributor Author

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 with SetActionMap, so I figured it'd make more sense for this to just be a private helper that SetActionMap calls

void CommandPalette::_populateCommands()
{
_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);
}
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();
}
}
}

Expand Down Expand Up @@ -1178,7 +1184,8 @@ namespace winrt::TerminalApp::implementation
for (const auto& nameAndCommand : parentCommand.NestedCommands())
{
const auto action = nameAndCommand.Value();
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) };
// nested commands cannot have keys bound to them, so just pass in the command and no keys
auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, winrt::hstring{}) };
auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) };
_currentNestedCommands.Append(nestedFilteredCommand);
}
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/CommandPalette.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ namespace winrt::TerminalApp::implementation

Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::FilteredCommand> FilteredActions();

void SetCommands(const Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command>& actions);
void SetTabs(const Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase>& tabs, const Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase>& mruTabs);
void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap);

Expand Down Expand Up @@ -81,6 +80,8 @@ namespace winrt::TerminalApp::implementation

bool _lastFilterTextWasEmpty{ true };

void _populateCommands();

void _filterTextChanged(const Windows::Foundation::IInspectable& sender,
const Windows::UI::Xaml::RoutedEventArgs& args);
void _previewKeyDownHandler(const Windows::Foundation::IInspectable& sender,
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/CommandPalette.idl
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ namespace TerminalApp

Windows.Foundation.Collections.IObservableVector<FilteredCommand> FilteredActions { get; };

void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);

void SetTabs(Windows.Foundation.Collections.IObservableVector<TabBase> tabs, Windows.Foundation.Collections.IObservableVector<TabBase> mruTabs);

void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap);
Expand Down
12 changes: 4 additions & 8 deletions src/cascadia/TerminalApp/SuggestionsControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 */);
}
}
Expand Down Expand Up @@ -751,17 +751,13 @@ namespace winrt::TerminalApp::implementation
return _filteredActions;
}

void SuggestionsControl::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap)
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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, winrt::hstring{}) };
auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) };
_allCommands.Append(filteredCommand);
}
Expand Down Expand Up @@ -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, winrt::hstring{}) };
auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) };
_currentNestedCommands.Append(nestedFilteredCommand);
}
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/SuggestionsControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ namespace winrt::TerminalApp::implementation
Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::FilteredCommand> FilteredActions();

void SetCommands(const Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command>& actions);
void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap);

bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);

Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/SuggestionsControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ namespace TerminalApp
SuggestionsMode Mode { get; set; };

void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap);
void SelectNextItem(Boolean moveDown);

void Open(SuggestionsMode mode, IVector<Microsoft.Terminal.Settings.Model.Command> commands, String filterText, Windows.Foundation.Point anchor, Windows.Foundation.Size space, Single characterHeight);
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/SuggestionsControl.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
MinHeight="0"
Padding="16,0,12,0"
HorizontalContentAlignment="Stretch"
AutomationProperties.AcceleratorKey="{x:Bind Item.KeyChordText, Mode=OneWay}"
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
AutomationProperties.Name="{x:Bind Item.Name, Mode=OneWay}"
FontSize="12" />
</DataTemplate>
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ namespace winrt::TerminalApp::implementation
// to happen before the Settings UI is reloaded and tries to re-read those values.
if (const auto p = CommandPaletteElement())
{
p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
p.SetActionMap(_settings.ActionMap());
}

Expand Down Expand Up @@ -1828,7 +1827,6 @@ namespace winrt::TerminalApp::implementation
{
const auto p = FindName(L"CommandPaletteElement").as<CommandPalette>();

p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
p.SetActionMap(_settings.ActionMap());

// When the visibility of the command palette changes to "collapsed",
Expand Down