Skip to content

Prevent copying an empty line by accident#59193

Merged
rebornix merged 3 commits intomicrosoft:masterfrom
usernamehw:copy_empty
Nov 12, 2018
Merged

Prevent copying an empty line by accident#59193
rebornix merged 3 commits intomicrosoft:masterfrom
usernamehw:copy_empty

Conversation

@usernamehw
Copy link
Contributor

Fixes #55243

@usernamehw usernamehw changed the title Prevent copying an empty string by accident Prevent copying an empty line by accident Sep 23, 2018
@rebornix rebornix added this to the October 2018 milestone Sep 26, 2018
return;
}
// prevent copying an empty string by accident
if (editor.getSelections().length === 1 && editor.getSelection().isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

You may want to compare between editor.getModel().getLineFirstNonWhitespaceColumn and editor.getModel().getLineMaxColumn instead of doing a substring.

Copy link
Member

@rebornix rebornix left a comment

Choose a reason for hiding this comment

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

The approach looks good to me. The only comment I have is doing less costly comparison.

@usernamehw
Copy link
Contributor Author

cc @rebornix

@joaomoreno joaomoreno modified the milestones: October 2018, November 2018 Nov 8, 2018
Copy link
Member

@rebornix rebornix left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@rebornix rebornix merged commit 0a1eb06 into microsoft:master Nov 12, 2018
@usernamehw usernamehw deleted the copy_empty branch November 12, 2018 20:15
@dkelosky
Copy link

dkelosky commented Apr 1, 2019

Is this setting configurable? I preferred the old behavior, copy whatever line I press 'Ctrl +c' on regardless of its content.

This PR assumes a user didn't want to copy a blank line which I usually do.

It seems the real fix is to copy whatever line the user used the keyboard shortcut on and recommend clipboard managers if you copy something that you didn't intend.

@dkelosky
Copy link

dkelosky commented Apr 1, 2019

FYI, I opened #69903 for this. Ctrl + C (or similar keyboard mappings on other editors like notepad++ or even online editors - like firebase rules editor) make no assumption on intention. Ctrl + C just copies the line.

I'm not opposed to inference, but in this case, it seems like it should be configurable and not assumed that copy of a blank line was in error. 😄

@zhanxiaoge
Copy link

I do not think this is a mistake, on the contrary I think he can improve my efficiency, please return to the original.

PKRoma pushed a commit to PKRoma/vscode that referenced this pull request Jul 25, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ctrl + C on blank lines

5 participants