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

[Request] Make evil-integration optional #992

Closed
jojojames opened this issue Dec 8, 2017 · 8 comments
Closed

[Request] Make evil-integration optional #992

jojojames opened this issue Dec 8, 2017 · 8 comments

Comments

@jojojames
Copy link
Contributor

  • Enhancement request

Hi, I'd like to request making evil-integration.el optional.

There seems to be some good integrations in that file so it might be a better idea to make only the extra keybindings optional?

Thanks!

@edkolev
Copy link
Member

edkolev commented Dec 8, 2017

I can submit a PR to make loading of evil-integration.el optional.

@jojojames what do you meen extra keybindings? Are you suggesting we split evil-integration into two parts and make loading of one of them optional?

Here's a patch which introduces a new variable evil-want-integration (default t):

diff --git a/evil-vars.el b/evil-vars.el
index 899aab5..8870b47 100644
--- a/evil-vars.el
+++ b/evil-vars.el
@@ -1853,6 +1853,11 @@ Otherwise the previous command is assumed as substitute.")
           "1.2.13"))))
   "The current version of Evil")

+(defcustom evil-want-integration t
+  "Whether to load evil-integration.el"
+  :type 'boolean
+  :group 'evil)
+
 (defun evil-version ()
   (interactive)
   (message "Evil version %s" evil-version))
diff --git a/evil.el b/evil.el
index 01b9bc1..5c9e54a 100644
--- a/evil.el
+++ b/evil.el
@@ -137,7 +137,9 @@
 (require 'evil-commands)
 (require 'evil-jumps)
 (require 'evil-maps)
-(require 'evil-integration)
+
+(when evil-want-integration
+  (require 'evil-integration))

 (run-hooks 'evil-after-load-hook)

@jojojames
Copy link
Contributor Author

There's quite a few useful functionality in evil-integration. The ones that are not so useful (potentially) are the keybinding ones or the ones that set the default modes and definitely one swith overriding maps.

These are quick examples of ones that might be better optional.

(evil-add-hjkl-bindings Buffer-menu-mode-map 'motion)

;; dictionary.el

(evil-add-hjkl-bindings dictionary-mode-map 'motion
  "?" 'dictionary-help        ; "h"
  "C-o" 'dictionary-previous) ; "l"

;;; Dired

(eval-after-load 'dired
  '(progn
     ;; use the standard Dired bindings as a base
     (defvar dired-mode-map)
     (evil-make-overriding-map dired-mode-map 'normal)
     (evil-add-hjkl-bindings dired-mode-map 'normal
       "J" 'dired-goto-file                   ; "j"
       "K" 'dired-do-kill-lines               ; "k"
       "r" 'dired-do-redisplay                ; "l"
       ;; ":d", ":v", ":s", ":e"
";" (lookup-key dired-mode-map ":"))))

But then things like

(defadvice show-paren-function (around evil disable)

are pretty useful.

At the very least, optionally disabling the entire file is great. I would just backport the rest to evil-collection, though it might be better to have more precision on what's optionally disabled.

@wasamasa
Copy link
Member

wasamasa commented Dec 8, 2017

A toggle for not loading the file is fine (it would make life for #797 (comment) easier), introducing more toggles, I dunno. My greatest issue with that is the debate following any finer-grained tunables, like what functionality exactly belongs to them and why they're not done correctly. See also #797 (comment) for another opinion.

@jojojames
Copy link
Contributor Author

If that's the easiest route for everyone, lets go with that. :)

@edkolev @wasamasa

Once that is in, I'll backport relevant changes to evil-collection instead so most of the integration can live in one place.

@wasamasa
Copy link
Member

wasamasa commented Dec 8, 2017

I'd like to think of evil-collection as serving as an alternative evil-integration.el here, so the all or nothing approach should be fine.

@noctuid
Copy link
Contributor

noctuid commented Dec 8, 2017

I'd prefer if anything having to do with keybindings was put in a separate file with the option to never load it. Any define-key, evil-define-key, evil-add-hjkl-bindings, and evil-make-overriding-map would be moved to this new file. It might make sense to keep some of the remap keybindings or put them in a separate file. I'd say it would be fine to keep the visual-line-mode integration since that is already opt-in. The avy integration with [remap] is also fairly non-intrusive. That said, for simplicity/consistency, I'd think it would be fine to put that in a new file as well.

@justbur
Copy link
Member

justbur commented Dec 8, 2017

If you just want to prevent loading the file, you could do

(provide 'evil-integration)
(require 'evil)

@wasamasa
Copy link
Member

Closed by #993.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants