search

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

Fix `modifyCell` or `modifyEditor` not called after text editor is opened after notebook #105

Closed firai closed 10 months ago

firai commented 10 months ago

Fixes #103.

On ILabShell currentChanged(), check if currentWidget matches one tracked by editorTracker or notebookTracker, and call modifyEditor() or modifyCell() accordingly.

@krassowski, can you please help review whether this is right approach?

github-actions[bot] commented 10 months ago

Binder :point_left: Launch a Binder on branch firai/jupyterlab-vim/modifyCell-when-changing-active-widget

firai commented 10 months ago

Yes, as far as I can tell. The code block and the console.warn were copied from here: https://github.com/jupyterlab-contrib/jupyterlab-vim/blob/f08ef414dd2908171b56768319205b1b3a33dd65/src/index.ts#L87-L104 Do you have any suggestions on what the console.warn should be replaced with? Also, I suppose I'm supposed to refactor this out into a function so that it isn't duplicated?

krassowski commented 10 months ago

Well, the difference is that when vim:enter-normal-mode is run we expect the current widget to be vim-enabled, but shell.currentChanged will fire for any kind of a widget. I would say you could use console.debug or just remove it (if you want to leave the logic for clarity you can add // no-op comment).

IMO duplication is fine as these are slightly different contexts.

firai commented 10 months ago

The build process fails without the if (!current) code path, so I'll replace with no-op comments