broadinstitute / wdl-ide

Rich IDE support for Workflow Description Language
BSD 3-Clause "New" or "Revised" License
41 stars 9 forks source link

bug -- vscode -- keybinding for 'editor.action.codeAction' breaks "Find Previous" keybinding #16

Open vincent-czi opened 3 years ago

vincent-czi commented 3 years ago

Bug: Open the "find widget" (cmd+f), type whatever you want to search (that has multiple matches in the open file), hit enter a couple times. It will cycle forward through matches. Now hit shift+enter. Expected behavior: The match should cycle backward to the previous match ("Find Previous"). Actual behavior šŸ›: VS Code instead displays a mini-text pop-up saying "No code actions for 'wdl.run' available".

image

Cause: This behavior is caused by the very broad shift+enter keybinding to kick off a wdl.run code action. See here: https://github.com/broadinstitute/wdl-ide/blob/e47a94dacb51b17a1dfee429bcf0004cf3965b06/client/vscode/package.json#L27 Because "Find Previous" also uses shift+enter, these two conflict (there are also a lot of other possible conflicts, see the Keyboard Shortcuts page in VS Code and search for "shift enter"). I think because wdl-ide is an Extension, it gets precedence over the Default keybindings, so it always beats out "Find Previous". I could be wrong about the cause of precedence though: the Keyboard Shortcuts page allows you to "Sort by Precedence (Highest First)", and the above code action actually appears listed below Find Previous, but according to this issue [Extensions beat out Defaults] and this StackOverflow [You actually read precedence from bottom up in that table], I think the sort order has been flipped?

Proposed Solutions:

dinvlad commented 3 years ago

Thanks for reporting - would you be willing to submit a PR with a fix?

vincent-czi commented 3 years ago

PR made. I would very rarely run the code command, so I also added alt to avoid shadowing the many other shift+enter commands. If you think most of your users rely on it heavily though, that should possibly be removed.

dinvlad commented 3 years ago

Merged! The new build should be published shortly - please let me know if you still have issues after the 0.0.77 update..