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

Esc does not work in visual mode #104

Closed
navdeeprana opened this issue Aug 11, 2023 · 15 comments · Fixed by #107
Closed

Esc does not work in visual mode #104

navdeeprana opened this issue Aug 11, 2023 · 15 comments · Fixed by #107
Labels
bug Something isn't working

Comments

@navdeeprana
Copy link

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

@navdeeprana navdeeprana added the bug Something isn't working label Aug 11, 2023
@ianhi
Copy link
Collaborator

ianhi commented Aug 11, 2023

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

Cannot execute key binding 'Escape': command 'documentsearch:end' is not enabled.

@lukashergt
Copy link

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
Copy link
Collaborator

ianhi commented Aug 11, 2023

I'm pretty sure that this and the others issues like will be resolved once jupyterlab 4.1 is released as that will include jupyterlab/jupyterlab#14421

@firai
Copy link
Collaborator

firai commented Aug 18, 2023

Can you try adding #101 (comment) 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
Copy link
Collaborator

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

@firai
Copy link
Collaborator

firai commented Sep 4, 2023

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

@krassowski
Copy link
Collaborator

With search box open, or closed?

@firai
Copy link
Collaborator

firai commented Sep 4, 2023

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
Copy link
Collaborator

firai commented Sep 4, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

firai commented Sep 5, 2023

@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
Copy link
Collaborator

Could we try:

  • removing the bindings for notebook
  • adding a CSS class when file editor is in visual/insert mode
  • adjusting the binding selector for file editor so that it is only active when not in command mode but not in visual or insert mode?

@krassowski
Copy link
Collaborator

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
Copy link
Collaborator

firai commented Sep 5, 2023

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

@firai
Copy link
Collaborator

firai commented Sep 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants