From 2bf8391d0cd45578dc8e3f5950c42032c9699ef2 Mon Sep 17 00:00:00 2001
From: Eric Nelson <e82.eric@gmail.com>
Date: Mon, 3 Jun 2024 20:07:08 -0700
Subject: [PATCH] Updates for SaveTask for PR feedback

---
 .../TerminalApp/AppActionHandlers.cpp         | 60 ++++++++++---------
 .../TerminalApp/AppCommandlineArgs.cpp        |  6 +-
 2 files changed, 34 insertions(+), 32 deletions(-)

diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
index 275b475c269..85c04399219 100644
--- 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
index 9477cac6c4e..fea050c1f92 100644
--- 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())