Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added: Copy text without dismissing the selection #15552

Merged
merged 8 commits into from Jul 6, 2023
Merged

Added: Copy text without dismissing the selection #15552

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1320f17
Added: DismissSelection
gonzalo-garcian Jun 14, 2023
721a64f
codeFormat
gonzalo-garcian Jun 14, 2023
68d4b9c
modified: update comments
gonzalo-garcian Jun 17, 2023
830bffe
modified: update comment
gonzalo-garcian Jun 17, 2023
0331c7e
codeFormat
gonzalo-garcian Jun 17, 2023
5e75065
Localization refactor
Jun 30, 2023
851ff24
Merge branch 'main' into copy-dismiss-selection
Jul 1, 2023
dfee27b
undo changes to resw
carlos-zamora Jul 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Open in desktop
Expand Up @@ -512,7 +512,7 @@ namespace winrt::TerminalApp::implementation
{
if (const auto& realArgs = args.ActionArgs().try_as<CopyTextArgs>())
{
const auto handled = _CopyText(realArgs.SingleLine(), realArgs.CopyFormatting());
const auto handled = _CopyText(realArgs.DismissSelection(), realArgs.SingleLine(), realArgs.CopyFormatting());
args.Handled(handled);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Expand Up @@ -2751,11 +2751,11 @@ namespace winrt::TerminalApp::implementation
// - formats: dictate which formats need to be copied
// Return Value:
// - true iff we we able to copy text (if a selection was active)
bool TerminalPage::_CopyText(const bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats)
bool TerminalPage::_CopyText(const bool dismissSelection, const bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats)
{
if (const auto& control{ _GetActiveControl() })
{
return control.CopySelectionToClipboard(singleLine, formats);
return control.CopySelectionToClipboard(dismissSelection, singleLine, formats);
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
View file
Open in desktop
Expand Up @@ -410,7 +410,7 @@ namespace winrt::TerminalApp::implementation
bool _IsUriSupported(const winrt::Windows::Foundation::Uri& parsedUri);

void _ShowCouldNotOpenDialog(winrt::hstring reason, winrt::hstring uri);
bool _CopyText(const bool singleLine, const Windows::Foundation::IReference<Microsoft::Terminal::Control::CopyFormat>& formats);
bool _CopyText(const bool dismissSelection, const bool singleLine, const Windows::Foundation::IReference<Microsoft::Terminal::Control::CopyFormat>& formats);

winrt::fire_and_forget _SetTaskbarProgressHandler(const IInspectable sender, const IInspectable eventArgs);

Expand Down
9 changes: 7 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
View file
Open in desktop
Expand Up @@ -2069,15 +2069,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - singleLine: collapse all of the text to one line
// - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr
// if we should defer which formats are copied to the global setting
bool TermControl::CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats)
bool TermControl::CopySelectionToClipboard(bool dismissSelection, bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats)
{
if (_IsClosing())
{
return false;
}

const auto successfulCopy = _interactivity.CopySelectionToClipboard(singleLine, formats);
_core.ClearSelection();

if (dismissSelection)
{
_core.ClearSelection();
}

return successfulCopy;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.h
View file
Open in desktop
Expand Up @@ -39,7 +39,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

hstring GetProfileName() const;

bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats);
bool CopySelectionToClipboard(bool dismissSelection, bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats);
void PasteTextFromClipboard();
void SelectAll();
bool ToggleBlockSelection();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/TermControl.idl
View file
Open in desktop
Expand Up @@ -67,7 +67,7 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, Object> CloseTerminalRequested;
event Windows.Foundation.TypedEventHandler<Object, Object> RestartTerminalRequested;

Boolean CopySelectionToClipboard(Boolean singleLine, Windows.Foundation.IReference<CopyFormat> formats);
Boolean CopySelectionToClipboard(Boolean dismissSelection, Boolean singleLine, Windows.Foundation.IReference<CopyFormat> formats);
void PasteTextFromClipboard();
void SelectAll();
Boolean ToggleBlockSelection();
Expand Down
9 changes: 9 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.cpp
View file
Open in desktop
Expand Up @@ -187,6 +187,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
std::wstringstream ss;

if (DismissSelection())
{
ss << RS_(L"DismissSelectionCommandKey").c_str();
}
else
{
ss << RS_(L"DismissSelectionFalseCommandKey").c_str();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're going to have to move this somewhere else in this method. So, here's how this function works:

  • if singleLine: true --> the command palette displays the action as "Copy text as a single line"
  • otherwise --> "Copy text"
  • if copyFormatting is set, we append "copyFormatting: "

Honestly, the easiest approach I recommend is appending "dismissSelection: false" to the string if dismissSelection: false.

@DHowett @nguyen-dows Thoughts? Y'all are generally good at wordsmithing stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The truth is that I have programmed it a bit by intuition. Where do these strings appear/be used for? I haven't had much time to investigate. I guess I'll wait for others to comment on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! haha GenerateName() spits out the localized string version of the action. These get displayed in the command palette and in the actions page of the settings UI.

if (SingleLine())
{
ss << RS_(L"CopyTextAsSingleLineCommandKey").c_str();
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalSettingsModel/ActionArgs.h
View file
Open in desktop
Expand Up @@ -99,8 +99,9 @@ private: \
// false, if we don't really care if the parameter is required or not.

////////////////////////////////////////////////////////////////////////////////
#define COPY_TEXT_ARGS(X) \
X(bool, SingleLine, "singleLine", false, false) \
#define COPY_TEXT_ARGS(X) \
X(bool, DismissSelection, "dismissSelection", false, false) \
gonzalo-garcian marked this conversation as resolved.
Show resolved Hide resolved
X(bool, SingleLine, "singleLine", false, false) \
X(Windows::Foundation::IReference<Control::CopyFormat>, CopyFormatting, "copyFormatting", false, nullptr)

////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.idl
View file
Open in desktop
Expand Up @@ -155,6 +155,7 @@ namespace Microsoft.Terminal.Settings.Model
[default_interface] runtimeclass CopyTextArgs : IActionArgs
{
CopyTextArgs();
Boolean DismissSelection { get; };
Boolean SingleLine { get; };
Windows.Foundation.IReference<Microsoft.Terminal.Control.CopyFormat> CopyFormatting { get; };
};
Expand Down
10 changes: 9 additions & 1 deletion src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw
View file
Open in desktop
Expand Up @@ -699,4 +699,12 @@
<data name="SelectCommandPreviousCommandKey" xml:space="preserve">
<value>Select previous command</value>
</data>
</root>
<data name="DismissSelectionCommandKey" xml:space="preserve">
<value>DismissSelectionCommandKey</value>
<comment>DismissSelectionCommandKey</comment>
</data>
<data name="DismissSelectionFalseCommandKey" xml:space="preserve">
<value>DismissSelectionFalseCommandKey</value>
<comment>DismissSelectionFalseCommandKey</comment>
</data>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building off of my comments in ActionArgs.cpp.

If you go with my suggestion of dismissSelection: false, you can delete these changes/keys.
If we end up going with an approach that'll be translated (i.e. how the CopyText key is translated to "Copy text"), you'll want to keep these keys, but replace the content of <value> with the translated string.

</root>
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved