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

Helm-minibuffer-history interferes with backward search: simple solution #51

Closed
andersjohansson opened this issue Jun 5, 2020 · 11 comments

Comments

@andersjohansson
Copy link
Contributor

When using helm and loading the default keybindings in helm-config, helm-minibuffer-history is bound to C-r in minibuffer-local-map, which the minibuffer keymap used in ctrlf inherits from. Pressing C-rwill then call helm-minibuffer-history instead of the remapped isearch-backward from ctrlf-mode-bindings.

I solved this simply for my case by:

(add-to-list 'ctrlf-minibuffer-bindings '("C-r" . nil))

I don’t know if anything should be done on the side of ctrlf to prevent this, or if it should be left to the user (perhaps with a note in the readme?). One idea otherwise is to bind C-r to isearch-backward and C-s to isearch-forward in ctrlf-minibuffer-bindings to prevent any other command from overriding it, but I suppose that's not a very good idea either as we can’t assume those are the keys the user wants to use.

What we would like to do is unbind (or bind correctly) any bindings for the keys used for isearch-forward/backward when the minibuffer keymap is created, but I don’t know if this is possible

@hartzell
Copy link

I ran into the same issue, but with counsel-minibuffer-history. The solution above makes things work for me (using straight):

(use-package ctrlf
  :init
  (ctrlf-mode +1)
  :config
  (add-to-list 'ctrlf-minibuffer-bindings '("C-r" . nil)))

raxod502 added a commit that referenced this issue Jun 18, 2020
@raxod502
Copy link
Member

Should be fixed, please test.

@hartzell
Copy link

Works for me. Thanks!

@andersjohansson
Copy link
Contributor Author

This works for me too. Big thanks! Although I am a bit confused as to why this works, since I thought the changes in bindings for ctrlf--keymap shouldn’t override the minibuffer bindings inherited from minibuffer-local-map (where helm-minibuffer-history is bound to C-rfor me) when I am in a ctrlf-session. There is something that I don’t quite understand about emacs bindings it seems.

@raxod502
Copy link
Member

I thought the changes in bindings for ctrlf--keymap shouldn’t override the minibuffer bindings inherited from minibuffer-local-map

Perhaps it has to do with the fact that ctrlf-mode is a minor mode, and minor mode keymaps typically override everything else. Otherwise, it wouldn't be possible to enable a minor mode in the minibuffer and have it override minibuffer bindings.

@andersjohansson
Copy link
Contributor Author

Perhaps it has to do with the fact that ctrlf-mode is a minor mode, and minor mode keymaps typically override everything else. Otherwise, it wouldn't be possible to enable a minor mode in the minibuffer and have it override minibuffer bindings.

I see!

@raxod502
Copy link
Member

I've come up with a more robust keymapping scheme. It's a bit of a hack but should address the conflicting needs of these various bug reports. My new solution should solve #51, #52, #67, and #80 simultaneously while also working with remap bindings. I have tested it a bit and it seems to work, but I don't actually use most of the modes that have been mentioned in these issue reports, so I could use some help to confirm if the bug is resolved for all of them.

@andersjohansson
Copy link
Contributor Author

In my testing the new change seems to solve the problem for helm-minibuffer-history.

@jsigman
Copy link

jsigman commented Oct 7, 2021

I get the same interference with helm-minibuffer-history using the latest code, and (add-to-list 'ctrlf-minibuffer-bindings '("C-r" . nil) disables using C-r to move backwards, as well. Is this known?

@raxod502
Copy link
Member

I don't think so. Can you open a separate issue for that? I just tested by setting

(define-key minibuffer-local-map (kbd "C-r") (lambda () (interactive) (message "Hello world!")))

to emulate what Helm does in binding a key in minibuffer-local-map, and typing C-r C-r still has the expected effect.

Notably, running

(add-to-list 'ctrlf-minibuffer-bindings '("C-r" . nil)

as you say you've done should not disable C-r, as it should result in the [remap isearch-backward] binding taking precedence instead. Something peculiar must be going on.

@jsigman
Copy link

jsigman commented Dec 2, 2021

It stopped being a problem and I can't recreate it

@raxod502 raxod502 mentioned this issue Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants