Race condition with VSpaceCode comma binding (whichkey.triggerKey)

[The-Compiler]

Bug description

It looks like the default , binding of VSpaceCode works by opening vscode-which-key, and then (probably asynchronously) sending the m key to it. However, when typing quickly, this can result in a race condition between what which-key does internally and what the user types.

To Reproduce

• Open a .tex file (or presumably, any other file with a x binding for "text")
• Press ,
• Press x very quickly

(Alternatively, on Linux/X11, run sleep 1; xdotool type ,x and then switch focus to VS Code)

Expected behavior

Ending up in the "Text" menu:

Screenshots

Ending up in the "Merge conflict" menu:

Additional context

The merge conflict menu is bound to SPC x m, so pressing , x ended up being interpreted as SPC x m rather than SPC m x.

Keybindings

Essentially the default recommended in the VSpaceCode readme:

Click to toggle contents of `keybindinds.json`

[
    // https://github.com/VSpaceCode/VSpaceCode/blob/master/src/configuration/keybindings.jsonc

    // Trigger vspacecode in empty editor group
    {
        "key": "space",
        "command": "vspacecode.space",
        "when": "activeEditorGroupEmpty && focusedView == '' && !whichkeyActive && !inputFocus"
    },
    // Trigger vspacecode when sidebar is in focus
    {
        "key": "space",
        "command": "vspacecode.space",
        "when": "sideBarFocus && !inputFocus && !whichkeyActive"
    },
    // Keybindings required for edamagit
    // https://github.com/kahole/edamagit#vim-support-vscodevim
    // Cannot be added to package.json because keybinding replacements
    {
        "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'"
    },
    {
        "key": "x",
        "command": "magit.discard-at-point",
        "when": "editorTextFocus && editorLangId == 'magit' && vim.mode =~ /^(?!SearchInProgressMode|CommandlineInProgress).*$/"
    },
    {
        "key": "k",
        "command": "-magit.discard-at-point"
    },
    {
        "key": "-",
        "command": "magit.reverse-at-point",
        "when": "editorTextFocus && editorLangId == 'magit' && vim.mode =~ /^(?!SearchInProgressMode|CommandlineInProgress).*$/"
    },
    {
        "key": "v",
        "command": "-magit.reverse-at-point"
    },
    {
        "key": "shift+-",
        "command": "magit.reverting",
        "when": "editorTextFocus && editorLangId == 'magit' && vim.mode =~ /^(?!SearchInProgressMode|CommandlineInProgress).*$/"
    },
    {
        "key": "shift+v",
        "command": "-magit.reverting"
    },
    {
        "key": "shift+o",
        "command": "magit.resetting",
        "when": "editorTextFocus && editorLangId == 'magit' && vim.mode =~ /^(?!SearchInProgressMode|CommandlineInProgress).*$/"
    },
    {
        "key": "shift+x",
        "command": "-magit.resetting"
    },
    {
        "key": "x",
        "command": "-magit.reset-mixed"
    },
    {
        "key": "ctrl+u x",
        "command": "-magit.reset-hard"
    },
    // Extra ref menu support for edamagit with the key "y"
    // Cannot be added to package.json because keybinding replacements
    {
        "key": "y",
        "command": "-magit.show-refs"
    },
    {
        "key": "y",
        "command": "vspacecode.showMagitRefMenu",
        "when": "editorTextFocus && editorLangId == 'magit' && vim.mode == 'Normal'"
    },
    // Easy navigation in quick open/QuickPick
    {
        "key": "ctrl+j",
        "command": "workbench.action.quickOpenSelectNext",
        "when": "inQuickOpen"
    },
    {
        "key": "ctrl+k",
        "command": "workbench.action.quickOpenSelectPrevious",
        "when": "inQuickOpen"
    },
    // Easy navigation in sugesstion/intellisense
    // Cannot be added to package.json because of conflict with vim's default bindings
    {
        "key": "ctrl+j",
        "command": "selectNextSuggestion",
        "when": "suggestWidgetMultipleSuggestions && suggestWidgetVisible && textInputFocus"
    },
    {
        "key": "ctrl+k",
        "command": "selectPrevSuggestion",
        "when": "suggestWidgetMultipleSuggestions && suggestWidgetVisible && textInputFocus"
    },
    {
        "key": "ctrl+l",
        "command": "acceptSelectedSuggestion",
        "when": "suggestWidgetMultipleSuggestions && suggestWidgetVisible && textInputFocus"
    },
    // Easy navigation in parameter hint (i.e. traverse the hints when there's multiple overload for one method)
    // Cannot be added to package.json because of conflict with vim's default bindings
    {
        "key": "ctrl+j",
        "command": "showNextParameterHint",
        "when": "editorFocus && parameterHintsMultipleSignatures && parameterHintsVisible"
    },
    {
        "key": "ctrl+k",
        "command": "showPrevParameterHint",
        "when": "editorFocus && parameterHintsMultipleSignatures && parameterHintsVisible"
    },
    // Add ctrl+h/l to navigate in file browser
    {
        "key": "ctrl+h",
        "command": "file-browser.stepOut",
        "when": "inFileBrowser"
    },
    {
        "key": "ctrl+l",
        "command": "file-browser.stepIn",
        "when": "inFileBrowser"
    }
]

