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

Fix(VIM-2760) notebookCommandMode detection #559

Merged
merged 2 commits into from
Dec 28, 2022

Conversation

Runinho
Copy link
Contributor

@Runinho Runinho commented Nov 25, 2022

Hi,
I tried to fix VIM-2760 because it really annoyed me :D

notebookCommandMode always returns false which results in handling the enter key wrong when in command mode of the notebook.
I tried to fix that by checking the client property ActionUtil.ALLOW_PlAIN_LETTER_SHORTCUTS that is set by NotebookEditorModeListenerAdapter.

I don't know if this is just a hack or a viable way to do it.
Another option to fix the issue would be to create our own listener. Sadly, I can't manage to register one:

import org.jetbrains.plugins.notebooks.ui.editor.actions.command.mode.NOTEBOOK_EDITOR_MODE

NOTEBOOK_EDITOR_MODE.subscribe(null, MyNotebookEditorModeListenerAdapter())

results in the following error:

 Class org.jetbrains.plugins.notebooks.ui.editor.actions.command.mode.NotebookEditorModeKt must not be requested from main classloader of com.intellij plugin

If the client property method is ok, I think we can remove the HandlerInjector Class and move the static notebookCommandMode function somewhere else (maybe into helper/EditorHelper.kt).

Let me know if I should do that refactoring or if there is a better way to detect the command mode.

Thanks,
Runinho

@AlexPl292
Copy link
Member

Hi there! Thank you for your fix. This is not a perfect solution, but an acceptable workaround.

@AlexPl292 AlexPl292 merged commit faebf66 into JetBrains:master Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants