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

Keybinding remapping applied beyond specified scope ("when":) #83412

Closed
janosh opened this issue Oct 28, 2019 · 14 comments
Closed

Keybinding remapping applied beyond specified scope ("when":) #83412

janosh opened this issue Oct 28, 2019 · 14 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug keybindings VS Code keybinding issues macos Issues with VS Code on MAC/OS X verified Verification succeeded
Milestone

Comments

@janosh
Copy link

janosh commented Oct 28, 2019

  • VSCode Version:Version: 1.39.2
  • OS Version: macOS 10.14.6
  • Also present in insider's build

Steps to Reproduce:

  1. Install LaTeX Utilities
  2. cmd + v stops working in the integrated terminal and search inputs on macOS. Linux is fine.

Presumably due to the shortcut

{
  "key": "cmd+v",
  "command": "latex-utilities.formattedPaste",
  "when": "editorTextFocus && editorLangId == 'latex' && config.latex-utilities.formattedPaste.useAsDefault"
}
@Tyriar Tyriar assigned alexdima and unassigned Tyriar Oct 28, 2019
@Tyriar
Copy link
Member

Tyriar commented Oct 28, 2019

editorTextFocus issue?

@janosh
Copy link
Author

janosh commented Oct 28, 2019

@Tyriar As @tecosaur mentioned here, the issue does not reproduce on Linux, suggesting that it's a Mac problem. The first possibility that came to mind was that editorTextFocus may not be disabling the shortcut as it should when the editor does not have focus.

@tecosaur
Copy link

@janosh with just plain vscode, have you tried adding something like

{
  "key": "cmd+v",
  "command": "editor.action.addCommentLine",
  "when": "editorTextFocus"
}

to your keybindings.json? I think that would provide a rather useful result, and maybe a more minimal MWE.

@janosh
Copy link
Author

janosh commented Oct 29, 2019

@tecosaur Good call! That works as expected. So not a problem with editorTextFocus and not a MWE, I'm afraid.

Still, even in an empty project (i.e. no config) and with all other extension disabled, as soon as I enable LaTeX Utilities, I can no longer shortcut paste (right click to paste still works) in the integrated terminal or in search inputs. With only LaTeX Workshop enabled, the problem is gone. So maybe it's LU after all?

@tecosaur
Copy link

To me this just seems odd.
Since it looks like editorTextFocus by itself works, I would expect editorTextFocus && editorLangId == 'latex' && config.latex-utilities.formattedPaste.useAsDefault to be false whenever editorTextFocus is.

As long as that a && b && c statement works as expected, I then wouldn't expect this keybinding to have any effect on pasting outside the 'main' editor.

Could you try removing that keybinding statement from the package.json in your installation and seeing if it fixes it?

  • If it doesn't that it's almost certainly LU.
  • However if it does fix it then it still looks like a vscode issue (to me).

@janosh
Copy link
Author

janosh commented Oct 29, 2019

In ~/.vscode/extensions/tecosaur.latex-utilities-0.3.3/package.json, I commented out

"keybindings": [
	// {
	// 	"key": "ctrl+shift+v",
	// 	"mac": "cmd+shift+v",
	// 	"command": "latex-utilities.formattedPaste",
	// 	"when": "!config.latex-utilities.formattedPaste.useAsDefault && editorTextFocus && editorLangId == 'latex'"
	// },
	// {
	// 	"key": "ctrl+shift+v",
	// 	"mac": "cmd+shift+v",
	// 	"command": "editor.action.clipboardPasteAction",
	// 	"when": "config.latex-utilities.formattedPaste.useAsDefault && editorTextFocus && editorLangId == 'latex'"
	// },
	// {
	// 	"key": "ctrl+v",
	// 	"mac": "cmd+v",
	// 	"command": "latex-utilities.formattedPaste",
	// 	"when": "config.latex-utilities.formattedPaste.useAsDefault && editorTextFocus && editorLangId == 'latex'"
	// }
],

and that solved the issue. After reloading VS Code, cmd + v worked again everywhere even with LU enabled.

One thing I should have mentioned earlier (but forgot) is that with LU enabled, the context menu shows that pasting is remapped to cmd + shift + v (that was before commenting out the "keybindings").

@tecosaur
Copy link

In that case, I return to suspecting vscode. Based on what you said about the context menu saying that remapping has occurred, perhaps the keybindings parser identifies the remapping and applies it globally (i.e. ignoring the when). That's just speculation though, I'll leave the detailed diagnosis to those working on vscode who have to fix it.

@tecosaur
Copy link

Oh @janosh, a separate MWE to try, how about just adding this to your user keybindings (and no LU etc.)

{
  "key": "cmd+shift+v",
  "command": "editor.action.clipboardPasteAction",
  "when": "editorTextFocus"
},
{
  "key": "cmd+v",
  "command": "editor.action.addCommentLine",
  "when": "editorTextFocus"
}

@janosh
Copy link
Author

janosh commented Oct 29, 2019

Both keybindings work as expected in the editor and, more importantly, the remapped clipboardPasteAction applies to the integrated terminal and search inputs as well which I assume is unexpected behavior. With only addCommentLine

// {
//   "key": "cmd+shift+v",
//   "command": "editor.action.clipboardPasteAction",
//   "when": "editorTextFocus"
// },
{
  "key": "cmd+v",
  "command": "editor.action.addCommentLine",
  "when": "editorTextFocus"
}

pasting in terminal and search is back to cmd + v.

@tecosaur
Copy link

Fantastic! I think we've narrowed down on the issue at hand.

Keybinding remapping is applied beyond the specified scope ("when":)

I'd consider updating the title and MWE on your initial comment at this point.

@janosh janosh changed the title Possible regression for editorTextFocus in 1.39.2 on macOS Keybinding remapping applied beyond specified scope ("when":) Oct 29, 2019
@alexdima
Copy link
Member

alexdima commented Oct 29, 2019

Nice analysis! The root cause is:

{
	"key": "ctrl+shift+v",
	"mac": "cmd+shift+v",
	"command": "editor.action.clipboardPasteAction",
	"when": "config.latex-utilities.formattedPaste.useAsDefault && editorTextFocus && editorLangId == 'latex'"
}

This makes our reverse lookup for editor.action.clipboardPasteAction believe that cmd+shift+v is the preferred key combination (it appears "later" in the keybindings list). So that's why the Edit menu at the top will most likely render cmd+shift+v next to the Paste action for you. That, by itself, is pretty reasonable...

But the unreasonable thing is how Paste actually works on macs in other inputs than the editor. Basically, cmd+v doesn't do anything on macs in Electron. The only reason cmd+v pastes in the terminal or the find input is because we display cmd+v next to the Paste action in the top level menu, and then cmd+v is captured by the top level menus (outside of our keybindings story), which end up dispatching the editor.action.clipboardPasteAction action to the renderer. That action is pretty nice and checks if an editor has focus, and if not, it invokes the corresponding command on the native inputs here

I have encountered this before and I thought it is so rare that it doesn't warrant further workarounds from us... See #74599 (comment)

But now I think we should perhaps add some workaround specifically for this case... Perhaps we should always consider cmd+v as the preferred keybinding for editor.action.clipboardPasteAction

The minimal repro would be in keybindings.json:

{
	"key": "alt+w", // something else than cmd+v
	"command": "editor.action.clipboardCopyAction",
	"when": "textInputFocus" // anything actually
}

@alexdima alexdima added macos Issues with VS Code on MAC/OS X keybindings VS Code keybinding issues bug Issue identified by VS Code Team member as probable bug labels Oct 29, 2019
@tecosaur
Copy link

@alexandrudima from what you've said, I understand that vscode captures the shortcut, and then tries the highest priority keybinding. Correct?

Would a decent approach be tweaking the behaviour such that if the "when" condition on the highest priority keybinding fails, the second-highest is tried etc.?

@alexdima
Copy link
Member

alexdima commented Oct 29, 2019

Normally, if you look at our keybindings.json we don't have any keybindings for paste outside of a code editor. Search for "cmd+v" without a "when" condition, there is no rule... So normally our keybinding dispatching does not interpret cmd+v, we leave the keybinding to fall through and we leave Electron to do something there... (for <input>s)

On Windows and on Linux, our lack of a keybinding results by default in Electron generating a paste event in the <input>s and pasting. On mac there is simply no paste event happening by default...

On top of this, the mac has a weird twist. When we render a keybinding in the top level menu, this is not only rendering it... The OS dispatches keybindings to the menu directly... In fact, we don't even get the keybinding anymore... You can notice this by seeing a blue flash in the menu... If we render anything in the menus, pressing that key combination will result in Electron/the OS interpreting and capturing that and converting that into a menu invocation... So when we render cmd+v next to the Edit > Paste menu, pressing cmd+v does not reach us. We only get to know that Edit > Paste was invoked... We don't even get to know (this might be an Electron limitation) if it was clicked via the mouse or the displayed keybinding was pressed.

Anyways, the mac menu is weird, the default cmd+v is missing on the mac and the way pasting in native inputs works today is too brittle. I will probably add another set of keybindings to capture cmd+v/cmd+c/cmd+x for simple <input>s and execute paste/copy/cut...

That should result in things working even when cmd+v is no longer displayed in the menus, because then our keybinding dispatching will dispatch those...

@tecosaur
Copy link

Huh. This all sounds like a huge hassle/mess. After reading that I'm quite happy just dealing with my nice little extension without having to deal with that headache 😃

Anyway, thanks for looking at this, and best of luck!

@alexdima alexdima added this to the October 2019 milestone Oct 29, 2019
@connor4312 connor4312 added the verified Verification succeeded label Oct 31, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug keybindings VS Code keybinding issues macos Issues with VS Code on MAC/OS X verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@Tyriar @connor4312 @alexdima @tecosaur @janosh and others