emacs-evil / evil-magit

Black magic or evil keys for magit
https://github.com/justbur/evil-magit
GNU General Public License v3.0
273 stars 16 forks source link

Support evil-search-module set to evil-search #33

Closed dieggsy closed 7 years ago

dieggsy commented 7 years ago

Currently, / is bound to evil-search-forward. However, that should be evil-ex-search-forward when evil-search-module is set to evil-search, for consistency with evil's own settings.

justbur commented 7 years ago

Why would it be bound to the ex version of the command? What evil settings are you referring to?

dieggsy commented 7 years ago

@justbur As I mentioned, I'm referring to the variable evil-search-module, that is

(setq evil-search-module 'evil-search)

set in :init of (use-package evil ...). With that set, I see / bound to evil-ex-search-forward, not evil-search-forward.

justbur commented 7 years ago

That is strange, but I can't reproduce it.

I tried this from emacs -Q

(use-package evil
  :init (setq evil-search-module 'evil-search)
  :config (evil-mode 1))

(use-package evil-magit)

and / is bound to evil-search-forward.

FWIW I also use the evil-search module and I've never seen anything like this.

dieggsy commented 7 years ago

@justbur to clarify, do you mean / is bound to evil-search-forward in all buffers, or just in magit?

I was saying that with it should be bound to evil-ex-search-forward in magit, as it is in other buffers when using the evil-search module.

dieggsy commented 7 years ago

By default, this seems to be set by evil-select-search-module in the evil-search-module defcustom, but it is hardcoded to use evil-search-forward regardless of the evil-search module in evil magit.

dieggsy commented 7 years ago

I'm not sure if I'm wording myself clearly enough. It is a default evil option to bind the key / to evil-ex-search-forward when using (setq evil-search-module 'evil-search). I'm saying that evil-magit doesn't honor that distinction by always binding / to evil-search-forward.

justbur commented 7 years ago

I understand now. I wasn't aware that customize flipped the commands like that. My first reaction is that this is an odd way for evil to handle the distinction. It's also confusing because the name evil-ex-... suggests that it's reserved for ex.

I think the right way to deal with this might be to clean up evil's handling of the search modules, but I am not sure at the moment.

cc @wasamasa

wasamasa commented 7 years ago

I'd just leave it alone, not worth breaking other people's customizations for a purely aesthetical issue.

dieggsy commented 7 years ago

@wasamasa leave what alone? should it at least be changed in evil-magit to reflect the evil option? As it stands, the customization (setq evil-search-module 'evil-search) is ignored by evil-magit.

justbur commented 7 years ago

@dieggsy I pushed a fix for this. I still think the way evil handles this is bad for several reasons, but I'll leave it alone for now.

@wasamasa Note that (setq evil-search-module 'evil-search) is not equivalent to using the customize interface, because using the latter also changes evil-motion-state-map. This is not clear at all from the docstrings.

dieggsy commented 7 years ago

thanks!