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

Teach CommandPalette to persist recent command lines #11030

Merged
7 commits merged into from
Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions src/cascadia/TerminalApp/CommandPalette.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ namespace winrt::TerminalApp::implementation
_allCommands = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_tabActions = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_mruTabActions = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();
_commandLineHistory = winrt::single_threaded_vector<winrt::TerminalApp::FilteredCommand>();

_switchToMode(CommandPaletteMode::ActionMode);

Expand Down Expand Up @@ -587,7 +586,7 @@ namespace winrt::TerminalApp::implementation
case CommandPaletteMode::TabSwitchMode:
return _tabSwitcherMode == TabSwitcherMode::MostRecentlyUsed ? _mruTabActions : _tabActions;
case CommandPaletteMode::CommandlineMode:
return _commandLineHistory;
return _loadRecentCommands();
default:
return _allCommands;
}
Expand Down Expand Up @@ -720,14 +719,10 @@ namespace winrt::TerminalApp::implementation
// - <none>
void CommandPalette::_dispatchCommandline(winrt::TerminalApp::FilteredCommand const& command)
{
const auto filteredCommand = command ? command : _buildCommandLineCommand(_getTrimmedInput());
const auto filteredCommand = command ? command : _buildCommandLineCommand(winrt::hstring(_getTrimmedInput()));
if (filteredCommand.has_value())
{
if (_commandLineHistory.Size() == CommandLineHistoryLength)
{
_commandLineHistory.RemoveAtEnd();
}
_commandLineHistory.InsertAt(0, filteredCommand.value());
_updateRecentCommands(filteredCommand.value().Item().Name());

TraceLoggingWrite(
g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider
Expand All @@ -744,15 +739,14 @@ namespace winrt::TerminalApp::implementation
}
}

std::optional<winrt::TerminalApp::FilteredCommand> CommandPalette::_buildCommandLineCommand(std::wstring const& commandLine)
std::optional<TerminalApp::FilteredCommand> CommandPalette::_buildCommandLineCommand(const hstring& commandLine)
{
if (commandLine.empty())
{
return std::nullopt;
Copy link
Member

Choose a reason for hiding this comment

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

Huh, I just realized this. A FilteredCommand is already nullable, because it is a smart pointer type. Is there value that we get out of using nullopt (which would mean "not null, and also not populated" to provide a third state?

}

winrt::hstring cl{ commandLine };
auto commandLinePaletteItem{ winrt::make<winrt::TerminalApp::implementation::CommandLinePaletteItem>(cl) };
auto commandLinePaletteItem{ winrt::make<CommandLinePaletteItem>(commandLine) };
return winrt::make<FilteredCommand>(commandLinePaletteItem);
}

Expand Down Expand Up @@ -1217,4 +1211,48 @@ namespace winrt::TerminalApp::implementation
itemContainer.DataContext(args.Item());
}
}

// Method Description:
// - Reads the list of recent commands from the persistent application state
// Return Value:
// - The list of FilteredCommand representing the ones stored in the state
IVector<TerminalApp::FilteredCommand> CommandPalette::_loadRecentCommands()
{
const auto recentCommands = ApplicationState::SharedInstance().RecentCommands();
std::vector<TerminalApp::FilteredCommand> parsedCommands;
parsedCommands.reserve(std::min(recentCommands.Size(), CommandLineHistoryLength));

for (const auto& c : recentCommands)
{
if (parsedCommands.size() >= CommandLineHistoryLength)
{
// Don't load more than CommandLineHistoryLength commands
break;
}

if (const auto parsedCommand = _buildCommandLineCommand(c))
{
parsedCommands.push_back(*parsedCommand);
}
}
return single_threaded_vector(std::move(parsedCommands));
}

