fix: <C-d> remapping disabled by default. functionality controlled by "handleKeys" #2269
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Your proposed fix also does not fix windows/linux. |
|
This "fix" is for windows/linux, well, if that remapping line is for mac, which I assumed, not sure what it does originally. |
|
This is the original PR adding the change. #1631 |
|
So after reexamined those issues you mentioned. How about if users explicitly disabled <C-d> means they want default <C-d> multi-cursor feature, else handle as the rest. |
ghost
changed the title
ghost
changed the title
Travis tests have failedHey Arxzin, Node.js: 8.9.1git ls-tree -r HEAD --name-only | grep ".*.[t|j]s$" | xargs ./node_modules/prettier/bin/prettier.js --write --print-width 100 --single-quote --trailing-comma es5gulp |
which match the standard *handlekeys config trumps usectrlkeys config*
jpoon
requested changes
Jan 10, 2018
src/mode/modeHandler.ts
Outdated
| if (key === '<C-d>' && !Configuration.useCtrlKeys) { | ||
| key = '<D-d>'; | ||
|
|
||
| // #2162 fixed by 5cc821e except <C-d> key due to this remapping |
There was a problem hiding this comment.
remove the comment. that information is in git blame
src/mode/modeHandler.ts
Outdated
| // Now keep the remapping but check if <C-d> is explicitly defined | ||
| // within the handleKeys scope firstly | ||
| if (key === '<C-d>') { | ||
| const useKeyCtrlD = Configuration.handleKeys['<C-d>']; |
There was a problem hiding this comment.
This can be all simplified to one statement.
if (Configuration.handleKeys['<C-d>'] ===false || !Configuration.useCtrlKeys) {
...
}
There was a problem hiding this comment.
remap <C-d> to <D-d> within this statement?
what about
"vim.useCtrlKeys": false,
"vim.handleKeys": {
"<C-d>": true
}
, should vim <C-d> enable?
|
Let's draw this out cause it's kind of confusing.
where VS Code default behaviour for cc: @Chillee cause you added this rebinding initially. |
|
That's the confusing part, thought we want
where I thought remap to <D-d> does the same thing as VS Code default behaviour, add selection to next find match, from these lines from actions.ts, 1, 2. From user's perspective, they want |
|
I want to eventually remove
|
|
Yep, it either further confuses users and developers, or makes people suffer from writing long configs to decide every Ctrl key. lol. Maybe remove it is a better idea. The "consistence" is more important after all. |
When I get rid of Not sure when I'll get around to doing that. As such, do you want to update this PR to match the latest discussed table then we can merge? |
|
Yes, already made Vim Or should it not be when it's undefined? just let |
Travis tests have failedHey Curvas, Node.js: 8.9.1git ls-tree -r HEAD --name-only | grep ".*.[t|j]s$" | xargs ./node_modules/prettier/bin/prettier.js --write --print-width 100 --single-quote --trailing-comma es5gulp |
Travis tests have failedHey Arxzin, Node.js: 8.9.1git ls-tree -r HEAD --name-only | grep ".*.[t|j]s$" | xargs ./node_modules/prettier/bin/prettier.js --write --print-width 100 --single-quote --trailing-comma es5gulp |
ghost
changed the title
ghost
changed the title
ghost
changed the title
ghost
changed the title
|
Updated PR. |
jpoon
changed the title
jpoon
approved these changes
Jan 12, 2018
|
Thanks @arxzin |
|
@jpoon hi, How could I set handleKeys to undefined ? I want use VS Code default behavior in C-d |
|
Try to use the trick instead before resolved:
|
|
1.(Use ⌘+K ⌘+S) Open Keyboard Shortcuts, this works for me perfectly, hope anyone see this solve the problem i have. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR make
<C-d>disabled by default, unless users explicitly defined<C-d>:trueinhandleKeys.This configuration
It should have
<C-d>working as Vim move page half down, now it behaviors asadd selection to next find match.This PR modified this line, eventually results in this logic:
<D-d><D-d>Which issue(s) this PR fixes
#2162
Special notes for your reviewer:
<D-d>perform as VS Code default behavior, which isadd selection to next find match.