Updates for SaveTask for PR feedback

microsoft · Jun 4, 2024 · 2bf8391 · 2bf8391
1 parent 3f15d6e
commit 2bf8391
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 32 deletions.
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -1267,47 +1267,49 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_HandleSaveTask(const IInspectable& /*sender*/,
                                        const ActionEventArgs& args)
     {
-        if (Feature_SaveTask::IsEnabled())
+        if constexpr (!Feature_SaveTask::IsEnabled())
         {
-            if (args)
+            return;
+        }
+
+        if (args)
+        {
+            if (const auto& realArgs = args.ActionArgs().try_as<SaveTaskArgs>())
             {
-                if (const auto& realArgs = args.ActionArgs().try_as<SaveTaskArgs>())
+                if (realArgs.Commandline().empty())
                 {
-                    if (realArgs.Commandline().empty())
+                    if (const auto termControl{ _GetActiveControl() })
                     {
-                        if (const auto termControl{ _GetActiveControl() })
+                        if (termControl.HasSelection())
                         {
-                            if (termControl.HasSelection())
-                            {
-                                const auto selections{ termControl.SelectedText(true) };
-                                const auto selection = std::accumulate(selections.begin(), selections.end(), std::wstring());
-                                realArgs.Commandline(selection);
-                            }
+                            const auto selections{ termControl.SelectedText(true) };
+                            const auto selection = std::accumulate(selections.begin(), selections.end(), std::wstring());
+                            realArgs.Commandline(selection);
                         }
                     }
+                }
 
-                    try
-                    {
-                        winrt::Microsoft::Terminal::Control::KeyChord keyChord = nullptr;
-                        if (!realArgs.KeyChord().empty())
-                        {
-                            keyChord = KeyChordSerialization::FromString(winrt::to_hstring(realArgs.KeyChord()));
-                        }
-                        _settings.GlobalSettings().ActionMap().AddSendInputAction(realArgs.Name(), realArgs.Commandline(), keyChord);
-                        _settings.WriteSettingsToDisk();
-                        ActionSaved(realArgs.Commandline(), realArgs.Name(), realArgs.KeyChord());
-                    }
-                    catch (const winrt::hresult_error& ex)
+                try
+                {
+                    winrt::Microsoft::Terminal::Control::KeyChord keyChord = nullptr;
+                    if (!realArgs.KeyChord().empty())
                     {
-                        auto code = ex.code();
-                        auto message = ex.message();
-                        ActionSaveFailed(message);
-                        args.Handled(true);
-                        return;
+                        keyChord = KeyChordSerialization::FromString(winrt::to_hstring(realArgs.KeyChord()));
                     }
-
+                    _settings.GlobalSettings().ActionMap().AddSendInputAction(realArgs.Name(), realArgs.Commandline(), keyChord);
+                    _settings.WriteSettingsToDisk();
+                    ActionSaved(realArgs.Commandline(), realArgs.Name(), realArgs.KeyChord());
+                }
+                catch (const winrt::hresult_error& ex)
+                {
+                    auto code = ex.code();
+                    auto message = ex.message();
+                    ActionSaveFailed(message);
                     args.Handled(true);
+                    return;
                 }
+
+                args.Handled(true);
             }
         }
     }

diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp
@@ -553,11 +553,11 @@ void AppCommandlineArgs::_buildSaveParser()
         // that `this` will still be safe - this function just lets us know this
         // command was parsed.
         subcommand->callback([&, this]() {
-            // Build the NewTab action from the values we've parsed on the commandline.
+            // Build the action from the values we've parsed on the commandline.
             ActionAndArgs saveAction{};
             saveAction.Action(ShortcutAction::SaveTask);
-            // _getNewTerminalArgs MUST be called before parsing any other options,
-            // as it might clear those options while finding the commandline
+            // First, parse out the commandline in the same way that
+            // _getNewTerminalArgs does it
             SaveTaskArgs args{};
 
             if (!_commandline.empty())