Added CopyOnSelect as a Global Setting #2152

carlos-zamora · 2019-07-30T17:12:41Z

Summary of the Pull Request

Introduces the "copyOnSelect" setting to "globals" as a boolean type. 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.

PR Checklist

Closes Feature: add copy on select (per Unix terminals) #524
CLA signed
Tests ~~added/passed~~ not necessary
Requires documentation to be updated
I am a core contributor

Detailed Description of the Pull Request / Additional comments

Standard "added a new global setting" changes
TermControl's PointerReleased event is updated to support a mouse button release event. Just hooked up the copy function right there.

Validation Steps Performed

copyOnSelect = true
- make selection
- right click to paste (selection persists)
- Works! ✔
copyOnSelect = true
- make selection
- right click to paste
- left click on a cell
- right click to paste
- same data should be pasted twice
- Works! ✔
copyOnSelect = true
- click on a cell (no selection)
- drag to another cell (selection visible)
- drag back to first cell (selection should persist, but should only cover one cell)
- Works! ✔
copyOnSelect = false
- make selection
- selection persists
- right click to copy (selection disappears)
- right click to paste
- Works! ✔

zadjii-msft

mcpiroman · 2019-07-30T18:11:13Z

Should the selection disappear? That's not how it works on other terminals.

Also, if it doesn't, I'd like to have some visual confirming the current selection is in clipboard (if it is), such as selection outline color. But that's another pr/issue, right?

DHowett-MSFT

we need to not allow a click to select a single character
look at how other terminals fix that

I agree with dustin on the whole "we need to not allow a click to select a single character" thing

carlos-zamora · 2019-07-31T16:18:57Z

Should the selection disappear? That's not how it works on other terminals.

Yeah. I originally had it disappear to provide feedback that you copied it. I'm going to change it to:

