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.

Sign up for GitHub

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

Jump to bottom

Add "recent commands" to Command Palette in commandline mode #8317

Merged
2 commits merged into from
Nov 25, 2020
Merged

Add "recent commands" to Command Palette in commandline mode #8317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10d2d64
8296: add in-memory history to command line mode
Don-Vito Nov 18, 2020
01ec21d
8296: add in-memory history to command line mode
Don-Vito Nov 18, 2020
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
89 changes: 47 additions & 42 deletions src/cascadia/TerminalApp/CommandPalette.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace winrt::TerminalApp::implementation
_currentNestedCommands = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_allCommands = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_tabActions = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_commandLineHistory = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();

_switchToMode(CommandPaletteMode::ActionMode);

Expand Down Expand Up @@ -303,21 +304,9 @@ namespace winrt::TerminalApp::implementation
}
else if (key == VirtualKey::Enter)
{
// Action, TabSwitch or TabSearchMode Mode: Dispatch the action of the selected command.
if (_currentMode != CommandPaletteMode::CommandlineMode)
{
const auto selectedCommand = _filteredActionsView().SelectedItem();
if (const auto filteredCommand = selectedCommand.try_as<winrt::TerminalApp::FilteredCommand>())
{
_dispatchCommand(filteredCommand);
}
}
// Commandline Mode: Use the input to synthesize an ExecuteCommandline action
else if (_currentMode == CommandPaletteMode::CommandlineMode)
{
_dispatchCommandline();
}

const auto selectedCommand = _filteredActionsView().SelectedItem();
const auto filteredCommand = selectedCommand.try_as<winrt::TerminalApp::FilteredCommand>();
_dispatchCommand(filteredCommand);
e.Handled(true);
}
else if (key == VirtualKey::Escape)
Expand Down Expand Up @@ -538,7 +527,7 @@ namespace winrt::TerminalApp::implementation
case CommandPaletteMode::TabSwitchMode:
return _tabActions;
case CommandPaletteMode::CommandlineMode:
return winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
return _commandLineHistory;
default:
return _allCommands;
}
Expand All @@ -555,7 +544,11 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::_dispatchCommand(winrt::TerminalApp::FilteredCommand const& filteredCommand)
{
if (filteredCommand)
if (_currentMode == CommandPaletteMode::CommandlineMode)
{
_dispatchCommandline(filteredCommand);
}
else if (filteredCommand)
{
if (filteredCommand.Command().HasNestedCommands())
{
Expand Down Expand Up @@ -628,32 +621,48 @@ namespace winrt::TerminalApp::implementation
// Method Description:
// - Dispatch the current search text as a ExecuteCommandline action.
// Arguments:
// - <none>
// - filteredCommand - Selected filtered command - might be null
// Return Value:
// - <none>
void CommandPalette::_dispatchCommandline()
void CommandPalette::_dispatchCommandline(winrt::TerminalApp::FilteredCommand const& command)
{
auto cmdline{ _getTrimmedInput() };
if (cmdline.empty())
const auto filteredCommand = command ? command : _buildCommandLineCommand(_getTrimmedInput());
if (filteredCommand.has_value())
{
return;
}
if (_commandLineHistory.Size() == CommandLineHistoryLength)
{
_commandLineHistory.RemoveAtEnd();
}
_commandLineHistory.InsertAt(0, filteredCommand.value());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you run the same command multiple times, will it keep getting added to the history? Should we try and de-duplicate entries in the history? (that is probably fine for a follow-up PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zadjii-msft - I took this approach because I didn't know what is the best behavior here, and in such cases I choose the simplest one. The options I considered were:

  • Set, sorted alphabetically
  • Set, sorted by MRU
  • List, sorted by MRU
  • A combination that you mentioned of don't push the command if it equals to the previous one

If you have a strong feeling about one of those, I can change this now or in followup (or we can add configs and leave to the user).


// Build the ExecuteCommandline action from the values we've parsed on the commandline.
ExecuteCommandlineArgs args{ cmdline };
ActionAndArgs executeActionAndArgs{ ShortcutAction::ExecuteCommandline, args };
TraceLoggingWrite(
g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider
"CommandPaletteDispatchedCommandline",
TraceLoggingDescription("Event emitted when the user runs a commandline in the Command Palette"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));

TraceLoggingWrite(
g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider
"CommandPaletteDispatchedCommandline",
TraceLoggingDescription("Event emitted when the user runs a commandline in the Command Palette"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));
if (_dispatch.DoAction(filteredCommand.value().Command().Action()))
{
_close();
}
}
}

if (_dispatch.DoAction(executeActionAndArgs))
std::optional<winrt::TerminalApp::FilteredCommand> CommandPalette::_buildCommandLineCommand(std::wstring const& commandLine)
{
if (commandLine.empty())
{
_close();
return std::nullopt;
}

// Build the ExecuteCommandline action from the values we've parsed on the commandline.
ExecuteCommandlineArgs args{ commandLine };
ActionAndArgs executeActionAndArgs{ ShortcutAction::ExecuteCommandline, args };
Command command{};
command.Action(executeActionAndArgs);
command.Name(commandLine);
Don-Vito marked this conversation as resolved.
Show resolved Hide resolved
return winrt::make<FilteredCommand>(command);
}

// Method Description:
Expand Down Expand Up @@ -698,7 +707,9 @@ namespace winrt::TerminalApp::implementation
_lastFilterTextWasEmpty = _searchBox().Text().empty();

_updateFilteredActions();
_filteredActionsView().SelectedIndex(0);

// In the command line mode we want the user to explicitly select the command
_filteredActionsView().SelectedIndex(_currentMode == CommandPaletteMode::CommandlineMode ? -1 : 0);

if (_currentMode == CommandPaletteMode::TabSearchMode || _currentMode == CommandPaletteMode::ActionMode)
{
Expand Down Expand Up @@ -871,7 +882,7 @@ namespace winrt::TerminalApp::implementation
// We want to present the commands sorted,
// unless we are in the TabSwitcherMode and TabSearchMode,
// in which we want to preserve the original order (to be aligned with the tab view)
if (_currentMode != CommandPaletteMode::TabSearchMode && _currentMode != CommandPaletteMode::TabSwitchMode)
if (_currentMode != CommandPaletteMode::TabSearchMode && _currentMode != CommandPaletteMode::TabSwitchMode && _currentMode != CommandPaletteMode::CommandlineMode)
{
std::sort(actions.begin(), actions.end(), FilteredCommand::Compare);
}
Expand All @@ -887,12 +898,6 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::_updateFilteredActions()
{
if (_currentMode == CommandPaletteMode::CommandlineMode)
{
_filteredActions.Clear();
return;
}

auto actions = _collectFilteredActions();

// Make _filteredActions look identical to actions, using only Insert and Remove.
Expand Down
7 changes: 6 additions & 1 deletion src/cascadia/TerminalApp/CommandPalette.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,17 @@ namespace winrt::TerminalApp::implementation
winrt::Windows::UI::Xaml::Controls::ListView::SizeChanged_revoker _sizeChangedRevoker;

void _dispatchCommand(winrt::TerminalApp::FilteredCommand const& command);
void _dispatchCommandline();
void _dispatchCommandline(winrt::TerminalApp::FilteredCommand const& command);
std::optional<winrt::TerminalApp::FilteredCommand> _buildCommandLineCommand(std::wstring const& commandLine);

void _dismissPalette();

void _scrollToIndex(uint32_t index);
uint32_t _getNumVisibleItems();

static constexpr int CommandLineHistoryLength = 10;
Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _commandLineHistory{ nullptr };

friend class TerminalAppLocalTests::TabTests;
};
}
Expand Down