From a00a15a230f0a10758e731a6eb7af16c96ed4b21 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 9 Jul 2024 08:28:50 -0500 Subject: [PATCH] Stop parsing `saveSnippets` actions in json In the spec review, we agreed these didn't really need to be saved to the user's own settings file. This removes parsing and saving for the `experimental.saveSnippet` action, but we still have the action _internally_. This is powered by a new x-macro for "INTERNAL_" actions. Follow-up from #16513. --- src/cascadia/TerminalApp/AppCommandlineArgs.cpp | 2 +- src/cascadia/TerminalApp/ShortcutActionDispatch.cpp | 1 + src/cascadia/TerminalApp/ShortcutActionDispatch.h | 1 + src/cascadia/TerminalApp/ShortcutActionDispatch.idl | 1 + src/cascadia/TerminalApp/TerminalPage.cpp | 1 + src/cascadia/TerminalApp/TerminalPage.h | 1 + src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp | 4 +++- src/cascadia/TerminalSettingsModel/ActionMap.cpp | 2 ++ .../TerminalSettingsModel/AllShortcutActions.h | 11 +++++++++-- src/cascadia/TerminalSettingsModel/Command.idl | 1 + 10 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp index c1154f56728..2083aed67ef 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp @@ -540,7 +540,7 @@ void AppCommandlineArgs::_buildFocusPaneParser() void AppCommandlineArgs::_buildSaveSnippetParser() { - _saveCommand = _app.add_subcommand("x-save-snippet", RS_A(L"SaveSnippetDesc")); + _saveCommand = _app.add_subcommand("x-save", RS_A(L"SaveSnippetDesc")); auto setupSubcommand = [this](auto* subcommand) { subcommand->add_option("--name,-n", _saveInputName, RS_A(L"SaveSnippetArgDesc")); diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp b/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp index b44e44976e3..0d8d2e8dbc6 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.cpp @@ -44,6 +44,7 @@ namespace winrt::TerminalApp::implementation { #define ON_ALL_ACTIONS(id) ACTION_CASE(id); ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS default: return false; diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.h b/src/cascadia/TerminalApp/ShortcutActionDispatch.h index d43d7fc2421..ac18b653583 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.h +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.h @@ -27,6 +27,7 @@ namespace winrt::TerminalApp::implementation #define ON_ALL_ACTIONS(action) DECLARE_ACTION(action); ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS private: diff --git a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl index 2798df77ac3..d6dd4ad8137 100644 --- a/src/cascadia/TerminalApp/ShortcutActionDispatch.idl +++ b/src/cascadia/TerminalApp/ShortcutActionDispatch.idl @@ -15,6 +15,7 @@ namespace TerminalApp // When adding a new action, add them to AllShortcutActions.h! #define ON_ALL_ACTIONS(action) ACTION_EVENT(action); ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS } diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 0e3dd5256bc..cfe597e92c6 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1684,6 +1684,7 @@ namespace winrt::TerminalApp::implementation // there's an actual keychord for them. #define ON_ALL_ACTIONS(action) HOOKUP_ACTION(action); ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 07dbf173242..aaa98db46a8 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -560,6 +560,7 @@ namespace winrt::TerminalApp::implementation // These are all defined in AppActionHandlers.cpp #define ON_ALL_ACTIONS(action) DECLARE_ACTION_HANDLER(action); ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS #pragma endregion diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 9b2ef821f58..8fe015b8b15 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -52,7 +52,6 @@ static constexpr std::string_view SwitchToTabKey{ "switchToTab" }; static constexpr std::string_view TabSearchKey{ "tabSearch" }; static constexpr std::string_view ToggleAlwaysOnTopKey{ "toggleAlwaysOnTop" }; static constexpr std::string_view ToggleCommandPaletteKey{ "commandPalette" }; -static constexpr std::string_view SaveSnippetKey{ "experimental.saveSnippet" }; static constexpr std::string_view SuggestionsKey{ "showSuggestions" }; static constexpr std::string_view ToggleFocusModeKey{ "toggleFocusMode" }; static constexpr std::string_view SetFocusModeKey{ "setFocusMode" }; @@ -124,12 +123,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation const std::map> ActionAndArgs::ActionKeyNamesMap{ #define ON_ALL_ACTIONS(action) KEY_TO_ACTION_PAIR(action) ALL_SHORTCUT_ACTIONS + // Don't include the INTERNAL_SHORTCUT_ACTIONS here #undef ON_ALL_ACTIONS }; static const std::map> ActionToStringMap{ #define ON_ALL_ACTIONS(action) ACTION_TO_KEY_PAIR(action) ALL_SHORTCUT_ACTIONS + // Don't include the INTERNAL_SHORTCUT_ACTIONS here #undef ON_ALL_ACTIONS }; @@ -152,6 +153,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation #define ON_ALL_ACTIONS_WITH_ARGS(action) ACTION_TO_SERIALIZERS_PAIR(action) ALL_SHORTCUT_ACTIONS_WITH_ARGS + // Don't include the INTERNAL_SHORTCUT_ACTIONS here #undef ON_ALL_ACTIONS_WITH_ARGS }; diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index 86a88c36858..354831073d0 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -47,6 +47,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation break; \ } ALL_SHORTCUT_ACTIONS_WITH_ARGS + INTERNAL_SHORTCUT_ACTIONS_WITH_ARGS #undef ON_ALL_ACTIONS_WITH_ARGS default: break; @@ -192,6 +193,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // now add any ShortcutActions that we might have missed #define ON_ALL_ACTIONS(action) RegisterShortcutAction(ShortcutAction::action, availableActions, visitedActionIDs); ALL_SHORTCUT_ACTIONS + // Don't include internal actions here #undef ON_ALL_ACTIONS _AvailableActionsCache = single_threaded_map(std::move(availableActions)); diff --git a/src/cascadia/TerminalSettingsModel/AllShortcutActions.h b/src/cascadia/TerminalSettingsModel/AllShortcutActions.h index 86ff9d49c9b..2432b40e12c 100644 --- a/src/cascadia/TerminalSettingsModel/AllShortcutActions.h +++ b/src/cascadia/TerminalSettingsModel/AllShortcutActions.h @@ -75,7 +75,6 @@ ON_ALL_ACTIONS(CloseTabsAfter) \ ON_ALL_ACTIONS(TabSearch) \ ON_ALL_ACTIONS(MoveTab) \ - ON_ALL_ACTIONS(SaveSnippet) \ ON_ALL_ACTIONS(BreakIntoDebugger) \ ON_ALL_ACTIONS(TogglePaneReadOnly) \ ON_ALL_ACTIONS(EnablePaneReadOnly) \ @@ -149,7 +148,6 @@ ON_ALL_ACTIONS_WITH_ARGS(SplitPane) \ ON_ALL_ACTIONS_WITH_ARGS(SwitchToTab) \ ON_ALL_ACTIONS_WITH_ARGS(ToggleCommandPalette) \ - ON_ALL_ACTIONS_WITH_ARGS(SaveSnippet) \ ON_ALL_ACTIONS_WITH_ARGS(FocusPane) \ ON_ALL_ACTIONS_WITH_ARGS(ExportBuffer) \ ON_ALL_ACTIONS_WITH_ARGS(ClearBuffer) \ @@ -159,3 +157,12 @@ ON_ALL_ACTIONS_WITH_ARGS(SelectCommand) \ ON_ALL_ACTIONS_WITH_ARGS(SelectOutput) \ ON_ALL_ACTIONS_WITH_ARGS(ColorSelection) + +// These two macros here are for actions that we only use as internal currency. +// They don't need to be parsed by the settings model, or saved as actions to +// JSON. +#define INTERNAL_SHORTCUT_ACTIONS \ + ON_ALL_ACTIONS(SaveSnippet) + +#define INTERNAL_SHORTCUT_ACTIONS_WITH_ARGS \ + ON_ALL_ACTIONS_WITH_ARGS(SaveSnippet)\ diff --git a/src/cascadia/TerminalSettingsModel/Command.idl b/src/cascadia/TerminalSettingsModel/Command.idl index a3a316e1a2e..a364eabb4dd 100644 --- a/src/cascadia/TerminalSettingsModel/Command.idl +++ b/src/cascadia/TerminalSettingsModel/Command.idl @@ -17,6 +17,7 @@ namespace Microsoft.Terminal.Settings.Model // When adding a new action, add them to AllShortcutActions.h! #define ON_ALL_ACTIONS(action) action, ALL_SHORTCUT_ACTIONS + INTERNAL_SHORTCUT_ACTIONS #undef ON_ALL_ACTIONS };