Fully replace evil-integration

[noctuid]

From emacs-evil/evil#992:

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.

Currently, evil-collection is mainly focused on keybindings and is not an alternative to evil-integration. I'm not suggesting doing any opinionated configuration by default, only adding "fixes" that it's unlikely anyone would object to (e.g. advising show-paren-function to make show-paren-mode work correctly in normal state, adding evil command properties such as :repeat, :type, :keep-visual, and :suppress-operator, etc.). Anything considered potentially intrusive could be optional. Unfortunately, since evil-integration is all or nothing and will stay that way, its useful, non-intrusive functionality needs to be replicated in another package for users who want to sanely only use those parts of evil-integration. Maybe evil-collection is not the suitable place to do this, but I'm interested to hear your thoughts.

[jojojames]

Yup, as mentioned in the same thread.

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

I'm fully ok with copy-pasting from evil-integration and disabling what doesn't make sense and was planning to once a version of evil with that change is out in the wild.

[jojojames]

Planning on attempting this in the next week (or few weeks).

I think a solid peacemeal plan would be to:

1. Copy over all/most of evil-integration and disable evil-integration in evil. (Achieve parity)
2. Remove all overriding maps in evil-integration-copy that are already handled here or at least gate it with a defcustom.
3. Refactor the integration into their separate packages (if there is a separate package for it). (Clean up)
4. If there's no suitable (doubt it) place for a binding in evil-integration-copy, we can wrap it with a defcustom to conditionally enable/disable it.

@Ambrevar @noctuid Sound good?

If anyone else has the time or energy, feel free to kick start the process too.

[Ambrevar]

Looks good to me.

Next few weeks will be busy ones on my end :/

[jojojames]

#66

Step 1 should be done.

The rest will probably come in piecemeal.

[jojojames]

I wonder how we want to handle something like this in evil-integration.

(eval-after-load 'company
  '(progn
     (mapc #'evil-declare-change-repeat
           '(company-complete-mouse
             company-complete-number
             company-complete-selection
             company-complete-common))

     (mapc #'evil-declare-ignore-repeat
           '(company-abort
             company-select-next
             company-select-previous
             company-select-next-or-abort
             company-select-previous-or-abort
             company-select-mouse
             company-show-doc-buffer
             company-show-location
             company-search-candidates
             company-filter-candidates))))

We can put it in the company file but it'd be reasonable for a user to disable the company integration (assuming they have their own setting). But doing that would mean they'd disable this nice integration too.

The want here is that these integrations should go with their integration file but I could see a user wanting this 'integration' but not wanting the bindings.

The changes in evil-integration just around bindings are easier because they're just bindings.

It might be better to leave this type of change in evil-collection-integration but that seems unsatisfying.

[Ambrevar]

Found this on reddit: https://www.reddit.com/r/emacs/comments/7obhve/evil_and_avy/

[jojojames]

Any idea on how we want to handle this? This might be one of those needs that would suggest tweaking the init system for these packages to handle the case where a user wants 'integration' but no bindings.

We could always just go for the absolutely simple route and consider integration to be the same as bindings and let the user recreate whatever setup is needed.

[noctuid]

Relevant PR to evil.

This might be one of those needs that would suggest tweaking the init system for these packages to handle the case where a user wants 'integration' but no bindings.

I think it would be best to give the user a way to opt out of getting keybindings. For example, another argument could be added to evil-collection-init that would disable binding keys (and this could be passed to each setup function). The user could then run evil-collection-init for modes individually or twice (once for the modes they want keybindings and once for other modes). Alternatively, there could be another list added of modes to make keybindings in that is checked in each setup function. I personally like the first way since each setup function takes an argument which is more direct (and simple I think).

Edit: evil-collection doesn't consistently do any intrusive/opinionated configuration/keybindings, so I think it would be fine to configure that with per-file settings. For example, I think it might be better if the minibuffer was included in evil-collection-mode-list, and there was a setting for whether it should make the minibuffer modal (off by default). By default, it could bind escape directly in the relevant minibuffer keymaps instead of in normal state.

[jojojames]

Yeah, I've been noodling about how to do this. I think that part is what's blocking the rest of evil-collection-integration being dispersed into separate modules.

@noctuid If you had a good idea on how to handle this, I think a PR would be awesome.

I'd think we'd want to be able to allow the user to disable either the integration and/or the keybindings. It may or may not look similar to what @Ambrevar was playing around with a while back on tweaking the init list.

[noctuid]

I can potentially make a pull request at some point if I have time, but I'd rather everyone agree on how to do it before doing any work. To elaborate on my previous suggestion, each setup function would now takes two optional arguments. For example: (cl-defun evil-collection-ag-setup (&optional (bind t) (integrate t))). That or the setup functions could be split (one for integration and one for keybindings).

As for evil-collection-init, either it could have extra arguments like the other setup functions, or evil-collection-mode-list could be extended, maybe with keyword arguments (e.g. (list ag avy (bookmark :bind nil) (mode :integrate nil))). The latter might be more convenient if the user wanted keybindings for most modes. On the other hand, if they just wanted integration for all modes and keybindings for only a few, it would require a lot of :bind nils. Maybe both methods could be supported.

was playing around with a while back on tweaking the init list.

Can you point me to that?

Regardless of the method used, work could potentially be done just to split integration from keybindings in each file (even if it still always runs both for now). If not for old files, it may be a good idea to do this for any new files to make things less work later on.

I haven't looked through everything, but we may want to discuss any special cases beforehand. There are some cases where keybindings should be considered integration (e.g. for the avy remap commands and for my suggested change to the minibuffer keybindings). I think integration should be only for any "fixes" that pretty much every user will want, but we can still have it be optionally disabled.

[jojojames]

That or the setup functions could be split (one for integration and one for keybindings).

Don't have a strong opinion either way. @Ambrevar

On the other hand, if they just wanted integration for all modes and keybindings for only a few, it would require a lot of :bind nils. Maybe both methods could be supported.

I'm erring on the side of someone that wants this package would probably use it mostly for the keybindings so maybe we can assume the case where the user wants 0 bindings is an edge-case.

@Ambrevar ?

Open to suggestions here if we can have our cake and eat it too.

Can you point me to that?

It was on a separate branch. I forget what the goal there was.

https://github.com/emacs-evil/evil-collection/tree/init

There are some cases where keybindings should be considered integration (e.g. for the avy remap commands and for my suggested change to the minibuffer keybindings). I think integration should be only for any "fixes" that pretty much every user will want, but we can still have it be optionally disabled.

I don't have a strong opinion where we draw the line.

[noctuid]

so maybe we can assume the case where the user wants 0 bindings is an edge-case.

I think anyone who would normally want to just use evil-integration but doesn't because of its extra keybindings/use of override maps may use this package mainly for the integration. I think we can probably cater to this use case without making things too much more complicated.

[Ambrevar]

On the other hand, if they just wanted integration for all modes and keybindings for only a > few, it would require a lot of :bind nils.

It's Lisp after all, users are free to use loops to simplify the setup. I don't think this would be a problem at all.