Unsetting a key doesn't appear to work with complex selector

[lee-dohm]

Ref: atom/markdown-preview#114

I was experimenting with my keybindings to unmap the default Markdown Preview key:

'atom-workspace, atom-workspace atom-text-editor':
  'ctrl-shift-M': 'markdown-preview:toggle'

and set a new, grammar-scoped key:

'atom-text-editor[data-grammar="source gfm"]':
  'ctrl-shift-M': 'markdown-preview:toggle'

So I added the following to my keymap.cson:

'atom-workspace, atom-workspace atom-text-editor':
  'ctrl-shift-M': 'unset!'
'atom-text-editor[data-grammar="source gfm"]':
  'ctrl-shift-m': 'markdown-preview:toggle'

which, in theory, should make it so that Ctrl+Shift+M should only activate the Markdown Preview when a GFM text editor is focused. But if I do the following:

1. With the keymap modification above active, open a Markdown file
2. Focus the tree view
3. Press Ctrl+Shift+M

Expected: Markdown Preview is not displayed
Actual: Markdown Preview is displayed

Here's what I see in the Keybinding Resolver:

[lee-dohm]

Ref: https://discuss.atom.io/t/how-to-replace-a-keymap-binding/16834

I have another case where this occurs. This is on OS X (change the mapped key to Ctrl+J for Linux/Windows):

'atom-workspace atom-text-editor:not([mini])':
  'cmd-j': 'unset!'
'atom-text-editor':
  'cmd-j': 'core:move-left'

The person posting the issue wants to use Cmd/Ctrl+J to move left in all text editors, mini or not. So the above should unmap core:join-lines from Cmd/Ctrl+J and allow the less specific atom-text-editor to take precedence. But it doesn't:

The unset core:join-lines command still executes and core:move-left does not.

[lee-dohm]

A workaround is documented on the Discuss topic listed above.

[fgimian]

Your workaround saved me too, thank you! But sadly, this doesn't still resolve the case where you may wish to disable a key mapping completely. I really wish !unset would work as documented since it is a neat implementation.

[fgimian]

Okies, I have a true workaround for disabling a keyboard shortcut. Simply set to an in-existent command:

'atom-workspace atom-text-editor:not([mini])':
  'ctrl-shift-D': 'bla'

You may wish to choose a more suitable dummy command name, hehe 😄

[izuzak]

I'm seeing some strange behavior with unset! as well, even with selectors which are not complex.

Example: the tree view has this keybinding:

'.platform-darwin':
  'cmd-\\': 'tree-view:toggle'

And I can't unset this by adding this to my keymap.cson file:

'.platform-darwin':
  'cmd-\\': 'unset!'

As far as I can tell, keymap manager is not triggering the command here (dispatchCommandEvent is not called), but something else is triggering the behavior. Using abort! instead of unset! or adding event.preventDefault() to how unset! is handled fixes things -- the tree-view:toggle command doesn't trigger when I press cmd-\

[nathansobo]

Taking a look at this now.

[nathansobo]

This is happening because there is an application menu item that is still getting triggered via the operating system's default key binding support. Currently, the unset! command simply continues the search from the ancestor of the unset element. If no matching binding is found however, the browser still handles the keyboard event, which ends up running the command out of the application menu.

My preferred fix for this would be to be much smarter about how key bindings are shown in the application menu. But this is tricky because in order to determine whether or not a given binding was unset, we may need to run an arbitrary selector match against a specific element. Currently we just decide on the menu bindings in a global way.

So as a simpler solution, I'm just going to call preventDefault() whenever we encounter an unset! binding. If we continue the search and find another command, we'll call preventDefault() again, which shouldn't be a problem. If we don't find a command, the command from the application menu won't be invoked.

[izuzak]

Reopening since d0af390 was reverted in 46d726e.