Settings

Click to toggle contents of `settings.json`

{
    /********** VSpaceCode boilerplate **********/
    // https://github.com/VSpaceCode/VSpaceCode/blob/master/src/configuration/settings.jsonc

    "vim.easymotion": true,  // Vim easymotion is required for Jump menu - <SPC> j
    "vim.useSystemClipboard": true,
    // Trigger the main which key menu with in the active editor with vim
    // This cannot be put into keybindings.json because it will
    // create conflict with Vim.
    // https://github.com/stevenguh/spacecode/issues/3
    "vim.normalModeKeyBindingsNonRecursive": [
        {
            "before": ["<space>"],
            "commands": ["vspacecode.space"]
        },
        {
            "before": [","],
            "commands": [
                "vspacecode.space",
                {
                    "command": "whichkey.triggerKey",
                    "args": "m"
                }
            ]
        }
    ],
    "vim.visualModeKeyBindingsNonRecursive": [
        {
            "before": ["<space>"],
            "commands": ["vspacecode.space"]
        },
        {
            "before": [","],
            "commands": [
                "vspacecode.space",
                {
                    "command": "whichkey.triggerKey",
                    "args": "m"
                }
            ]
        }
    ],

    /********** Custom VSpaceCode bindings **********/
    "vspacecode.bindingOverrides": [
        {
            "keys": "g.b",
            "name": "Blame file with GitLens",
            "type" : "command",
            "command": "gitlens.toggleFileBlame",
        },
        {
            "keys": "g.B",
            "name": "Blame file with magit",
            "type" : "command",
            "command": "magit.blame-file",
        },
        {
            "keys": "g.g",
            "name": "GitLens Commands",
            "type" : "command",
            "command": "gitlens.gitCommands",
        },


        {
            "keys": "h.z",
            "name": "Zeal for current text",
            "type" : "command",
            "command": "extension.dash.specific",
        },
        {
            "keys": "h.Z",
            "name": "Open Zeal",
            "type" : "command",
            "command": "extension.dash.emptySyntax",
        },

        {
            "keys": "p.o",
            "name": "Open folder",
            "type" : "command",
            "command": "workbench.action.files.openFolder",
        },
    ],

    /********** Appearance **********/
    "editor.cursorBlinking": "phase",
    "editor.cursorSurroundingLines": 2,
    "editor.fontFamily": "Iosevka",
    "editor.fontLigatures": true,
    "editor.fontSize": 18,
    "editor.inlineHints.fontFamily": "Iosevka",
    "editor.minimap.scale": 2,
    "editor.minimap.showSlider": "always",
    "editor.renderControlCharacters": true,
    "editor.renderLineHighlightOnlyWhenFocus": true,
    "editor.renderWhitespace": "trailing",
    // "editor.cursorSmoothCaretAnimation": true,
    "errorLens.fontFamily": "Iosevka Extralight",
    "errorLens.fontSize": "10pt",
    "search.showLineNumbers": true,
    "vim.showMarksInGutter": true,
    "window.dialogStyle": "custom",
    "window.menuBarVisibility": "toggle",
    "window.zoomLevel": 2,
    "workbench.colorTheme": "Monokai Classic",
    "workbench.editor.highlightModifiedTabs": true,
    "workbench.iconTheme": "Monokai Classic Icons",
    "workbench.tree.indent": 16,

    /********** Behavior **********/
    "debug.console.closeOnEnd": true,
    "editor.find.autoFindInSelection": "multiline",
    "editor.mouseWheelZoom": true,
    "editor.tabCompletion": "on",
    "file-browser.hideIgnoredFiles": true,
    "file-browser.labelIgnoredFiles": true,
    "files.insertFinalNewline": true,
    "files.simpleDialog.enable": true,
    "keyboard.dispatch": "keyCode",  // https://stackoverflow.com/a/50875402/2085149
    "search.smartCase": true,
    "terminal.external.linuxExec": "kitty",
    "vim.camelCaseMotion.enable": true,
    //"vim.textwidth": 88,
    "workbench.startupEditor": "newUntitledFile",

    /********** Privacy **********/
    "update.mode": "none",
    "telemetry.enableCrashReporter": false,
    "telemetry.enableTelemetry": false,
    "workbench.settings.enableNaturalLanguageSearch": false,

    /*********** Git **********/
    "magit.forge-enabled": true,
    "gitlens.hovers.currentLine.over": "line",
    "gitlens.currentLine.enabled": false,
    // "gitlens.codeLens.scopes": [
    //     "document",
    //     "containers",
    //     "blocks"
    // ],
    "gitlens.hovers.currentLine.enabled": false,

    /********** Python **********/
    "python.useIsolation": false,  // for pytest to find qutebrowser package

    /********** LaTeX **********/
	"latex-workshop.latex.tools": [
		{
			"name": "latexmk",
			"command": "latexmk",
			"args": [
                "-shell-escape",  // FIXME do we really want this globally?
				"-synctex=1",
				"-interaction=nonstopmode",
				"-file-line-error",
				"-pdf",
				"-outdir=%OUTDIR%",
				"%DOC%"
			],
			"env": {}
		},
    ],
    // https://github.com/James-Yu/LaTeX-Workshop/wiki/View#using-synctex-with-an-external-viewer
    "latex-workshop.view.pdf.viewer": "external",
    "latex-workshop.view.pdf.external.viewer.command": "zathura",
    "latex-workshop.view.pdf.external.viewer.args": [
        "--synctex-editor-command",
        "code --reuse-window -g \"%{input}:%{line}\"",
        "%PDF%"
    ],
    "latex-workshop.view.pdf.external.synctex.command": "zathura",
    "latex-workshop.view.pdf.external.synctex.args": [
        "--synctex-forward=%LINE%:0:%TEX%",
        "%PDF%"
    ],

    /********** Dash/Zeal **********/
    "dash.languageIdToDocsetMap": {
        "ansible": [
            "ansible"
        ],
        "python": [
            "python",
            "qt",
        ],
    },
    "workbench.editorAssociations": [
        {
            "viewType": "jupyter.notebook.ipynb",
            "filenamePattern": "*.ipynb"
        }
    ],

    /********** Unknown **********/
    // "workbench.editorAssociations": [
    //     {
    //         "viewType": "jupyter.notebook.ipynb",
    //         "filenamePattern": "*.ipynb"
    //     }
    // ],
    // "workbench.colorCustomizations": {
    //     "statusBar.background": "#005f5f",
    //     "statusBar.noFolderBackground": "#005f5f",
    //     "statusBar.debuggingBackground": "#005f5f"
    // },
    // "yaml.customTags": [
    //     "!encrypted/pkcs1-oaep scalar",
    //     "!vault scalar"
    // ],
}

