-
Notifications
You must be signed in to change notification settings - Fork 19
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
Reliable key sequence handling #85
Comments
Thanks for writing this up. These days I have been using vscode web and experienced losing key due to slow rendering of initial menu (I can think of a few things to optimize the initial quick pick rendering -- needs profiling to see if those actually matter however). I suspect this may be similar, have you tried adding a delay after the space key in your reproduction steps?
I think once the Quickpick is rendered, no keys should be lost. Proposed SolutionsIIUC to sum this up, the idea is to use setContext to shorten key handling time, and use the vscode built-in shortcut's when condition to filter out what was entered. My understanding is that this approach would make the menu UI very hard to implement. Not to mention that it will need to "capture" a lot of keys as shortcuts, and makes it really hard to update for end user. Without QuickpickThis is what this repo before QuickPick was used: https://github.com/VSpaceCode/VSpaceCode/tree/vscode-vim. This relies on the key already "captured" by vscode vim so it should have any delay. But no menu. |
I agree with this according to my understanding of the quickpick API and after reading the current implementation.
I'm not an expert on vscode extension development but I think the latency of running commands (via some sort of RPC from the frontend to the extension host) is another contributing factor, even though it runs as "ui". For web version on this latency is probably better.
I took a quick look just now and I can imagine that the user experience is much worse than rendering possible commands with the quickpick UI. |
Hey, I've got some interesting updates :) Recently I've spent quite some time writing a new extension that mimics what VSpaceCode+which-key do. The experience is much better (still not perfect in theory) by not depending on the quickpick menu (which I believe is the main source of problems). You are welcome to try this with minimal setup, similar to the VSpaceCode extension (see the readme)! and the source code is here Also for the web vscode you should set it to be run in ui mode "remote.extensionKind": {
"JimmyZJX.leaderkey": [
"ui"
]
}, |
Thanks for the quick feedback!!
If instead one bind the "space" key with {
"key": "space",
"command": "runCommands",
"args": {
"commands": [
{
"command": "_setContext",
"args": [
"leaderkeyState",
"SPC"
]
},
{
"command": "leaderkey.render",
"args": "SPC"
}
]
},
"when": "!leaderkeyState && vim.mode == 'Normal'",
} it becomes extremely reliable for keys typed starting from Unfortunately, it's not the way vscodevim works, and you might run into problems like I'm still open to new solutions proposed but it's been slightly more reliable for me. I think the reason is that we don't pay additional delay to wait for the quickpick UI to be rendered. Also I feel like the quickpick UI sometimes might enter a weird state say when |
2-4 are minor issues in comparison. For 2 and 3, the vscode API is not flexible enough for me to get the viewport of the current editor and the UI positioning end up being very dumb in may aspects. Btw there is a status bar showing you the current state of leaderkey (e.g. For 4, due to the current implementation of the key states, I'll need register callbacks (e.g. active editor being closed or swapped out) that intuitively should cause the state to quit. That should be somewhat straightfoward in general and I'll address that soon |
This works more consistently for me as well 👍 Q: How come you are using
It is very unfortunately. IIRC that's also why the default integration for vspacecode is using vscodevim remapping instead of the built-in shortcuts so to not break something like
Agree. But it does make me wondering if the same capture hack should be used in this repo to capture keys before menu is being rendered. edamagit used a separate readonly editor, but I doubt that would be any faster. Not sure if there's other APIs.
Maybe the TAB was being processed by the editor? Speaking of TAB, I think you are missed the capturing of TAB in the default shortcuts because
Agree. A lot of these really are the limitation of the APIs.
The editor is in the new window that's why there is no status bar I think. |
I'll reply to your other bullets later, but this
is likely caused by a vspacecode configuration that looks like {
"key": "tab",
"command": "extension.vim_tab",
"when": "editorFocus && vim.active && !inDebugRepl && vim.mode != 'Insert' && editorLangId != 'magit'"
},
{
"key": "tab",
"command": "-extension.vim_tab",
"when": "editorFocus && vim.active && !inDebugRepl && vim.mode != 'Insert'"
}, |
Problem
I constantly lose part of my key sequence when the vscode window is not very responsive.
For example, when I type "SPC f s" very fast, sometimes the which-key menu ends up in the state of "SPC s" and leave vscodevim in the "f" mode.
Analysis
Fundamentally, according to my understanding, it is caused by the multi-process design of VSCode.
vscode-which-key
commands are handled in the extension-host process and the user's keystrokes are asynchronously send via RPC. There's just no way to make theQuickPick.show
call reliably happen before the key is "handled".One can easily verify it via the autohotkey script (preferrably with some wrapper like adding a shortcut and some delay before)
In my test setup, I've used a virtual machine with 1GB memory with some VSCode windows opened. I can reproduce the misbahave easily.
Solution
Given that VSCode does not allow the extension command to block key handling, we should seek for reliable state tracking within the main process. And the idea that comes to my mind is to rely on the
command. Calling this command via
keybindings.json
is extremely reliable as it's just a synchronous function called on the main process.I wrote a tiny demo extension to prove the concept
and set up the keybindings like this
And now, no keys will ever get a chance to escape which-key. Even with no delays sending the keys with autohotkey, it is always reliable!
Limitations
I think it would be impossible to unset the context without any delay involved. My approach does not solve problems like
being unreliable. I think there's much less need for that to be handled without any race condition. This is also a problem with the current implementation because the quickpick events are sent asynchronously.
In general, I think this is just impossible if you want to dynamically quit the context. (Otherwise you can keep track of all the states by encoding the key-sequence state in
keybindings.json
viasetContext
)One possible drawback of my approach is that it can be unreliable when using together with vscodevim, since the space is not handled by vscodevim. However, I think we can just configure both and fallback to the current behavior if e.g.
ESC
andSPC
are typed very fast when the editor was in the insert mode.Default keybindings
We should declare all possible key strokes that which-key recognizes in VSCode shortcuts. This should be a reasonably enumerable list. Notice that we only need the
inWhichKey
condition in those bindings and not the complex conditions. Only the trigger key should be carefully defined with a correct set of conditions, and the user should be able to figure out the correct conditions by themselves.One plus we get from this approch is that now keys with modifiers can also be handled, like
ctrl+u
, in the middle of a sequence. We just need to also register them as keybindings like any other key. Users can also define their own bindings with modifiers as long as they register the keybinding.Possible implementations
Without Quickpick
One possibility is to completely get rid of the quickpick menu and read keys only via shortcuts. The main drawback here is that we need to decide when to "cancel" which-key, e.g. on vscode losing focus, or changing active editor. I'm not sure if this can be implemented with a clean solution.
For the UI, we can draw texts using decorations on the active editor like "SPC j w". If there's no active editor, not drawing is acceptable I think, or we fallback to the current quickpick. But that would be some work to completely rewrite the UI and behaviors that relies on the quickpick component.
Hybrid key handling
The other idea I have is to make use of the context to keep track of missing keys before the quickpick is shown while keeping the key sequence handling logic still bundled with the quickpick UI. Missing keys will be kept in a local queue and processed before the UI gets any text change event, and optionally hide the quickpick if it's already a command.
I haven't tried writing any real code on this idea, and I suspect that it might not work because we have no "onDidShown" event for the quickpick. Only by then we can safely unset the context and always handle keys with quickpick. It is also possible that calling
runCommand("setContext", ...)
afterquickpick.show()
always gurantees order of execution. If that's the case, then this approach will work and requires less changes on the existing codebase.Thanks for reading all these text and please let me know about your thoughts!
I think making key sequence handling reliable is very valuable in general and should be a top priority.
The text was updated successfully, but these errors were encountered: