emacs-evil/evil-cleverparens

evil-cleverparens shadows evil-snipe s/S binds

Closed this issue · 5 comments

To avoid shadowing/clobbering evil-snipe's s/S binds, evil-cleverparens requires two things:

  1. evil-snipe must be loaded before evil-cleverparens (this should probably appear in the README); and
  2. evil-snipe set the variable evil-snipe-auto-disable-substitute to nil.

However, evil-snipe stopped using or defining that variable in June 2017, which was around the time of the last commit in this repo (though notably a few weeks before the last commit here). See hlissner/evil-snipe@3cf3228

Or, in other words, that evil-snipe commit happened around the time when this repo was abandoned, by all appearances.

The workaround is to set evil--snipe-auto-disable-substitute to nil before evil-cleverparens is loaded:

(use-package evil-cleverparens
  :init (setq evil-snipe-auto-disable-substitute nil))

But if any maintainer is still around and interested in maintaining this project, here is the offending code:

;; If evil-snipe is not present or does not want to use s and S bindings,
  ;; then we can use them. To take effect, evil-snipe must be loaded before us.
  (when (and evil-cleverparens-use-s-and-S
             (not (bound-and-true-p evil-snipe-auto-disable-substitute)))
    (evil-define-key 'normal evil-cleverparens-mode-map
      "s" 'evil-cp-substitute
      "S" 'evil-cp-change-whole-line)))

Bumping this now that the project appears to be active again.

Thanks @rallyemax - I'll take a look when I can. Unfortunately I got hit by a wave of busyness in the last couple of months, but when it dies down a little this'll be top of my list :)

@rallyemax I've found a bit of time to look at this, but I'm confused. I would have thought this is wrong:

(use-package evil-cleverparens
  :init (setq evil-snipe-auto-disable-substitute nil))

Surely it needs to be set to t in order to avoid evil-cleverparens overriding evil-snipe's bindings?

Also, isn't the solution to this to add something like this to your config:

(evil-define-key 'normal evil-cleverparens-mode-map "s" nil "S" nil)

I'm not really sure why this ever needed to read a variable from another package at initialization time. My inclination for this issue is to remove the code that makes this conditional on the obsolete evil-snipe var, and to put something in the readme about either using evil-cleverparens-use-s-and-S or the just unbinding the keys from the mode map by using nil as above.

Or maybe I'm missing something. Like I said, I'm a bit confused 😄

I agree with your inclination to avoid cross-package variable checking and to make the configuration of evil-cleverparens self-contained and declarative.

Surely it needs to be set to t in order to avoid evil-cleverparens overriding evil-snipe's bindings?

You are correct. I've always stumbled when combining and negating boolean variables that are defined in the negative (things like disable-foo). It's a weird case of my intuition jumping in with Oh, I know this! before the analytical neurons have a chance to weigh in...and the intuition is wrong every single time. I've had a post-it over my primary coding monitor for years with "avoid negative boolean names" in bright red all caps.

I fixed the oops in my config -- probably shortly after filing this issue -- but forgot to update the issue itself.

I think it's worthwhile to mention this in the README, as you point out. Feel free to close this issue at your convenience.