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.

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

feat!: better composite key support #1917

Merged
merged 14 commits into from
Apr 29, 2024

Conversation

xiyaowong
Copy link
Collaborator

@xiyaowong xiyaowong commented Apr 28, 2024

The main idea is to always register the type command, but when there is no need to take over user input (generally in insert mode), forward the received inputs to the default:type command.

The implementation is to judge the input before forwarding. If it matches, call the configured command, otherwise continue to forward to the default:type

At present, this implementation is normal, but it feels that forwarding input causes a slight delay in input (this may be a psychological effect). Need help testing this. If there is indeed a delay, consider forwarding inputs only when the user needs the composite keys. If the user does not need to use composite keys, then just dispose the command when there is no need to take over user input. ( Even if there is a delay, it is very slight.)


New configurations:

  • compositeTimeout
  • compositeKeys

Removed commands: vscode-neovim.compositeEscape1 vscode-neovim.compositeEscape2

Usage

  1. jj to escape
{
    "vscode-neovim.compositeKeys": {
        "jj": {
            "command": "vscode-neovim.escape"
        }
    }
}
  1. jk to escape and save
{
    "vscode-neovim.compositeKeys": {
        "jk": {
            "command": "vscode-neovim.lua",
            "args": ["vim.api.nvim_input('<ESC>')\nrequire('vscode-neovim').action('workbench.action.files.save')"]
        }
    }
}

@xiyaowong xiyaowong marked this pull request as ready for review April 28, 2024 12:52
@xiyaowong xiyaowong changed the title feat!: better composite key method feat!: better composite key support Apr 28, 2024
src/typing_manager.ts Outdated Show resolved Hide resolved
src/typing_manager.ts Outdated Show resolved Hide resolved
src/typing_manager.ts Outdated Show resolved Hide resolved
}
],
"patternProperties": {
"^[a-zA-Z]{2}$": {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ollien VSCode does not warn about configuration errors here, not sure what's wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll take a look :)

Copy link
Collaborator

@ollien ollien Apr 28, 2024

Choose a reason for hiding this comment

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

Without altering the schema, I don't think we can do this without making the composite key itself a value (e.g. we could have an array of composite key mapping objects like {"mapping": "jj", ...}). There's patternErrorMessage in the schema, but I don't think that works for anything but string values (i.e. not keys).

Your in-line error is probably sufficient, but would be nice if it specified which composite key was the problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thx! I actually forgot the vscode docs...

@ollien
Copy link
Collaborator

ollien commented Apr 29, 2024

FWIW, I've been using this most of today with no composites configured and haven't noticed any(?) perceptible delays on input

@xiyaowong xiyaowong marked this pull request as draft April 29, 2024 07:19
@xiyaowong xiyaowong marked this pull request as ready for review April 29, 2024 13:28
@xiyaowong
Copy link
Collaborator Author

So far, everything is normal.

@xiyaowong xiyaowong merged commit 44808b3 into vscode-neovim:master Apr 29, 2024
8 checks passed
@MaheshNat
Copy link

MaheshNat commented Apr 30, 2024

I used to have jk mapped to composeEscape 1 and 2 using the following:

{
  "command": "vscode-neovim.compositeEscape1",
  "key": "j",
  "when": "neovim.mode == insert && editorTextFocus",
  "args": "j"
},
{
  "command": "vscode-neovim.compositeEscape2",
  "key": "k",
  "when": "neovim.mode == insert && editorTextFocus",
  "args": "k"
},

after switching to:

"vscode-neovim.compositeKeys": {
  "jk": {
    "command": "vscode-neovim.lua",
    "args": [
      "vim.api.nvim_input('<ESC>')\nrequire('vscode-neovim').action('workbench.action.files.save')"
    ]
  }
}

I am getting a bug where when I have my cursor position set in insert mode (say on line x), then use my mouse to click elsewhere (say on line y), then press jk, my cursor ends up going back to line x instead of staying on line y. This throws me off and did not occur with the old settings.

@xiyaowong
Copy link
Collaborator Author

@MaheshNat Change args to
require('vscode-neovim').action('vscode-neovim.escape') require('vscode-neovim').action('workbench.action.files.save')

@jmhammond
Copy link

Is there a benefit for doing the composite keys in VSCode vs having in my init.lua file:

vim.keymap.set("n", "<Leader>fs", ":write<CR>", { noremap = true, silent = true })
vim.keymap.set("n", "<Leader>ff", ":e ", { noremap = true, silent = true })

I'm new to the extension (since about last week, and it's been great!). I just want to make sure I'm not cobbling something fragile vs. intended features.

@theol0403
Copy link
Member

Composite escape should only be for insert mode commands.

@uduse
Copy link
Contributor

uduse commented Apr 30, 2024

Hey thanks for the feature!

This actually breaks my use case. My escape is ,, (two commas), which worked with the previous version but not this one.

Another feedback is that it's not clear in the documentation that we are removing a key config and replacing it with something settings.json.

@goyalyashpal
Copy link

goyalyashpal commented May 1, 2024

hi! would it allow to use alt-key as escape key for all the keys used by neovim?

i.e. say, i press alt-j in insert mode, then it would become equivalent to pressing escape then j thus bringing me in normal mode and then navigate down.

but rather than me configuring for each combination hard-codingly, it is performed automatically for at least good enough range of the most significant combos

like alt- h, j, k, l, i, a, o, y, p, x, d, r, c, u, (, ), {, }, etc

this would help me immensly.

@ollien
Copy link
Collaborator

ollien commented May 1, 2024

@goyalyashpal please open a separate issue for this, as it's pretty unrelated to this PR

@echaya
Copy link

echaya commented May 2, 2024

The new setting will trigger a file save in "interactive python window" which I have no intention to save the results there. Can we use anything other than 'workbench.action.files.save so it does not trigger the file saving please?

@xiyaowong
Copy link
Collaborator Author

{
    "vscode-neovim.compositeKeys": {
        "jj": {
            "command": "vscode-neovim.escape"
        }
    }
}

@echaya
You can use any commands you want. workbench.action.files.save is just for example.

@vscode-neovim vscode-neovim locked as resolved and limited conversation to collaborators May 2, 2024
@vscode-neovim vscode-neovim unlocked this conversation May 2, 2024
@KenzKD
Copy link

KenzKD commented May 2, 2024

Hey thanks for the feature!

This actually breaks my use case. My escape is ,, (two commas), which worked with the previous version but not this one.

Another feedback is that it's not clear in the documentation that we are removing a key config and replacing it with something settings.json.

I had made this error and was placing the code snippet in the keyboard shortcuts.json file. It would really help if I this specific change was mentioned on the composite Keys section of the Details panel. I had to find out about this by asking on Reddit.

@echaya
Copy link

echaya commented May 2, 2024

Hey thanks for the feature!
This actually breaks my use case. My escape is ,, (two commas), which worked with the previous version but not this one.
Another feedback is that it's not clear in the documentation that we are removing a key config and replacing it with something settings.json.

I had made this error and was placing the code snippet in the keyboard shortcuts.json file. It would really help if I this specific change was mentioned on the composite Keys section of the Details panel. I had to find out about this by asking on Reddit.

I second this. I also had to come to this post to figure out it is settings.json that i need to add the new setting.

@xiyaowong
Copy link
Collaborator Author

xiyaowong commented May 2, 2024

If there is a problem, please open an issue.

@xiyaowong xiyaowong deleted the feat/better-composite-keys branch May 2, 2024 09:38
@k-chainalysis
Copy link

Hey thanks for the feature!
This actually breaks my use case. My escape is ,, (two commas), which worked with the previous version but not this one.
Another feedback is that it's not clear in the documentation that we are removing a key config and replacing it with something settings.json.

I had made this error and was placing the code snippet in the keyboard shortcuts.json file. It would really help if I this specific change was mentioned on the composite Keys section of the Details panel. I had to find out about this by asking on Reddit.

I second this. I also had to come to this post to figure out it is settings.json that i need to add the new setting.

Same! But now that I figured out settings.json (not keybindings.json) good to go. Thanks.

@vscode-neovim vscode-neovim locked as resolved and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet