Don't copy if there's no selection

src/cascadia/TerminalApp/ActionArgs.h

@@ -15,19 +15,10 @@
 #include "../../cascadia/inc/cppwinrt_utils.h"
 
 // Notes on defining ActionArgs and ActionEventArgs:
-// * All properties specific to an action should be defined as in the ActionArgs
-//   class.
-// * ActionEventArgs should extend from the ActionArgs class, and add an
-//   aditional `Handled` property. For EventArgs that require actual args (ex:
-//   NewTabWithProfileEventArgs)
-// * For ActionEventArgs:
-//   - the ctor is internal to the TerminalApp implementation. It uses the
-//     implementation type of the IActionArgs. This lets us construct the args
-//     without getting the activation factory.
-//   - You need to manually set each member of the EventArgs using the Args
-//     that's passed in. DO NOT declare the properties a second time in the
-//     event! Because the EventArgs class uses the Args class as a base class,
-//     the EventArgs already has methods for each property defined.
+// * All properties specific to an action should be defined as an ActionArgs
+//   class that implements IActionArgs
+// * ActionEventArgs holds a single IActionArgs. For events that don't need
+//   additional args, this can be nullptr.
 
 namespace winrt::TerminalApp::implementation
 {

src/cascadia/TerminalApp/ActionArgs.idl

@@ -10,7 +10,7 @@ namespace TerminalApp
     interface IActionEventArgs
     {
         Boolean Handled;
-        IActionArgs ActionArgs;
+        IActionArgs ActionArgs { get; };
     };
 
     enum Direction
@@ -28,27 +28,27 @@ namespace TerminalApp
 
     [default_interface] runtimeclass CopyTextArgs : IActionArgs
     {
-        Boolean TrimWhitespace;
+        Boolean TrimWhitespace { get; };
     };
 
     [default_interface] runtimeclass NewTabWithProfileArgs : IActionArgs
     {
-        Int32 ProfileIndex;
+        Int32 ProfileIndex { get; };
     };
 
     [default_interface] runtimeclass SwitchToTabArgs : IActionArgs
     {
-        Int32 TabIndex;
+        Int32 TabIndex { get; };
     };
 
     [default_interface] runtimeclass ResizePaneArgs : IActionArgs
     {
-        Direction Direction;
+        Direction Direction { get; };
     };
 
     [default_interface] runtimeclass MoveFocusArgs : IActionArgs
     {
-        Direction Direction;
+        Direction Direction { get; };
     };
 
 }

src/cascadia/TerminalApp/App.cpp

@@ -1045,12 +1045,7 @@ namespace winrt::TerminalApp::implementation
     bool App::_CopyText(const bool trimTrailingWhitespace)
     {
         const auto control = _GetFocusedControl();
-        if (control.HasSelection())
-        {
-            control.CopySelectionToClipboard(trimTrailingWhitespace);
-            return true;
-        }
-        return false;
+        return control.CopySelectionToClipboard(trimTrailingWhitespace);
     }
 
     // Method Description:
@@ -1075,13 +1070,18 @@ namespace winrt::TerminalApp::implementation
     }
 
     // Method Description:
-    // - Sets focus to the desired tab.
-    void App::_SelectTab(const int tabIndex)
+    // - Sets focus to the desired tab. Returns false if the provided tabIndex
+    //   is greater than the number of tabs we have.
+    // Return Value:
+    // true iff we were able to select that tab index, false otherwise
+    bool App::_SelectTab(const int tabIndex)
     {
         if (tabIndex >= 0 && tabIndex < gsl::narrow_cast<decltype(tabIndex)>(_tabs.size()))
         {
             _SetFocusedTabIndex(tabIndex);
+            return true;
         }
+        return false;
     }
 
     // Method Description:
