Skip to content

fix: change default keybinding of cursorColumnSelectUp to ctrl+shift+…#41471

Merged
alexdima merged 2 commits intomicrosoft:masterfrom
mikaoelitiana:fix-36819
Mar 26, 2018
Merged

fix: change default keybinding of cursorColumnSelectUp to ctrl+shift+…#41471
alexdima merged 2 commits intomicrosoft:masterfrom
mikaoelitiana:fix-36819

Conversation

@mikaoelitiana
Copy link
Contributor

…up on Windows

fixes #36819

@alexdima alexdima added this to the March 2018 milestone Mar 26, 2018
@alexdima alexdima merged commit a6ef8ca into microsoft:master Mar 26, 2018
@alexdima
Copy link
Member

I actually ended up reverting this commit.

I cannot accept it because the column selection keybindings work together:

{ "key": "ctrl+shift+alt+down",   "command": "cursorColumnSelectDown",
                                     "when": "textInputFocus" },
{ "key": "ctrl+shift+alt+left",   "command": "cursorColumnSelectLeft",
                                     "when": "textInputFocus" },
{ "key": "ctrl+shift+alt+pagedown", "command": "cursorColumnSelectPageDown",
                                     "when": "textInputFocus" },
{ "key": "ctrl+shift+alt+pageup", "command": "cursorColumnSelectPageUp",
                                     "when": "textInputFocus" },
{ "key": "ctrl+shift+alt+right",  "command": "cursorColumnSelectRight",
                                     "when": "textInputFocus" },
{ "key": "ctrl+shift+alt+up",     "command": "cursorColumnSelectUp",
                                     "when": "textInputFocus" },

Changing just two of them only creates confusion. And we cannot change all of them because ctrl+shift+left and ctrl+shift+right are used for word selection.

@jhasse
Copy link
Contributor

jhasse commented Mar 26, 2018

What's the difference between cursorColumnSelectLeft and a simple Shift+Left?

@alexdima
Copy link
Member

The latter wraps (goes up) at line starts.

@jhasse
Copy link
Contributor

jhasse commented Mar 26, 2018

Ah I see, quite a minor detail IMHO. Also it doesn't seem to work with multiple cursors for me or is this a bug? If not, I don't see the use-case for the command at all.

I'm using cursorColumnSelectUp/Down to quickly get multiple cursors in a straight line. As I can't even use cursorColumnSelectLeft after that in a meaningful way and they do different things (adding a new cusor vs. expanding the selection), I don't see how they are related (beside the name).

Also note that cursorColumnSelectLeft/Right doesn't have any keybindings assigned by default on Linux and no one seems to mind (no issue about it).

@alexdima
Copy link
Member

alexdima commented Mar 26, 2018

@jhasse I am referring to Windows. On Windows, we bind the following:

{ "key": "ctrl+shift+alt+down",     "command": "cursorColumnSelectDown" },
{ "key": "ctrl+shift+alt+left",     "command": "cursorColumnSelectLeft" },
{ "key": "ctrl+shift+alt+pagedown", "command": "cursorColumnSelectPageDown" },
{ "key": "ctrl+shift+alt+pageup",   "command": "cursorColumnSelectPageUp" },
{ "key": "ctrl+shift+alt+right",    "command": "cursorColumnSelectRight" },
{ "key": "ctrl+shift+alt+up",       "command": "cursorColumnSelectUp" },

They are special commands that implement a special kind of selection which some other editors call box selection. They store hidden state (e.g. the right variant can go beyond the max column on a line). Rebinding two of the commands makes no sense, as they are useful together.

If all you are doing is adding cursors in a straight line, then, feel free to use ctrl+alt+up and ctrl+alt+down which do that. These commands (column selection) are different, and the differences are indeed subtle, but important.

@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.

Change default keybinding of cursorColumnSelectUp to ctrl+shift+up on Windows

3 participants