From dc0ff1a1ad1750de1e6ac4ca32ef29a7b9d26153 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 9 Jul 2019 09:21:59 -0700 Subject: [PATCH 1/8] Added CopyOnSelect as a ControlSetting --- .../TerminalApp/GlobalAppSettings.cpp | 21 ++++++++++++++++++- src/cascadia/TerminalApp/GlobalAppSettings.h | 4 ++++ src/cascadia/TerminalControl/TermControl.cpp | 14 ++++++++++++- .../TerminalSettings/IControlSettings.idl | 2 ++ .../TerminalSettings/TerminalSettings.cpp | 10 +++++++++ .../TerminalSettings/terminalsettings.h | 4 ++++ 6 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/GlobalAppSettings.cpp b/src/cascadia/TerminalApp/GlobalAppSettings.cpp index 4f6a9d8d396..eba5beedca7 100644 --- a/src/cascadia/TerminalApp/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalApp/GlobalAppSettings.cpp @@ -24,6 +24,7 @@ static constexpr std::string_view ShowTitleInTitlebarKey{ "showTerminalTitleInTi static constexpr std::string_view RequestedThemeKey{ "requestedTheme" }; static constexpr std::string_view ShowTabsInTitlebarKey{ "showTabsInTitlebar" }; static constexpr std::string_view WordDelimitersKey{ "wordDelimiters" }; +static constexpr std::string_view CopyOnSelectKey{ "copyOnSelect" }; static constexpr std::wstring_view LightThemeValue{ L"light" }; static constexpr std::wstring_view DarkThemeValue{ L"dark" }; @@ -39,7 +40,8 @@ GlobalAppSettings::GlobalAppSettings() : _showTitleInTitlebar{ true }, _showTabsInTitlebar{ true }, _requestedTheme{ ElementTheme::Default }, - _wordDelimiters{ DEFAULT_WORD_DELIMITERS } + _wordDelimiters{ DEFAULT_WORD_DELIMITERS }, + _copyOnSelect{ false } { } @@ -117,6 +119,16 @@ void GlobalAppSettings::SetWordDelimiters(const std::wstring wordDelimiters) noe _wordDelimiters = wordDelimiters; } +bool GlobalAppSettings::GetCopyOnSelect() const noexcept +{ + return _copyOnSelect; +} + +void GlobalAppSettings::SetCopyOnSelect(const bool copyOnSelect) noexcept +{ + _copyOnSelect = copyOnSelect; +} + #pragma region ExperimentalSettings bool GlobalAppSettings::GetShowTabsInTitlebar() const noexcept { @@ -141,6 +153,7 @@ void GlobalAppSettings::ApplyToSettings(TerminalSettings& settings) const noexce settings.InitialRows(_initialRows); settings.InitialCols(_initialCols); settings.WordDelimiters(_wordDelimiters); + settings.CopyOnSelect(_copyOnSelect); } // Method Description: @@ -160,6 +173,7 @@ Json::Value GlobalAppSettings::ToJson() const jsonObject[JsonKey(ShowTitleInTitlebarKey)] = _showTitleInTitlebar; jsonObject[JsonKey(ShowTabsInTitlebarKey)] = _showTabsInTitlebar; jsonObject[JsonKey(WordDelimitersKey)] = winrt::to_string(_wordDelimiters); + jsonObject[JsonKey(CopyOnSelectKey)] = _copyOnSelect; jsonObject[JsonKey(RequestedThemeKey)] = winrt::to_string(_SerializeTheme(_requestedTheme)); jsonObject[JsonKey(KeybindingsKey)] = AppKeyBindingsSerialization::ToJson(_keybindings); @@ -210,6 +224,11 @@ GlobalAppSettings GlobalAppSettings::FromJson(const Json::Value& json) result._wordDelimiters = GetWstringFromJson(wordDelimiters); } + if (auto copyOnSelect{ json[JsonKey(CopyOnSelectKey)] }) + { + result._copyOnSelect = copyOnSelect.asBool(); + } + if (auto requestedTheme{ json[JsonKey(RequestedThemeKey)] }) { result._requestedTheme = _ParseTheme(GetWstringFromJson(requestedTheme)); diff --git a/src/cascadia/TerminalApp/GlobalAppSettings.h b/src/cascadia/TerminalApp/GlobalAppSettings.h index 293ba743233..11dab24ccaa 100644 --- a/src/cascadia/TerminalApp/GlobalAppSettings.h +++ b/src/cascadia/TerminalApp/GlobalAppSettings.h @@ -50,6 +50,9 @@ class TerminalApp::GlobalAppSettings final std::wstring GetWordDelimiters() const noexcept; void SetWordDelimiters(const std::wstring wordDelimiters) noexcept; + bool GetCopyOnSelect() const noexcept; + void SetCopyOnSelect(const bool copyOnSelect) noexcept; + winrt::Windows::UI::Xaml::ElementTheme GetRequestedTheme() const noexcept; Json::Value ToJson() const; @@ -72,6 +75,7 @@ class TerminalApp::GlobalAppSettings final bool _showTabsInTitlebar; std::wstring _wordDelimiters; + bool _copyOnSelect; winrt::Windows::UI::Xaml::ElementTheme _requestedTheme; static winrt::Windows::UI::Xaml::ElementTheme _ParseTheme(const std::wstring& themeString) noexcept; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 14e3b776c40..8344a9214c0 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -843,7 +843,19 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto ptr = args.Pointer(); - if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Touch) + if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse) + { + const auto modifiers = static_cast(args.KeyModifiers()); + // static_cast to a uint32_t because we can't use the WI_IsFlagSet + // macro directly with a VirtualKeyModifiers + const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast(VirtualKeyModifiers::Shift)); + + if (_settings.CopyOnSelect() && _terminal->IsSelectionActive()) + { + CopySelectionToClipboard(!shiftEnabled); + } + } + else if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Touch) { _touchAnchor = std::nullopt; } diff --git a/src/cascadia/TerminalSettings/IControlSettings.idl b/src/cascadia/TerminalSettings/IControlSettings.idl index e706f82b183..caf6beff8d6 100644 --- a/src/cascadia/TerminalSettings/IControlSettings.idl +++ b/src/cascadia/TerminalSettings/IControlSettings.idl @@ -39,5 +39,7 @@ namespace Microsoft.Terminal.Settings Windows.UI.Xaml.Media.Stretch BackgroundImageStretchMode; Windows.UI.Xaml.HorizontalAlignment BackgroundImageHorizontalAlignment; Windows.UI.Xaml.VerticalAlignment BackgroundImageVerticalAlignment; + + Boolean CopyOnSelect; }; } diff --git a/src/cascadia/TerminalSettings/TerminalSettings.cpp b/src/cascadia/TerminalSettings/TerminalSettings.cpp index 3ac52e87320..cb7a36243ff 100644 --- a/src/cascadia/TerminalSettings/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettings/TerminalSettings.cpp @@ -258,6 +258,16 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _backgroundImageVerticalAlignment = value; } + bool TerminalSettings::CopyOnSelect() + { + return _copyOnSelect; + } + + void TerminalSettings::CopyOnSelect(bool value) + { + _copyOnSelect = value; + } + Settings::IKeyBindings TerminalSettings::KeyBindings() { return _keyBindings; diff --git a/src/cascadia/TerminalSettings/terminalsettings.h b/src/cascadia/TerminalSettings/terminalsettings.h index 39894c6674f..468a75779ea 100644 --- a/src/cascadia/TerminalSettings/terminalsettings.h +++ b/src/cascadia/TerminalSettings/terminalsettings.h @@ -74,6 +74,9 @@ namespace winrt::Microsoft::Terminal::Settings::implementation winrt::Windows::UI::Xaml::VerticalAlignment BackgroundImageVerticalAlignment(); void BackgroundImageVerticalAlignment(winrt::Windows::UI::Xaml::VerticalAlignment value); + bool CopyOnSelect(); + void CopyOnSelect(bool value); + winrt::Microsoft::Terminal::Settings::IKeyBindings KeyBindings(); void KeyBindings(winrt::Microsoft::Terminal::Settings::IKeyBindings const& value); @@ -113,6 +116,7 @@ namespace winrt::Microsoft::Terminal::Settings::implementation winrt::Windows::UI::Xaml::Media::Stretch _backgroundImageStretchMode; winrt::Windows::UI::Xaml::HorizontalAlignment _backgroundImageHorizontalAlignment; winrt::Windows::UI::Xaml::VerticalAlignment _backgroundImageVerticalAlignment; + bool _copyOnSelect; hstring _commandline; hstring _startingDir; hstring _envVars; From 1d8b88bf5fbb2df7e56c2eb7b660009a01081bd3 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 30 Jul 2019 10:06:36 -0700 Subject: [PATCH 2/8] Updated doc --- doc/cascadia/SettingsSchema.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/cascadia/SettingsSchema.md b/doc/cascadia/SettingsSchema.md index 4ee2aa9cd15..adf0cafceff 100644 --- a/doc/cascadia/SettingsSchema.md +++ b/doc/cascadia/SettingsSchema.md @@ -6,6 +6,7 @@ Properties listed below affect the entire window, regardless of the profile sett | Property | Necessity | Type | Default | Description | | -------- | --------- | ---- | ------- | ----------- | | `alwaysShowTabs` | _Required_ | Boolean | `true` | When set to `true`, tabs are always displayed. When set to `false` and `showTabsInTitlebar` is set to `false`, tabs only appear after typing Ctrl + T. | +| `copyOnSelect` | Optional | Boolean | `false` | When set to `true`, a selection is immediately copied to yor clipboard upon creation. When set to `false`, the selection persists and awaits further action. | | `defaultProfile` | _Required_ | String | PowerShell guid | Sets the default profile. Opens by typing Ctrl + T or by clicking the '+' icon. The guid of the desired default profile is used as the value. | | `initialCols` | _Required_ | Integer | `120` | The number of columns displayed in the window upon first load. | | `initialRows` | _Required_ | Integer | `30` | The number of rows displayed in the window upon first load. | From f5d84932a3257f6fe87888888a8f705f9b24cb97 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 30 Jul 2019 10:07:22 -0700 Subject: [PATCH 3/8] Updated doc --- doc/cascadia/SettingsSchema.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/cascadia/SettingsSchema.md b/doc/cascadia/SettingsSchema.md index adf0cafceff..a4bdb891612 100644 --- a/doc/cascadia/SettingsSchema.md +++ b/doc/cascadia/SettingsSchema.md @@ -6,7 +6,7 @@ Properties listed below affect the entire window, regardless of the profile sett | Property | Necessity | Type | Default | Description | | -------- | --------- | ---- | ------- | ----------- | | `alwaysShowTabs` | _Required_ | Boolean | `true` | When set to `true`, tabs are always displayed. When set to `false` and `showTabsInTitlebar` is set to `false`, tabs only appear after typing Ctrl + T. | -| `copyOnSelect` | Optional | Boolean | `false` | When set to `true`, a selection is immediately copied to yor clipboard upon creation. When set to `false`, the selection persists and awaits further action. | +| `copyOnSelect` | Optional | Boolean | `false` | When set to `true`, a selection is immediately copied to your clipboard upon creation. When set to `false`, the selection persists and awaits further action. | | `defaultProfile` | _Required_ | String | PowerShell guid | Sets the default profile. Opens by typing Ctrl + T or by clicking the '+' icon. The guid of the desired default profile is used as the value. | | `initialCols` | _Required_ | Integer | `120` | The number of columns displayed in the window upon first load. | | `initialRows` | _Required_ | Integer | `30` | The number of rows displayed in the window upon first load. | From e5078ad73656bf90aa2def379b83eee7a512b811 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 31 Jul 2019 13:32:48 -0700 Subject: [PATCH 4/8] CopyOnSelect feature changes (like, overall) --- src/cascadia/TerminalControl/TermControl.cpp | 17 +++++++-- src/cascadia/TerminalCore/Terminal.cpp | 4 ++ src/cascadia/TerminalCore/Terminal.hpp | 7 +++- .../TerminalCore/TerminalSelection.cpp | 37 ++++++++++++++++++- 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 8344a9214c0..a5ddecfff6b 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -732,7 +732,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation else if (point.Properties().IsRightButtonPressed()) { // copy selection, if one exists - if (_terminal->IsSelectionActive()) + // copyOnSelect causes right-click to always paste + if (_terminal->IsSelectionActive() && !_settings.CopyOnSelect()) { CopySelectionToClipboard(!shiftEnabled); } @@ -850,9 +851,16 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // macro directly with a VirtualKeyModifiers const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast(VirtualKeyModifiers::Shift)); + // copyOnSelect if there's an active selection... if (_settings.CopyOnSelect() && _terminal->IsSelectionActive()) { - CopySelectionToClipboard(!shiftEnabled); + // copy if not single cell selection... + // or if single cell copy allowed (drag off of single cell then back onto it) + if (!_terminal->IsSingleCellSelection() || + (_terminal->IsSingleCellSelection() && _terminal->IsSingleCellCopyAllowed())) + { + CopySelectionToClipboard(!shiftEnabled); + } } } else if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Touch) @@ -1366,7 +1374,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // extract text from buffer const auto copiedData = _terminal->RetrieveSelectedTextFromBuffer(trimTrailingWhitespace); - _terminal->ClearSelection(); + if (!_settings.CopyOnSelect()) + { + _terminal->ClearSelection(); + } // send data up for clipboard _clipboardCopyHandlers(copiedData); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index c7304b1af3a..41cc2a6b85f 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -45,6 +45,8 @@ Terminal::Terminal() : _snapOnInput{ true }, _boxSelection{ false }, _selectionActive{ false }, + _allowSingleCharSelection{ false }, + _copyOnSelect{ false }, _selectionAnchor{ 0, 0 }, _endSelectionPosition{ 0, 0 } { @@ -135,6 +137,8 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting _wordDelimiters = settings.WordDelimiters(); + _copyOnSelect = settings.as().CopyOnSelect(); + // TODO:MSFT:21327402 - if HistorySize has changed, resize the buffer so we // have a smaller scrollback. We should do this carefully - if the new buffer // size is smaller than where the mutable viewport currently is, we'll want diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index b41a0a9f46c..b81a4304e73 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -133,6 +133,8 @@ class Microsoft::Terminal::Core::Terminal final : #pragma region TextSelection // These methods are defined in TerminalSelection.cpp + const bool IsSingleCellSelection() const noexcept; + const bool IsSingleCellCopyAllowed() const noexcept; const bool IsSelectionActive() const noexcept; void DoubleClickSelection(const COORD position); void TripleClickSelection(const COORD position); @@ -160,14 +162,17 @@ class Microsoft::Terminal::Core::Terminal final : bool _snapOnInput; - // Text Selection +#pragma region Text Selection COORD _selectionAnchor; COORD _endSelectionPosition; bool _boxSelection; bool _selectionActive; + bool _allowSingleCharSelection; + bool _copyOnSelect; SHORT _selectionAnchor_YOffset; SHORT _endSelectionPosition_YOffset; std::wstring _wordDelimiters; +#pragma endregion std::shared_mutex _readWriteLock; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 4d968f8f72f..266add7d16e 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -14,7 +14,14 @@ std::vector Terminal::_GetSelectionRects() const { std::vector selectionArea; - if (!_selectionActive) + // copyOnSelect: don't render selection on single cell + // (unless specifically allowed by highlighting more than one cell then reducing it) + if (_copyOnSelect && !_allowSingleCharSelection && IsSingleCellSelection()) + { + return selectionArea; + } + + if (!IsSelectionActive()) { return selectionArea; } @@ -121,6 +128,24 @@ const SHORT Terminal::_ExpandWideGlyphSelectionRight(const SHORT xPos, const SHO return position.X; } +// Method Description: +// - Checks if selection is on a single cell +// Return Value: +// - bool representing if selection is only a single cell. Used for copyOnSelect +const bool Terminal::IsSingleCellSelection() const noexcept +{ + return (_selectionAnchor == _endSelectionPosition); +} + +// Method Description: +// - Checks if selection on a single cell is allowed for rendering purposes +// Return Value: +// - bool representing if selection is only a single cell. Used for copyOnSelect +const bool Terminal::IsSingleCellCopyAllowed() const noexcept +{ + return _allowSingleCharSelection; +} + // Method Description: // - Checks if selection is active // Return Value: @@ -180,6 +205,10 @@ void Terminal::SetSelectionAnchor(const COORD position) _selectionAnchor_YOffset = gsl::narrow(_ViewStartIndex()); _selectionActive = true; + if (_copyOnSelect) + { + _allowSingleCharSelection = false; + } SetEndSelectionPosition(position); } @@ -197,6 +226,11 @@ void Terminal::SetEndSelectionPosition(const COORD position) // copy value of ViewStartIndex to support scrolling // and update on new buffer output (used in _GetSelectionRects()) _endSelectionPosition_YOffset = gsl::narrow(_ViewStartIndex()); + + if (_copyOnSelect && !IsSingleCellSelection()) + { + _allowSingleCharSelection = true; + } } // Method Description: @@ -213,6 +247,7 @@ void Terminal::SetBoxSelection(const bool isEnabled) noexcept void Terminal::ClearSelection() { _selectionActive = false; + _allowSingleCharSelection = false; _selectionAnchor = { 0, 0 }; _endSelectionPosition = { 0, 0 }; _selectionAnchor_YOffset = 0; From 02c04725b7b1f66c0d2723ba54fd3863249b5782 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 5 Aug 2019 12:23:19 -0700 Subject: [PATCH 5/8] Made CopyOnSelect a CoreSetting CopyOnSelect value accessible through Terminal's IsCopyOnSelectActive --- src/cascadia/TerminalControl/TermControl.cpp | 6 +++--- src/cascadia/TerminalCore/Terminal.cpp | 2 +- src/cascadia/TerminalCore/Terminal.hpp | 1 + .../TerminalCore/TerminalSelection.cpp | 9 ++++++++ .../TerminalSettings/IControlSettings.idl | 2 -- .../TerminalSettings/ICoreSettings.idl | 1 + .../TerminalSettings/TerminalSettings.cpp | 21 ++++++++++--------- .../TerminalSettings/terminalsettings.h | 5 ++--- .../ScreenSizeLimitsTest.cpp | 2 ++ 9 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index a5ddecfff6b..aecd768c9ca 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -733,7 +733,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation { // copy selection, if one exists // copyOnSelect causes right-click to always paste - if (_terminal->IsSelectionActive() && !_settings.CopyOnSelect()) + if (_terminal->IsSelectionActive() && !_terminal->IsCopyOnSelectActive()) { CopySelectionToClipboard(!shiftEnabled); } @@ -852,7 +852,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast(VirtualKeyModifiers::Shift)); // copyOnSelect if there's an active selection... - if (_settings.CopyOnSelect() && _terminal->IsSelectionActive()) + if (_terminal->IsCopyOnSelectActive() && _terminal->IsSelectionActive()) { // copy if not single cell selection... // or if single cell copy allowed (drag off of single cell then back onto it) @@ -1374,7 +1374,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // extract text from buffer const auto copiedData = _terminal->RetrieveSelectedTextFromBuffer(trimTrailingWhitespace); - if (!_settings.CopyOnSelect()) + if (!_terminal->IsCopyOnSelectActive()) { _terminal->ClearSelection(); } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 41cc2a6b85f..9b11c39e342 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -137,7 +137,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting _wordDelimiters = settings.WordDelimiters(); - _copyOnSelect = settings.as().CopyOnSelect(); + _copyOnSelect = settings.CopyOnSelect(); // TODO:MSFT:21327402 - if HistorySize has changed, resize the buffer so we // have a smaller scrollback. We should do this carefully - if the new buffer diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index b81a4304e73..04904b09540 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -136,6 +136,7 @@ class Microsoft::Terminal::Core::Terminal final : const bool IsSingleCellSelection() const noexcept; const bool IsSingleCellCopyAllowed() const noexcept; const bool IsSelectionActive() const noexcept; + const bool IsCopyOnSelectActive() const noexcept; void DoubleClickSelection(const COORD position); void TripleClickSelection(const COORD position); void SetSelectionAnchor(const COORD position); diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 266add7d16e..e06622bed4d 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -155,6 +155,15 @@ const bool Terminal::IsSelectionActive() const noexcept return _selectionActive; } +// Method Description: +// - Checks if the CopyOnSelect setting is active +// Return Value: +// - true if feature is active, false otherwise. +const bool Terminal::IsCopyOnSelectActive() const noexcept +{ + return _copyOnSelect; +} + // Method Description: // - Select the sequence between delimiters defined in Settings // Arguments: diff --git a/src/cascadia/TerminalSettings/IControlSettings.idl b/src/cascadia/TerminalSettings/IControlSettings.idl index caf6beff8d6..e706f82b183 100644 --- a/src/cascadia/TerminalSettings/IControlSettings.idl +++ b/src/cascadia/TerminalSettings/IControlSettings.idl @@ -39,7 +39,5 @@ namespace Microsoft.Terminal.Settings Windows.UI.Xaml.Media.Stretch BackgroundImageStretchMode; Windows.UI.Xaml.HorizontalAlignment BackgroundImageHorizontalAlignment; Windows.UI.Xaml.VerticalAlignment BackgroundImageVerticalAlignment; - - Boolean CopyOnSelect; }; } diff --git a/src/cascadia/TerminalSettings/ICoreSettings.idl b/src/cascadia/TerminalSettings/ICoreSettings.idl index 99e08d9c0aa..b715bd70868 100644 --- a/src/cascadia/TerminalSettings/ICoreSettings.idl +++ b/src/cascadia/TerminalSettings/ICoreSettings.idl @@ -28,6 +28,7 @@ namespace Microsoft.Terminal.Settings CursorStyle CursorShape; UInt32 CursorHeight; String WordDelimiters; + Boolean CopyOnSelect; }; } diff --git a/src/cascadia/TerminalSettings/TerminalSettings.cpp b/src/cascadia/TerminalSettings/TerminalSettings.cpp index cb7a36243ff..8cefc1c8e53 100644 --- a/src/cascadia/TerminalSettings/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettings/TerminalSettings.cpp @@ -21,6 +21,7 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _cursorShape{ CursorStyle::Vintage }, _cursorHeight{ DEFAULT_CURSOR_HEIGHT }, _wordDelimiters{ DEFAULT_WORD_DELIMITERS }, + _copyOnSelect{ false }, _useAcrylic{ false }, _closeOnExit{ true }, _tintOpacity{ 0.5 }, @@ -148,6 +149,16 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _wordDelimiters = value; } + bool TerminalSettings::CopyOnSelect() + { + return _copyOnSelect; + } + + void TerminalSettings::CopyOnSelect(bool value) + { + _copyOnSelect = value; + } + bool TerminalSettings::UseAcrylic() { return _useAcrylic; @@ -258,16 +269,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation _backgroundImageVerticalAlignment = value; } - bool TerminalSettings::CopyOnSelect() - { - return _copyOnSelect; - } - - void TerminalSettings::CopyOnSelect(bool value) - { - _copyOnSelect = value; - } - Settings::IKeyBindings TerminalSettings::KeyBindings() { return _keyBindings; diff --git a/src/cascadia/TerminalSettings/terminalsettings.h b/src/cascadia/TerminalSettings/terminalsettings.h index 468a75779ea..b89263d7604 100644 --- a/src/cascadia/TerminalSettings/terminalsettings.h +++ b/src/cascadia/TerminalSettings/terminalsettings.h @@ -47,6 +47,8 @@ namespace winrt::Microsoft::Terminal::Settings::implementation void CursorHeight(uint32_t value); hstring WordDelimiters(); void WordDelimiters(hstring const& value); + bool CopyOnSelect(); + void CopyOnSelect(bool value); // ------------------------ End of Core Settings ----------------------- bool UseAcrylic(); @@ -74,9 +76,6 @@ namespace winrt::Microsoft::Terminal::Settings::implementation winrt::Windows::UI::Xaml::VerticalAlignment BackgroundImageVerticalAlignment(); void BackgroundImageVerticalAlignment(winrt::Windows::UI::Xaml::VerticalAlignment value); - bool CopyOnSelect(); - void CopyOnSelect(bool value); - winrt::Microsoft::Terminal::Settings::IKeyBindings KeyBindings(); void KeyBindings(winrt::Microsoft::Terminal::Settings::IKeyBindings const& value); diff --git a/src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp b/src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp index bdaa57012b8..51e8fb46c79 100644 --- a/src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp @@ -37,6 +37,7 @@ namespace TerminalCoreUnitTests CursorStyle CursorShape() const noexcept { return CursorStyle::Vintage; } uint32_t CursorHeight() { return 42UL; } winrt::hstring WordDelimiters() { return winrt::to_hstring(DEFAULT_WORD_DELIMITERS.c_str()); } + bool CopyOnSelect() { return false; } // other implemented methods uint32_t GetColorTableEntry(int32_t) const { return 123; } @@ -52,6 +53,7 @@ namespace TerminalCoreUnitTests void CursorShape(CursorStyle const&) noexcept {} void CursorHeight(uint32_t) {} void WordDelimiters(winrt::hstring) {} + void CopyOnSelect(bool) {} // other unimplemented methods void SetColorTableEntry(int32_t /* index */, uint32_t /* value */) {} From 3405d9f6b88f5a784ed6e64d6fa1f4ff770b62ff Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 16 Aug 2019 14:04:19 -0700 Subject: [PATCH 6/8] Refactor a bit. --- src/cascadia/TerminalControl/TermControl.cpp | 33 ++++++++++--------- .../TerminalCore/TerminalSelection.cpp | 21 +++++------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 403a373c263..c847fedbeab 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -773,16 +773,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } else if (point.Properties().IsRightButtonPressed()) { - // copy selection, if one exists // copyOnSelect causes right-click to always paste - if (_terminal->IsSelectionActive() && !_terminal->IsCopyOnSelectActive()) + if (_terminal->IsCopyOnSelectActive() || !_terminal->IsSelectionActive()) { - CopySelectionToClipboard(!shiftEnabled); + PasteTextFromClipboard(); } - // paste selection, otherwise else { - PasteTextFromClipboard(); + CopySelectionToClipboard(!shiftEnabled); } } } @@ -809,7 +807,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Mouse) { - if (_terminal->IsSelectionActive() && point.Properties().IsLeftButtonPressed()) + if (point.Properties().IsLeftButtonPressed()) { const auto cursorPosition = point.Position(); _SetEndSelectionPointAtCursor(cursorPosition); @@ -893,16 +891,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // macro directly with a VirtualKeyModifiers const auto shiftEnabled = WI_IsFlagSet(modifiers, static_cast(VirtualKeyModifiers::Shift)); - // copyOnSelect if there's an active selection... - if (_terminal->IsCopyOnSelectActive() && _terminal->IsSelectionActive()) + if (_terminal->IsCopyOnSelectActive()) { - // copy if not single cell selection... - // or if single cell copy allowed (drag off of single cell then back onto it) - if (!_terminal->IsSingleCellSelection() || - (_terminal->IsSingleCellSelection() && _terminal->IsSingleCellCopyAllowed())) - { - CopySelectionToClipboard(!shiftEnabled); - } + CopySelectionToClipboard(!shiftEnabled); } } else if (ptr.PointerDeviceType() == Windows::Devices::Input::PointerDeviceType::Touch) @@ -1407,12 +1398,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation } // Method Description: - // - get text from buffer and send it to the Windows Clipboard (CascadiaWin32:main.cpp). Also removes rendering of selection. + // - Given a copy-able selection, get the selected text from the buffer and send it to the + // Windows Clipboard (CascadiaWin32:main.cpp). + // - CopyOnSelect does NOT clear the selection // Arguments: // - trimTrailingWhitespace: enable removing any whitespace from copied selection // and get text to appear on separate lines. void TermControl::CopySelectionToClipboard(bool trimTrailingWhitespace) { + // no selection --> nothing to copy + // prevent single cell selection (if not allowed) + if (!_terminal->IsSelectionActive() || + (!_terminal->IsSingleCellCopyAllowed() && _terminal->IsSingleCellSelection())) + { + return; + } + // extract text from buffer const auto copiedData = _terminal->RetrieveSelectedTextFromBuffer(trimTrailingWhitespace); diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 32439b206f6..816284ccc57 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -14,13 +14,6 @@ std::vector Terminal::_GetSelectionRects() const { std::vector selectionArea; - // copyOnSelect: don't render selection on single cell - // (unless specifically allowed by highlighting more than one cell then reducing it) - if (_copyOnSelect && !_allowSingleCharSelection && IsSingleCellSelection()) - { - return selectionArea; - } - if (!IsSelectionActive()) { return selectionArea; @@ -74,7 +67,7 @@ std::vector Terminal::_GetSelectionRects() const if (_multiClickSelectionMode == SelectionExpansionMode::Word) { const auto cellChar = _buffer->GetCellDataAt(selectionAnchorWithOffset)->Chars(); - if (_selectionAnchor == _endSelectionPosition && _isWordDelimiter(cellChar)) + if (IsSingleCellSelection() && _isWordDelimiter(cellChar)) { // only highlight the cell if you double click a delimiter } @@ -173,6 +166,12 @@ const bool Terminal::IsSingleCellCopyAllowed() const noexcept // - bool representing if selection is active. Used to decide copy/paste on right click const bool Terminal::IsSelectionActive() const noexcept { + // A single cell selection is not considered an active selection, + // if it's not allowed + if (!_allowSingleCharSelection && IsSingleCellSelection()) + { + return false; + } return _selectionActive; } @@ -245,10 +244,8 @@ void Terminal::SetSelectionAnchor(const COORD position) _selectionAnchor_YOffset = gsl::narrow(_ViewStartIndex()); _selectionActive = true; - if (_copyOnSelect) - { - _allowSingleCharSelection = false; - } + _allowSingleCharSelection = (_copyOnSelect) ? false : true; + SetEndSelectionPosition(position); _multiClickSelectionMode = SelectionExpansionMode::Cell; From 3871c104b296ffbb335e8898f94bd6bd0aa4f4cd Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 16 Aug 2019 14:48:55 -0700 Subject: [PATCH 7/8] CopyOnSelect Tests --- .../UnitTests_TerminalCore/MockTermSettings.h | 5 +- .../UnitTests_TerminalCore/SelectionTest.cpp | 58 +++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/MockTermSettings.h b/src/cascadia/UnitTests_TerminalCore/MockTermSettings.h index 5284b812d81..eb5e4011a7c 100644 --- a/src/cascadia/UnitTests_TerminalCore/MockTermSettings.h +++ b/src/cascadia/UnitTests_TerminalCore/MockTermSettings.h @@ -32,7 +32,7 @@ namespace TerminalCoreUnitTests CursorStyle CursorShape() const noexcept { return CursorStyle::Vintage; } uint32_t CursorHeight() { return 42UL; } winrt::hstring WordDelimiters() { return winrt::to_hstring(DEFAULT_WORD_DELIMITERS.c_str()); } - bool CopyOnSelect() { return false; } + bool CopyOnSelect() { return _copyOnSelect; } // other implemented methods uint32_t GetColorTableEntry(int32_t) const { return 123; } @@ -48,7 +48,7 @@ namespace TerminalCoreUnitTests void CursorShape(CursorStyle const&) noexcept {} void CursorHeight(uint32_t) {} void WordDelimiters(winrt::hstring) {} - void CopyOnSelect(bool) {} + void CopyOnSelect(bool copyOnSelect) { _copyOnSelect = copyOnSelect; } // other unimplemented methods void SetColorTableEntry(int32_t /* index */, uint32_t /* value */) {} @@ -57,5 +57,6 @@ namespace TerminalCoreUnitTests int32_t _historySize; int32_t _initialRows; int32_t _initialCols; + bool _copyOnSelect{ false }; }; } diff --git a/src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp b/src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp index 01465aa3d3a..530f7b29d04 100644 --- a/src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp @@ -504,5 +504,63 @@ namespace TerminalCoreUnitTests selection = term.GetViewport().ConvertToOrigin(selectionRects.at(1)).ToInclusive(); VERIFY_ARE_EQUAL(selection, SMALL_RECT({ 0, 11, 99, 11 })); } + + TEST_METHOD(CopyOnSelect) + { + Terminal term; + DummyRenderTarget emptyRT; + term.Create({ 100, 100 }, 0, emptyRT); + + // set copyOnSelect for terminal + auto settings = winrt::make(0, 100, 100); + settings.CopyOnSelect(true); + term.UpdateSettings(settings); + + // Simulate click at (x,y) = (5,10) + term.SetSelectionAnchor({ 5, 10 }); + + // Simulate move to (x,y) = (5,10) + // (So, no movement) + term.SetEndSelectionPosition({ 5, 10 }); + + // Case 1: single cell selection not allowed + { + // Simulate renderer calling TriggerSelection and acquiring selection area + auto selectionRects = term.GetSelectionRects(); + + // Validate selection area + VERIFY_ARE_EQUAL(selectionRects.size(), static_cast(0)); + + // single cell selection should not be allowed + // thus, selection is NOT active + VERIFY_IS_FALSE(term.IsSelectionActive()); + } + + // Case 2: move off of single cell + term.SetEndSelectionPosition({ 6, 10 }); + { // Simulate renderer calling TriggerSelection and acquiring selection area + auto selectionRects = term.GetSelectionRects(); + + // Validate selection area + VERIFY_ARE_EQUAL(selectionRects.size(), static_cast(1)); + auto selection = term.GetViewport().ConvertToOrigin(selectionRects.at(0)).ToInclusive(); + VERIFY_ARE_EQUAL(selection, SMALL_RECT({ 5, 10, 6, 10 })); + VERIFY_IS_TRUE(term.IsSelectionActive()); + } + + // Case 3: move back onto single cell (now allowed) + term.SetEndSelectionPosition({ 5, 10 }); + { // Simulate renderer calling TriggerSelection and acquiring selection area + auto selectionRects = term.GetSelectionRects(); + + // Validate selection area + VERIFY_ARE_EQUAL(selectionRects.size(), static_cast(1)); + auto selection = term.GetViewport().ConvertToOrigin(selectionRects.at(0)).ToInclusive(); + VERIFY_ARE_EQUAL(selection, SMALL_RECT({ 5, 10, 5, 10 })); + + // single cell selection should now be allowed + VERIFY_IS_TRUE(term.IsSelectionActive()); + } + } }; } From 7c6302c2ddda1d25c91d241d397cd4aaad4b3bfa Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 19 Aug 2019 15:07:59 -0700 Subject: [PATCH 8/8] PR nits --- src/cascadia/TerminalControl/TermControl.cpp | 5 +---- src/cascadia/TerminalCore/Terminal.hpp | 3 +-- src/cascadia/TerminalCore/TerminalSelection.cpp | 17 ++++------------- 3 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 85b0e575622..3b5c3fdc358 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1407,10 +1407,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation bool TermControl::CopySelectionToClipboard(bool trimTrailingWhitespace) { // no selection --> nothing to copy - // prevent single cell selection (if not allowed) - if (_terminal == nullptr || - !_terminal->IsSelectionActive() || - (!_terminal->IsSingleCellCopyAllowed() && _terminal->IsSingleCellSelection())) + if (_terminal == nullptr || !_terminal->IsSelectionActive()) { return false; } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index abdc71ae902..83082957a10 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -137,8 +137,6 @@ class Microsoft::Terminal::Core::Terminal final : #pragma region TextSelection // These methods are defined in TerminalSelection.cpp - const bool IsSingleCellSelection() const noexcept; - const bool IsSingleCellCopyAllowed() const noexcept; const bool IsSelectionActive() const noexcept; const bool IsCopyOnSelectActive() const noexcept; void DoubleClickSelection(const COORD position); @@ -231,5 +229,6 @@ class Microsoft::Terminal::Core::Terminal final : COORD _ExpandDoubleClickSelectionRight(const COORD position) const; const bool _isWordDelimiter(std::wstring_view cellChar) const; const COORD _ConvertToBufferCell(const COORD viewportPos) const; + const bool _isSingleCellSelection() const noexcept; #pragma endregion }; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 816284ccc57..5814471abd7 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -67,7 +67,7 @@ std::vector Terminal::_GetSelectionRects() const if (_multiClickSelectionMode == SelectionExpansionMode::Word) { const auto cellChar = _buffer->GetCellDataAt(selectionAnchorWithOffset)->Chars(); - if (IsSingleCellSelection() && _isWordDelimiter(cellChar)) + if (_isSingleCellSelection() && _isWordDelimiter(cellChar)) { // only highlight the cell if you double click a delimiter } @@ -146,20 +146,11 @@ const SHORT Terminal::_ExpandWideGlyphSelectionRight(const SHORT xPos, const SHO // - Checks if selection is on a single cell // Return Value: // - bool representing if selection is only a single cell. Used for copyOnSelect -const bool Terminal::IsSingleCellSelection() const noexcept +const bool Terminal::_isSingleCellSelection() const noexcept { return (_selectionAnchor == _endSelectionPosition); } -// Method Description: -// - Checks if selection on a single cell is allowed for rendering purposes -// Return Value: -// - bool representing if selection is only a single cell. Used for copyOnSelect -const bool Terminal::IsSingleCellCopyAllowed() const noexcept -{ - return _allowSingleCharSelection; -} - // Method Description: // - Checks if selection is active // Return Value: @@ -168,7 +159,7 @@ const bool Terminal::IsSelectionActive() const noexcept { // A single cell selection is not considered an active selection, // if it's not allowed - if (!_allowSingleCharSelection && IsSingleCellSelection()) + if (!_allowSingleCharSelection && _isSingleCellSelection()) { return false; } @@ -266,7 +257,7 @@ void Terminal::SetEndSelectionPosition(const COORD position) // and update on new buffer output (used in _GetSelectionRects()) _endSelectionPosition_YOffset = gsl::narrow(_ViewStartIndex()); - if (_copyOnSelect && !IsSingleCellSelection()) + if (_copyOnSelect && !_isSingleCellSelection()) { _allowSingleCharSelection = true; }