System information

[stevenguh]

I didn't think user can type faster than the rendering of the QuickPick 😅. This actually was on my mind when deciding different solutions for implementing the shortcut , for major mode.

Anyway, I will implement a solution/fix that won't have this race condition for shortcut to major mode.

[The-Compiler]

Hehe 😊 I guess when already having the keybinding memorized, and when it happens to be on two separate fingers, that doesn't hold true anymore 😄

Thanks for taking a look, let me know if I can help somehow!

[The-Compiler]

Let me note that this seems like an issue in generic too - not sure if it's fixable though, and if so, whether I should open a separate issue for it.

If I hit SPC p f quickly, I can see pf being entered into the first QuickPick (before the second one gets displayed), then I end up in the +Project menu rather than the +Find file in project one.

edit: I can see there's some handling for this kind of case here:

vscode-which-key/src/menu/menu.ts

Lines 107 to 125 in 9c6ad3a

	private async onDidChangeValue(value: string, when?: string) {
	this.when = when;
	if (this.timeoutId) {
	// When the menu is in the delay display mode
	if (value.startsWith(this.enteredValue)) {
	// Normal case where user enters key while in delay
	value = value.substring(this.enteredValue.length);
	} else if (this.enteredValue.startsWith(value)) {
	// Disallow user from removing entered value when in delay
	await this.setValue(this.enteredValue);
	return;
	} else {
	// special case for fixing the quick pick value if trigger key command
	// was called before the menu is displayed. When the menu is displayed,
	// quick pick value is then selected. Any subsequent key will remove
	// the value in the input.
	await this.setValue(this.enteredValue + value);
	}
	}

and indeed it gets much better when I set whichkey.delay to 1 instead of 0. Maybe that logic should be active in some way even without a delay. I can try contributing a fix, but it might take me a while as I have a lot on my plate with my own projects 🙂

[stevenguh]

To solving the race condition of entering second key right after ,, I am thinking to supply initial value instead of simulating a key press after waiting the menu to open. The race condition for the original issue is that the simulated press comes after the key x.

---

If I hit SPC p f quickly, I can see pf being entered into the first QuickPick (before the second one gets displayed), then I end up in the +Project menu rather than the +Find file in project one.

For this case, it can be adjacent to original issue. But let's open another issue to track it. I think the cause of this is probably because multiple onDidChangeValue events are called before the pervious one has a chance to complete, and the time to finish onDidChangeValue probably also increased by #34. Two attempts in my heads are

1. Have a queue to batch the onDidChangeValue to handle event in sequence. However, probably won't work by itself for the upcoming 1.57 (Extension broken with the latest VS Code update (1.57.0-insider) #34) due to how we modify the input.
2. Change the input handling completely to eliminate modification of the input field.

indeed it gets much better when I set whichkey.delay to 1 instead of 0. Maybe that logic should be active in some way even without a delay

The difference is that

vscode-which-key/src/menu/menu.ts

Lines 239 to 245 in 9c6ad3a

	const updateQuickPick = async () => {
	this.quickPick.busy = false;
	this.enteredValue = '';
	await this.setValue('');
	this.quickPick.title = this.title;
	this.quickPick.items = this.items;
	};

is running on asynchronously when delay is 1. The might be due to us needing to have a workaround to await for set value for #34.

@The-Compiler Wondering what's the version of vscode you are running? Can you also try which-key version v0.8.4 to see if it's any better? Btw, I am also having trouble reproducing the issue on my computer

[The-Compiler]

But let's open another issue to track it.

Alright - let's continue that part in #39!

[The-Compiler]

FWIW I can still reproduce this one even after downgrading to v0.8.4, using sleep 2 && xdotool type ',x'.

[stevenguh]

Thanks for checking in :) This in some sense is easier to fix than #39. We just need to build a proper support to pass in initial keys instead of "simulating" keys pressed with consecutively commands.

[stevenguh]

On a second thought, this might be solved with approach outlined in #39 with better input handling (queue).

[stevenguh]

Released in v0.10.0. The new implementation should be faster and more robust as it uses a processing queue to handle input change.

It is tested with

sleep 1; xdotool type ,; sleep 0.01; xdotool type x;
# or
sleep 1; xdotool type " "; sleep 0.01; xdotool type mx;

We have to wait for at least 10ms for the UI input to be shown before we can type. That's the limit of what we can do.

[The-Compiler]

Seems to work beautifully now, at least from what I can see so far. Thanks! 👍