selection persists after copy
right-click immediately pastes (doesn't do a redundant copy)
click/tap (no drag) should...
- not create a new selection
- ~~@DHowett-MSFT should it erase old selection?~~ erase old selection

carlos-zamora · 2019-07-31T20:55:12Z

Question: should setting be renamed to copyMode = {Windows, Unix}? 👍 or 👎 and put your thoughts below!

zadjii-msft · 2019-07-31T21:01:17Z

I think I personally prefer copyOnSelect. It's much more literally what I want - I wouldn't necessarily know that unix means "copy on select". Is there other possible values of copyMode you're thinking of other than Windows vs Unix?

carlos-zamora · 2019-07-31T21:05:06Z

I think I personally prefer copyOnSelect. It's much more literally what I want - I wouldn't necessarily know that unix means "copy on select". Is there other possible values of copyMode you're thinking of other than Windows vs Unix?

IIRC, MacOS's Terminal has a drop-down on right-click. But that's also weird because there'd be different functionality through that drop down menu like "search with Google", I think. I don't remember what the other drop-down options are.

Edit: here's a snapshot of right-clicking "Dropbox" on MacOS's Terminal

0xabu · 2019-08-01T20:01:06Z

Question: should setting be renamed to copyMode = {Windows, Unix}?

A copyMode of "unix" is pretty ambiguous, IMO it could just as easily refer to line ending conversion as it could to the select/click behaviour. Also, AFAIK (grumpy old man tone) copy-on-select derives more from X and its ancestors than it does Unix.

CopyOnSelect value accessible through Terminal's IsCopyOnSelectActive

carlos-zamora · 2019-08-05T19:45:16Z

Alright, decided on the following model for the settings:

copyOnSelect: bool --> when enabled, this makes it so that...
- when you make a selection, it gets copied
- a single cell selection is not supported (unless you select more than a cell then drag the mouse back to get a single cell)
- right-click always pastes
pauseOnSelect: bool (Feature Request: Pause output or scrolling on click (and make it a setting) #980) --> when enabled, this makes it so that...
- output is paused when a selection is displayed/active
pointer bindings (Feature Request: Mouse/Touch/Pointer Bindings (like middle-click paste, right-click context menu, etc.) #1553) --> we'll have a bunch of actions supported. A few include...
- right-click (as an example) being bound to...
  - paste: always paste
  - smartCopyPaste: copy, if selection active. Paste otherwise.
  - menu: context menu as shown above

The only tricky part I see here is copyOnSelect's 3rd function potentially interfering with pointer bindings. For now, I say we keep it as is. When pointer bindings comes along, we can make it so that smartCopyPaste specifically clears the selection. Thus kinda breaking this up into 2 things (not a big fan but I feel that that's the best way to approach this issue).

Significant changes.

DHowett-MSFT · 2019-08-13T21:59:24Z

src/cascadia/TerminalCore/TerminalSelection.cpp

+// - 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


can this and the above be folded into IsSelectionActive?

this one can be deleted once the TermControl caller goes away

zadjii-msft

I'm of two minds:

I want to ship this feature
I want tests

I'm not approving, but only based on the fact there aren't any tests in a very testable part of the code. The other two can get together and override me though if they want.

src/cascadia/TerminalCore/Terminal.hpp

# Conflicts: # src/cascadia/TerminalCore/Terminal.hpp # src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp

carlos-zamora · 2019-08-16T21:52:30Z

@zadjii-msft CopyOnSelect tests got added. Note that an "active selection" is considered copy-able. Handling a single cell selection is really weird but I thing the refactor and the test should make it clear what kind of behavior is now expected when using this new mode.

zadjii-msft

Thanks for adding the tests C:

carlos-zamora · 2019-08-19T16:45:51Z

@msftbot make sure @DHowett-MSFT signs off

ghost · 2019-08-19T16:45:57Z

Hello @carlos-zamora!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I'll only merge this pull request if it's approved by @DHowett-MSFT

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett-MSFT

Minor nits, but I'd like to see them answered before the bot merges the PR ;)

src/cascadia/TerminalCore/Terminal.hpp

src/cascadia/TerminalControl/TermControl.cpp

DHowett-MSFT · 2019-08-19T18:23:12Z

src/cascadia/TerminalCore/TerminalSelection.cpp

+// - 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


this one can be deleted once the TermControl caller goes away

timriker · 2019-08-20T21:52:30Z

I'm running the version in the store. (0.3.2171.0) With this version, click-drag, double-click and triple-click all select and copy to clipboard. Right click removes selection, but does NOT paste. A second right click DOES paste. This is odd.

On other terminal apps on windows and Linux, middle button is paste and right button is menu. Is there an option to get that functionality?

What does middle button do currently?

I don't see a "CopyOnSelect" option in settings.

DHowett-MSFT · 2019-08-20T22:02:00Z

Our release process can't possibly be that fast.

DHowett-MSFT · 2019-08-20T22:03:08Z

As of 0.3.2171.0: selection just selects, right-click copies and deselects, and then right-click pastes.
As of [next version]: copyOnSelect is added, but the preexisting behavior stays the same if it is set to false.

timriker · 2019-08-20T22:09:42Z

I didn't expect it to be in the store release yet, just being clear that I'm not yet testing the current change. :)

I would like to see more Linux-y feel with left button select modes, middle button paste mode, and right button menu. It's been requested before on other posts. I was just wondering if there's a plan for adding that. Thanks for the updates!

DHowett-MSFT · 2019-08-21T01:03:41Z

Thanks! We haven't yet looked at middle-paste, but we've talked about "mouse bindings" (as a post-1.0 feature)!

ghost · 2019-08-27T16:09:11Z

🎉Windows Terminal Preview v0.4.2382.0 has been released which incorporates this pull request.:tada:

Handy links:

alecive · 2019-09-25T15:39:41Z

Question: I'm using 0.4.2382.0 but I am still having big issues when using WSL with windows terminal. Should I see the copyOnSelect toggle somewhere in the settings? Why don't I see it there?

I am just trying to fix the annoying bug of copying something in my clipboard [from e.g. the notepad or the browser] and then pasting it in my WSL instance within windows terminal. It this PR fixing this issue?

DHowett-MSFT · 2019-09-25T15:42:56Z

fix the annoying bug of copying something

@alecive what you have described is not a bug (???) so we can't tell if this pull request, which adds an option that makes it so text you select gets copied to the clipboard automatically, fixes it.

(fwiw: add copyOnSelect to your profiles.json as a boolean, either in the globals section or not (if you don't have one))

lagunary · 2020-02-05T22:02:05Z

Awesome!!!!

carlos-zamora added 3 commits July 30, 2019 09:59

Added CopyOnSelect as a ControlSetting

dc0ff1a

Updated doc

1d8b88b

Updated doc

f5d8493

carlos-zamora self-assigned this Jul 30, 2019

zadjii-msft previously approved these changes Jul 30, 2019

View reviewed changes

miniksa previously approved these changes Jul 30, 2019

View reviewed changes

DHowett-MSFT previously requested changes Jul 30, 2019

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 30, 2019

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 31, 2019

CopyOnSelect feature changes (like, overall)

e5078ad

carlos-zamora mentioned this pull request Jul 31, 2019

Chunk Selection Expansion for Double/Triple Click Selection #2184

Merged

5 tasks

Made CopyOnSelect a CoreSetting

02c0472

CopyOnSelect value accessible through Terminal's IsCopyOnSelectActive

DHowett-MSFT mentioned this pull request Aug 7, 2019

Copy & Paste Keybindings #968

Closed

DHowett-MSFT reviewed Aug 13, 2019

View reviewed changes

carlos-zamora added the Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) label Aug 13, 2019

Merge branch 'master' into dev/cazamor/copy-on-select

c0ee9b7

zadjii-msft reviewed Aug 14, 2019

View reviewed changes

src/cascadia/TerminalCore/Terminal.hpp Show resolved Hide resolved

Merge branch 'master' into dev/cazamor/copy-on-select

83ecab9

# Conflicts: # src/cascadia/TerminalCore/Terminal.hpp # src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp

This comment has been minimized.

Sign in to view

Refactor a bit.

3405d9f

CopyOnSelect Tests

3871c10

Merge branch 'master' into dev/cazamor/copy-on-select

fb082c9

zadjii-msft approved these changes Aug 19, 2019

View reviewed changes

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 19, 2019

DHowett-MSFT suggested changes Aug 19, 2019

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 19, 2019

carlos-zamora added 2 commits August 19, 2019 15:07

PR nits

7c6302c

resolve merge conflicts

d02a0f4

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 19, 2019

DHowett-MSFT approved these changes Aug 19, 2019

View reviewed changes

Merge branch 'master' into dev/cazamor/copy-on-select

1419601

ghost merged commit ff87190 into master Aug 20, 2019

ghost deleted the dev/cazamor/copy-on-select branch August 20, 2019 16:42

ghost mentioned this pull request Aug 27, 2019

Feature: add copy on select (per Unix terminals) #524

Closed

adamburgess mentioned this pull request Sep 25, 2019

Feature: paste on right click regardless of selection microsoft/vscode#81447

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added CopyOnSelect as a Global Setting #2152

Added CopyOnSelect as a Global Setting #2152

carlos-zamora commented Jul 30, 2019 •

edited

Loading

zadjii-msft left a comment

mcpiroman commented Jul 30, 2019 •

edited

Loading

DHowett-MSFT left a comment

carlos-zamora commented Jul 31, 2019 •

edited

Loading

carlos-zamora commented Jul 31, 2019

zadjii-msft commented Jul 31, 2019

carlos-zamora commented Jul 31, 2019 •

edited

Loading

0xabu commented Aug 1, 2019

carlos-zamora commented Aug 5, 2019

DHowett-MSFT Aug 13, 2019

DHowett-MSFT Aug 19, 2019

zadjii-msft left a comment

This comment has been minimized.

carlos-zamora commented Aug 16, 2019

zadjii-msft left a comment

carlos-zamora commented Aug 19, 2019

ghost commented Aug 19, 2019

DHowett-MSFT left a comment

DHowett-MSFT Aug 19, 2019

timriker commented Aug 20, 2019

DHowett-MSFT commented Aug 20, 2019

DHowett-MSFT commented Aug 20, 2019

timriker commented Aug 20, 2019 •

edited

Loading

DHowett-MSFT commented Aug 21, 2019

ghost commented Aug 27, 2019

alecive commented Sep 25, 2019

DHowett-MSFT commented Sep 25, 2019

lagunary commented Feb 5, 2020

Added CopyOnSelect as a Global Setting #2152

Added CopyOnSelect as a Global Setting #2152

Conversation

carlos-zamora commented Jul 30, 2019 • edited Loading

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

zadjii-msft left a comment

Choose a reason for hiding this comment

mcpiroman commented Jul 30, 2019 • edited Loading

DHowett-MSFT left a comment

Choose a reason for hiding this comment

carlos-zamora commented Jul 31, 2019 • edited Loading

carlos-zamora commented Jul 31, 2019

zadjii-msft commented Jul 31, 2019

carlos-zamora commented Jul 31, 2019 • edited Loading

0xabu commented Aug 1, 2019

carlos-zamora commented Aug 5, 2019

DHowett-MSFT Aug 13, 2019

Choose a reason for hiding this comment

DHowett-MSFT Aug 19, 2019

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

This comment has been minimized.

carlos-zamora commented Aug 16, 2019

zadjii-msft left a comment

Choose a reason for hiding this comment

carlos-zamora commented Aug 19, 2019

ghost commented Aug 19, 2019

DHowett-MSFT left a comment

Choose a reason for hiding this comment

DHowett-MSFT Aug 19, 2019

Choose a reason for hiding this comment

timriker commented Aug 20, 2019

DHowett-MSFT commented Aug 20, 2019

DHowett-MSFT commented Aug 20, 2019

timriker commented Aug 20, 2019 • edited Loading

DHowett-MSFT commented Aug 21, 2019

ghost commented Aug 27, 2019

alecive commented Sep 25, 2019

DHowett-MSFT commented Sep 25, 2019

lagunary commented Feb 5, 2020

carlos-zamora commented Jul 30, 2019 •

edited

Loading

mcpiroman commented Jul 30, 2019 •

edited

Loading

carlos-zamora commented Jul 31, 2019 •

edited

Loading

carlos-zamora commented Jul 31, 2019 •

edited

Loading

timriker commented Aug 20, 2019 •

edited

Loading