strindberg / emacsj

Emacs-inspired IntelliJ plugin
MIT License
12 stars 3 forks source link

ISearch Backward Text conflict. #41

Open charjr opened 3 months ago

charjr commented 3 months ago

ISearch Backward Text appears to be conflicting with Search In Command History in a terminal. Removing/changing the binding for Search In Command History makes it work as expected. This has only been an issue for me as of version 2024.1

image

Other than that, brilliant plug in! Thank you for your hard work.

strindberg commented 3 months ago

Thank you very much for your report! I see it as well, but it must be some new addition in 2024.1. I will investigate.

strindberg commented 3 months ago

After further investigation, I am doubtful I should do anything about this in the EmacsJ plugin. The ctrl-r binding seems to be a new addition to the Terminal plugin in 2024.1.

This is a genuine clash of traditions, in my view: ctrl-r is bound to isearch-backward in Emacs, but it is also the standard keybinding for history search in bash, zsh and other shells. Starting a terminal within an editor puts these two traditions/contexts into conflict.

My mantra in developing this plugin has often been "when in doubt, do as Emacs does it", and I believe Emacs actually uses ctrl-r for history search if one starts a terminal within Emacs: using it for isearch-backward requires a rebinding of the key.

Furthermore, it would feel a bit presumptuous for me to override the key bindings of another plugin: it seems more reasonable that users of both plugins will resolve such conflicts manually. I actually don't know exactly what it is that makes the terminal plugin's key binding take precedence over EmacsJ's, and truth be told I haven't even figured out exactly how this keybinding is registered by the terminal plugin; it seems to use some mechanism that I haven't seen before.

But in any case, for the above reasons, I believe that we should leave the plugin as it is, and that people who, like you, want to use ctrl-r for isearch-backward within the terminal can reconfigure the keys.

That said, I'd be open for further discussion, of course.

charjr commented 3 months ago

I agree, ctrl+r to search history in terminal makes sense. But let me elaborate further on the conflict I was experiencing.

The clash I was having is: If I pressed ctrl+r outside the terminal it pops up the isearch-backward prompt, this is what I expect.

Still outside a terminal, once the isearch-backward prompt is displayed, pressing ctrl+r a second time closes it, this is not what I expect.

This means I cannot successively perform isearch-backwards, I can only perform the action once.

My short-term fix has been to remove the ctrl+r keybinding for terminal history searching which makes isearch-backward work as expected.

I also do not know why a terminal keybinding is conflicting with bindings outside of the terminal (And I'm afraid I have no experience writing IntelliJ plugins so I do not know how to fix it either)

Either way, thank you for the fantastic plugin and I'll be continuing to use it!

strindberg commented 3 months ago

I see. Thank you for the clarification. This is worse than I thought, of course. At the same time, I cannot right away see why the Teminal plugin would "steal" a key binding in this way and if it's the fault of my plugin or not. I'll have to reflect on this.

charjr commented 2 months ago

@strindberg Not helpful as a fix, but maybe helpful to anyone with the same issue:

Strangely enough if I remove the terminal keybinding, it still works but it also stops stealing the keybinding from EmacsJ.

So I'm now able to reverse-i-search in the editor but I'm also reverse-i-search my command history even though it's not bound to anything.

Maybe it's a specific combination of settings I've got, I honestly don't know.

strindberg commented 2 months ago

To add not-helpful notes to this issue: I just noticed that ctrl+wbehaves the same as ctrl+r: the Terminal plugin steals the shortcut if it's bound to a terminal shortcut (delete word backwards by default). De-activating the terminal's shortcut also restores the functionality. I have filed a bug report with the Terminal plugin: https://youtrack.jetbrains.com/issue/IDEA-352735/Keyboard-shortcuts-from-ActionPromoter-are-not-respected-by-Terminal-plugin