jupyterlab-contrib / jupyterlab-vim

Vim notebook cell bindings for JupyterLab
https://jupyterlab-contrib.github.io/jupyterlab-vim.html
MIT License
660 stars 43 forks source link

`Esc` does not work in visual mode #104

Closed navdeeprana closed 10 months ago

navdeeprana commented 10 months ago

When in visual mode, I expect Esc to exit and return to the normal mode, but it does nothing.

ianhi commented 10 months ago

hmm indeed. This seems to be another conflict with the documentsearch commands:

Cannot execute key binding 'Escape': command 'documentsearch:end' is not enabled.
lukashergt commented 10 months ago

I can confirm, pressing v in vim mode within a cell will get you into visual select mode. Normally Esc should get you out of visual mode, but it currently has no effect.

Work-around: You can press v again to exit visual mode.

ianhi commented 10 months ago

I'm pretty sure that this and the others issues like will be resolved once jupyterlab 4.1 is released as that will include https://github.com/jupyterlab/jupyterlab/pull/14421

firai commented 10 months ago

Can you try adding https://github.com/jupyterlab-contrib/jupyterlab-vim/issues/101#issuecomment-1664928232 to your keyboard shortcut settings in the Advanced Settings Editor to see if it fixes the problem? The settings may need to be added at the top for them to take effect if you're keeping the full set of settings that JL populates by default.

krassowski commented 10 months ago

Is this solved jupyterlab 4.1 alpha (pip install -U --pre jupyterlab)?

firai commented 10 months ago

I'm still getting the Cannot execute key binding 'Escape': command 'documentsearch:end' is not enabled. error with JL 4.1.0a1

krassowski commented 10 months ago

With search box open, or closed?

firai commented 10 months ago

With the search box closed. Once I open search box at least once, the error no longer shows up, but I still can't Esc out of either visual mode. Escing out of insert mode works fine.

firai commented 10 months ago

What are the scenarios that the documentsearch:end bindings are added for again? I just tried deleting the bindings for both JL4.0 (test at https://mybinder.org/v2/gh/firai/jupyterlab-vim/del-search-box-bindings) and JL4.1a1 (test at https://mybinder.org/v2/gh/firai/jupyterlab-vim/del-search-box-bindings-alpha-JL), and both escaping vim modes and escaping the search box seem to work for both?

krassowski commented 10 months ago

In plain JupyterLab the scenario is as follows:

  1. open search box
  2. click on a cell to enter the edit mode
  3. press escape to get into JupyterLab command mode
  4. press escape to dismiss the search box

For jupyterlab-vim one could expect this to work as:

  1. open search box
  2. click on a cell to enter the edit mode and vim insert mode
  3. press (shift) escape to get into vim command mode
  4. press escape to get into JupyterLab command mode
  5. press escape to dismiss the search box

After testing your binders it indeed appears to work this way.

The only problem I see is that when you open a text editor (rather than notebook) there is no step (4) which means that the search dialog cannot be dismissed without focusing on it. Any ideas?

firai commented 10 months ago

@krassowski, I see what you mean now. It would nice to be able to just keep pressing Esc to close the search box, but the fact that it doesn't in the file editor may not be a serious issue. Users who are accustomed to vim may be more likely to use / and :sover the search box in a file editor context anyway. The search box is only a necessity for vim users in notebooks because codemirror-vim search can't cross cell boundaries. (Although I wonder if there's a way to hijack the commands and make them proxies for the search box...) Also, if the user does use the search box, it doesn't seem like an unreasonable burden to press Ctrl+F to refocus on the search box, such that Esc would dismiss it.

What do you think about removing the bindings for now to fix core behavior for the plugin, and then file a new issue about this, in case someone comes up with a fix for this specific search box interaction later?

@ianhi, thoughts?

krassowski commented 10 months ago

Could we try:

krassowski commented 10 months ago

Sorry, by adding a CSS class when file editor is in visual/insert mode I meant hooking into the vim events/logic and adding a custom class to expose it to the selector. I think this should be possible but may be tricky. Depending on how difficult it is we may decide to go either way, but I would think that if this is not currently doable we should open an issue upstream.

firai commented 10 months ago

Sorry, I realized what you mean after I wrote the comment, so I deleted it. May be worth opening upstream anyway.

firai commented 10 months ago

Working on this custom hook is a bigger project than what I have time for right now. Are you available to work on this? If we don't expect to implement what you said in the near future, I would still propose that we remove the bindings for now (PR #107) so that we restore the core behavior of the plugin as a priority, and then leave what you're proposing as a future to-do.