// Method Description:
// - Update recent commands by putting the provided command as most recent.
// Upon race condition might override an update made by another window.
// Return Value:
// - <none>
void CommandPalette::_updateRecentCommands(const hstring& command)
{
const auto recentCommands = ApplicationState::SharedInstance().RecentCommands();
const auto countToCopy = std::min(recentCommands.Size(), CommandLineHistoryLength - 1);
std::vector<hstring> newRecentCommands{ countToCopy + 1 };
til::at(newRecentCommands, 0) = command;
if (countToCopy)
{
recentCommands.GetMany(0, { newRecentCommands.data() + 1, countToCopy });
}
ApplicationState::SharedInstance().RecentCommands(single_threaded_vector(std::move(newRecentCommands)));
}
}
7 changes: 4 additions & 3 deletions src/cascadia/TerminalApp/CommandPalette.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,16 @@ namespace winrt::TerminalApp::implementation
void _dispatchCommand(winrt::TerminalApp::FilteredCommand const& command);
void _dispatchCommandline(winrt::TerminalApp::FilteredCommand const& command);
void _switchToTab(winrt::TerminalApp::FilteredCommand const& command);
std::optional<winrt::TerminalApp::FilteredCommand> _buildCommandLineCommand(std::wstring const& commandLine);
static std::optional<winrt::TerminalApp::FilteredCommand> _buildCommandLineCommand(const winrt::hstring& 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 };
static constexpr uint32_t CommandLineHistoryLength = 20;
static Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _loadRecentCommands();
static void _updateRecentCommands(const winrt::hstring& command);
::TerminalApp::AppCommandlineArgs _appArgs;

void _choosingItemContainer(Windows::UI::Xaml::Controls::ListViewBase const& sender, Windows::UI::Xaml::Controls::ChoosingItemContainerEventArgs const& args);
Expand Down
42 changes: 42 additions & 0 deletions src/cascadia/TerminalSettingsModel/ApplicationState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,48 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
return fmt::format("{}[]", ConversionTrait<GUID>{}.TypeDescription());
}
};

template<typename T>
struct ConversionTrait<winrt::Windows::Foundation::Collections::IVector<T>>
Copy link
Member

Choose a reason for hiding this comment

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

everybody's writing this exact same converter this week, aren't they 😝

{
winrt::Windows::Foundation::Collections::IVector<T> FromJson(const Json::Value& json) const
{
ConversionTrait<T> trait;
std::vector<T> val;
val.reserve(json.size());

for (const auto& element : json)
{
val.push_back(trait.FromJson(element));
}

return winrt::single_threaded_vector(move(val));
}

bool CanConvert(const Json::Value& json) const
{
ConversionTrait<T> trait;
return json.isArray() && std::all_of(json.begin(), json.end(), [trait](const auto& json) -> bool { return trait.CanConvert(json); });
}

Json::Value ToJson(const winrt::Windows::Foundation::Collections::IVector<T>& val)
{
ConversionTrait<T> trait;
Json::Value json{ Json::arrayValue };

for (const auto& key : val)
{
json.append(trait.ToJson(key));
}

return json;
}

std::string TypeDescription() const
{
return fmt::format("vector ({})", ConversionTrait<T>{}.TypeDescription());
}
};
}

using namespace ::Microsoft::Terminal::Settings::Model;
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalSettingsModel/ApplicationState.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ Module Name:
// This macro generates all getters and setters for ApplicationState.
// It provides X with the following arguments:
// (type, function name, JSON key, ...variadic construction arguments)
#define MTSM_APPLICATION_STATE_FIELDS(X) \
X(std::unordered_set<winrt::guid>, GeneratedProfiles, "generatedProfiles")
#define MTSM_APPLICATION_STATE_FIELDS(X) \
X(std::unordered_set<winrt::guid>, GeneratedProfiles, "generatedProfiles") \
X(Windows::Foundation::Collections::IVector<hstring>, RecentCommands, "recentCommands")

namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/ApplicationState.idl
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ namespace Microsoft.Terminal.Settings.Model
void Reload();

String FilePath { get; };

Windows.Foundation.Collections.IVector<String> RecentCommands { get; set; };
}
}
4 changes: 2 additions & 2 deletions src/cascadia/TerminalSettingsModel/JsonUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
return til::u8u16(Detail::GetStringView(json));
}

bool CanConvert(const Json::Value& json)
bool CanConvert(const Json::Value& json) const
{
return json.isString();
}
Expand Down Expand Up @@ -252,7 +252,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils
return til::u16u8(val);
}

bool CanConvert(const Json::Value& json)
bool CanConvert(const Json::Value& json) const
{
// hstring has a specific behavior for null, so it can convert it
return ConversionTrait<std::wstring>::CanConvert(json) || json.isNull();
Expand Down