@@ -1427,153 +1427,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    void App::_HandleNewTab(const IInspectable& /*sender*/,
-                            const TerminalApp::ActionEventArgs& args)
-    {
-        _OpenNewTab(std::nullopt);
-        args.Handled(true);
-    }
-
-    void App::_HandleDuplicateTab(const IInspectable& /*sender*/,
-                                  const TerminalApp::ActionEventArgs& args)
-    {
-        _DuplicateTabViewItem();
-        args.Handled(true);
-    }
-
-    void App::_HandleCloseTab(const IInspectable& /*sender*/,
-                              const TerminalApp::ActionEventArgs& args)
-    {
-        _CloseFocusedTab();
-        args.Handled(true);
-    }
-
-    void App::_HandleClosePane(const IInspectable& /*sender*/,
-                               const TerminalApp::ActionEventArgs& args)
-    {
-        _CloseFocusedPane();
-        args.Handled(true);
-    }
-
-    void App::_HandleScrollUp(const IInspectable& /*sender*/,
-                              const TerminalApp::ActionEventArgs& args)
-    {
-        _Scroll(-1);
-        args.Handled(true);
-    }
-
-    void App::_HandleScrollDown(const IInspectable& /*sender*/,
-                                const TerminalApp::ActionEventArgs& args)
-    {
-        _Scroll(1);
-        args.Handled(true);
-    }
-
-    void App::_HandleNextTab(const IInspectable& /*sender*/,
-                             const TerminalApp::ActionEventArgs& args)
-    {
-        _SelectNextTab(true);
-        args.Handled(true);
-    }
-
-    void App::_HandlePrevTab(const IInspectable& /*sender*/,
-                             const TerminalApp::ActionEventArgs& args)
-    {
-        _SelectNextTab(false);
-        args.Handled(true);
-    }
-
-    void App::_HandleSplitVertical(const IInspectable& /*sender*/,
-                                   const TerminalApp::ActionEventArgs& args)
-    {
-        _SplitVertical(std::nullopt);
-        args.Handled(true);
-    }
-
-    void App::_HandleSplitHorizontal(const IInspectable& /*sender*/,
-                                     const TerminalApp::ActionEventArgs& args)
-    {
-        _SplitHorizontal(std::nullopt);
-        args.Handled(true);
-    }
-
-    void App::_HandleScrollUpPage(const IInspectable& /*sender*/,
-                                  const TerminalApp::ActionEventArgs& args)
-    {
-        _ScrollPage(-1);
-        args.Handled(true);
-    }
-
-    void App::_HandleScrollDownPage(const IInspectable& /*sender*/,
-                                    const TerminalApp::ActionEventArgs& args)
-    {
-        _ScrollPage(1);
-        args.Handled(true);
-    }
-
-    void App::_HandleOpenSettings(const IInspectable& /*sender*/,
-                                  const TerminalApp::ActionEventArgs& args)
-    {
-        _OpenSettings();
-        args.Handled(true);
-    }
-
-    void App::_HandlePasteText(const IInspectable& /*sender*/,
-                               const TerminalApp::ActionEventArgs& args)
-    {
-        _PasteText();
-        args.Handled(true);
-    }
-
-    void App::_HandleNewTabWithProfile(const IInspectable& /*sender*/,
-                                       const TerminalApp::ActionEventArgs& args)
-    {
-        if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::NewTabWithProfileArgs>())
-        {
-            _OpenNewTab({ realArgs.ProfileIndex() });
-            args.Handled(true);
-        }
-    }
-
-    void App::_HandleSwitchToTab(const IInspectable& /*sender*/,
-                                 const TerminalApp::ActionEventArgs& args)
-    {
-        if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::SwitchToTabArgs>())
-        {
-            _SelectTab({ realArgs.TabIndex() });
-            args.Handled(true);
-        }
-    }
-
-    void App::_HandleResizePane(const IInspectable& /*sender*/,
-                                const TerminalApp::ActionEventArgs& args)
-    {
-        if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::ResizePaneArgs>())
-        {
-            _ResizePane(realArgs.Direction());
-            args.Handled(true);
-        }
-    }
-
-    void App::_HandleMoveFocus(const IInspectable& /*sender*/,
-                               const TerminalApp::ActionEventArgs& args)
-    {
-        if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::MoveFocusArgs>())
-        {
-            _MoveFocus(realArgs.Direction());
-            args.Handled(true);
-        }
-    }
-
-    void App::_HandleCopyText(const IInspectable& /*sender*/,
-                              const TerminalApp::ActionEventArgs& args)
-    {
-        if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::CopyTextArgs>())
-        {
-            args.Handled(_CopyText(realArgs.TrimWhitespace()));
-        }
-    }
-
     // Method Description:
     // - Creates a new connection based on the profile settings
     // Arguments:

src/cascadia/TerminalApp/App.h

@@ -108,7 +108,7 @@ namespace winrt::TerminalApp::implementation
         void _CloseFocusedTab();
         void _CloseFocusedPane();
         void _SelectNextTab(const bool bMoveRight);
-        void _SelectTab(const int tabIndex);
+        bool _SelectTab(const int tabIndex);
 
         void _SetFocusedTabIndex(int tabIndex);
         int _GetFocusedTabIndex() const;
@@ -146,6 +146,8 @@ namespace winrt::TerminalApp::implementation
 
         static void _SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::Settings::KeyChord& keyChord);
 
+#pragma region ActionHandlers
+        // These are all defined in AppActionHandlers.cpp
         void _HandleNewTab(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
         void _HandleDuplicateTab(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
         void _HandleCloseTab(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
@@ -165,6 +167,7 @@ namespace winrt::TerminalApp::implementation
         void _HandleResizePane(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
         void _HandleMoveFocus(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
         void _HandleCopyText(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
+#pragma endregion
     };
 }