Skip to content

Commit

Permalink
Avoid generating extra formatted copies when no action specific `copy…
Browse files Browse the repository at this point in the history
…Formatting` is set (#16480)

Avoid generating extra formatted copies when action's `copyFormatting`
is not present and globally set `copyFormatting` is used.

Previously, when the action's `copyFormatting` wasn't set we deferred
the decision of which formats needed to be copied to the
`TerminalPage::CopyToClipboard` handler. This meant we needed to copy
the text in all the available formats and pass it to the handler to copy
the required formats after querying the global `copyFormatting`.

To avoid making extra copies, we'll store the global `copyFormatting` in
TerminalSettings and pass it down to `TermControl`. If
`ControlCore::CopySelectionToClipboard()` doesn't receive action
specific `copyFormatting`, it will fall back to the global one _before
generating the texts_.

## Validation Steps Performed

- no `copyFormatting` set for the copy action: Copies formats according
to the global `copyFormatting`.
- `copyFormatting` is set for the copy action: Copies formats according
to the action's `copyFormatting`.
  • Loading branch information
tusharsnx authored Jan 24, 2024
1 parent 521a300 commit da182e6
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 9 deletions.
9 changes: 3 additions & 6 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2593,12 +2593,9 @@ namespace winrt::TerminalApp::implementation
auto dataPack = DataPackage();
dataPack.RequestedOperation(DataPackageOperation::Copy);

// The EventArgs.Formats() is an override for the global setting "copyFormatting"
// iff it is set
auto useGlobal = copiedData.Formats() == nullptr;
auto copyFormats = useGlobal ?
_settings.GlobalSettings().CopyFormatting() :
copiedData.Formats().Value();
const auto copyFormats = copiedData.Formats() != nullptr ?
copiedData.Formats().Value() :
static_cast<CopyFormat>(0);

// copy text to dataPack
dataPack.SetText(copiedData.Text());
Expand Down
10 changes: 7 additions & 3 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return false;
}

// use action's copyFormatting if it's present, else fallback to globally
// set copyFormatting.
const auto copyFormats = formats != nullptr ? formats.Value() : _settings->CopyFormatting();

// extract text from buffer
// RetrieveSelectedTextFromBuffer will lock while it's reading
const auto bufferData = _terminal->RetrieveSelectedTextFromBuffer(singleLine);
Expand All @@ -1267,15 +1271,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// GH#5347 - Don't provide a title for the generated HTML, as many
// web applications will paste the title first, followed by the HTML
// content, which is unexpected.
const auto htmlData = formats == nullptr || WI_IsFlagSet(formats.Value(), CopyFormat::HTML) ?
const auto htmlData = WI_IsFlagSet(copyFormats, CopyFormat::HTML) ?
TextBuffer::GenHTML(bufferData,
_actualFont.GetUnscaledSize().height,
_actualFont.GetFaceName(),
bgColor) :
"";

// convert to RTF format
const auto rtfData = formats == nullptr || WI_IsFlagSet(formats.Value(), CopyFormat::RTF) ?
const auto rtfData = WI_IsFlagSet(copyFormats, CopyFormat::RTF) ?
TextBuffer::GenRTF(bufferData,
_actualFont.GetUnscaledSize().height,
_actualFont.GetFaceName(),
Expand All @@ -1287,7 +1291,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
winrt::make<CopyToClipboardEventArgs>(winrt::hstring{ textData },
winrt::to_hstring(htmlData),
winrt::to_hstring(rtfData),
formats));
copyFormats));
return true;
}

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/IControlSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import "IKeyBindings.idl";
import "IControlAppearance.idl";
import "EventArgs.idl";

namespace Microsoft.Terminal.Control
{
Expand Down Expand Up @@ -48,6 +49,7 @@ namespace Microsoft.Terminal.Control
Microsoft.Terminal.Control.IKeyBindings KeyBindings { get; };

Boolean CopyOnSelect { get; };
Microsoft.Terminal.Control.CopyFormat CopyFormatting { get; };
Boolean FocusFollowMouse { get; };

String Commandline { get; };
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation

_WordDelimiters = globalSettings.WordDelimiters();
_CopyOnSelect = globalSettings.CopyOnSelect();
_CopyFormatting = globalSettings.CopyFormatting();
_FocusFollowMouse = globalSettings.FocusFollowMouse();
_ForceFullRepaintRendering = globalSettings.ForceFullRepaintRendering();
_SoftwareRendering = globalSettings.SoftwareRendering();
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/TerminalSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
INHERITABLE_SETTING(Model::TerminalSettings, uint32_t, CursorHeight, DEFAULT_CURSOR_HEIGHT);
INHERITABLE_SETTING(Model::TerminalSettings, hstring, WordDelimiters, DEFAULT_WORD_DELIMITERS);
INHERITABLE_SETTING(Model::TerminalSettings, bool, CopyOnSelect, false);
INHERITABLE_SETTING(Model::TerminalSettings, Microsoft::Terminal::Control::CopyFormat, CopyFormatting, 0);
INHERITABLE_SETTING(Model::TerminalSettings, bool, FocusFollowMouse, false);
INHERITABLE_SETTING(Model::TerminalSettings, bool, TrimBlockSelection, true);
INHERITABLE_SETTING(Model::TerminalSettings, bool, DetectURLs, true);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/inc/ControlProperties.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,5 @@
X(bool, UseAtlasEngine, true) \
X(bool, UseBackgroundImageForWindow, false) \
X(bool, ShowMarks, false) \
X(winrt::Microsoft::Terminal::Control::CopyFormat, CopyFormatting, 0) \
X(bool, RightClickContextMenu, false)

0 comments on commit da182e6

Please sign in to comment.