emacs-evil / evil-collection

A set of keybindings for evil-mode
GNU General Public License v3.0
1.21k stars 275 forks source link

evil-collection-mu4e update required #695

Closed theophilusx closed 1 year ago

theophilusx commented 1 year ago

There are some changes in the development version of mu4e which break evil-collection-mu4e.el In particular, the function, mu4e~view-quit-buffer no longer exists. This means that mu4e-view-mode-map entry for 'q' needs to be changed. Instead of binding q to mu4e~view-quit-buffer, I found binding to quit-window works.

Note that this only affect the current 1.9.x version (1.9.7 specifically). It does not affect earlier versions of mu4e.

As it stands now, when the user hits q to leave the view buffer (mu4e-article window), an error is thrown - wrong argument commandp as the function mu4e~viewquit-buffer does not exist.

theophilusx commented 1 year ago

There have been even more significant refactoring of mu4e which has now completely broken the evil-colleciton mu4e module for mu4e version 1.9.14. In particular, the code which updated the mu4e main 'menu' screen so that the key bindings reflect the new evil bindings is now broken because the way the data in that screen is being generated has been completely re-factored.

The bad news is the existing evil-collection code in the mu4e module will need complete re-working. The good news is that this could be cleaner and easier given the refactoring which has been done in mu4e.

The challenge will be in keeping compatibility with the 1.8.x (release) branch, which still uses the older code.

The quick and dirty fix I've used was to just comment out the add-hook call which adds the function that updates the main screen. I don't care if it shows the wrong key bindings as I don't actually use that information anyway.

jojojames commented 1 year ago

We'll need a volunteer for someone to update mu4e as both I and @condy0919 don't use mu4e anymore/ever.

theophilusx commented 1 year ago

I reached out to the creator of mu4e to see if there was anything that could be done in mu4e to simplify updating of the interface to handle evil key bindings.

Good news is, Dirk responded very quickly and made some adjustments to the development version of mu4e so that modified key bindings are automatically reflected in the mu4e main page. This greatly simplifies the evil collection mu4e module as it removes the need to do the search and replace operations on the mu4e main buffer to update key binding info.

I made a very simple cut down version of the evil collection mu4e module for testing purposes and it seems to be working really well. The mu4e main buffer now reports the evil key bidings rather than the standard emacs bindings and there is no need to do any search/replace on that buffer (which was fragile and prone to breakage with updates).

A couple of issues have since been identified for people using hydra which Dirk is working on, so things are still in a little flux. I really just wanted to update the issue to let people know what progress is happening. Just in case anyone is interested, attached is the elisp file I'm using in place of the normal evil colleciton mu4e module. Note that it only works with the most recent development version of mu4e and things are likely to change.

Once things settle and assuming I can find the time, I will submit a PR. However, that may be a ways off.

(require 'mu4e nil t)

(defconst evil-collection-mu4e-maps '(mu4e-main-mode-map mu4e-headers-mode-map mu4e-view-mode-map mu4e-compose-mode-map mu4e-search-minor-mode-map))

(defun evil-collection-mu4e-set-state () "Set the appropriate initial state of all mu4e modes." (dolist (mode '(mu4e-main-mode mu4e-headers-mode mu4e-view-mode mu4e-org-mode)) (evil-set-initial-state mode 'normal)) (evil-set-initial-state 'mu4e-compose-mode 'insert))

;; When using org-mu4e, the above leads to an annoying behaviour, because ;; switching from message body to header activates mu4e-compose-mode, thus ;; putting the user into insert-state. The below code, together with the hooks ;; set in evil-collection-mu4e-setup fixes this issue. (defun evil-collection-mu4e-org-set-header-to-normal-mode () (evil-set-initial-state 'mu4e-compose-mode 'normal))

(defun evil-collection-mu4e-org-set-header-to-insert-mode () (evil-set-initial-state 'mu4e-compose-mode 'insert))

(defvar evil-collection-mu4e-mode-map-bindings `((mu4e-main-mode-map "J" mu4e~headers-jump-to-maildir "j" next-line "k" previous-line "u" mu4e-update-mail-and-index "gr" revert-buffer "b" mu4e-search-bookmark "B" mu4e-search-bookmark-edit "N" mu4e-news ";" mu4e-context-switch "H" mu4e-display-manual "C" mu4e-compose-new "cc" mu4e-compose-new "x" mu4e-kill-update-mail "A" mu4e-about "f" smtpmail-send-queued-mail "m" mu4e~main-toggle-mail-sending-mode "s" mu4e-search "q" mu4e-quit)

(mu4e-headers-mode-map
 "q" mu4e~headers-quit-buffer
 "J" mu4e~headers-jump-to-maildir
 "C" mu4e-compose-new
 "E" mu4e-compose-edit
 "F" mu4e-compose-forward
 "R" mu4e-compose-reply
 "cc" mu4e-compose-new
 "ce" mu4e-compose-edit
 "cf" mu4e-compose-forward
 "cr" mu4e-compose-reply
 "o" mu4e-headers-change-sorting
 "j" mu4e-headers-next
 "k" mu4e-headers-prev
 "gr" mu4e-search-rerun
 "b" mu4e-search-bookmark
 "B" mu4e-search-bookmark-edit
 ";" mu4e-context-switch
 ,(kbd "RET") mu4e-headers-view-message
 "/" mu4e-search-narrow
 "s" mu4e-search
 "S" mu4e-search-edit
 "x" mu4e-mark-execute-all
 "a" mu4e-headers-action
 "*" mu4e-headers-mark-for-something ; TODO: Don't override evil-seach-word-forward?
 "&" mu4e-headers-mark-custom
 "A" mu4e-headers-mark-for-action
 "m" mu4e-headers-mark-for-move
 "r" mu4e-headers-mark-for-refile
 "D" mu4e-headers-mark-for-delete
 "d" mu4e-headers-mark-for-trash
 "=" mu4e-headers-mark-for-untrash
 "u" mu4e-headers-mark-for-unmark
 "U" mu4e-mark-unmark-all
 "?" mu4e-headers-mark-for-unread
 "!" mu4e-headers-mark-for-read
 "%" mu4e-headers-mark-pattern
 "+" mu4e-headers-mark-for-flag
 "-" mu4e-headers-mark-for-unflag
 "[[" mu4e-headers-prev-unread
 "]]" mu4e-headers-next-unread
 "gk" mu4e-headers-prev-unread
 "gj" mu4e-headers-next-unread
 "\C-j" mu4e-headers-next
 "\C-k" mu4e-headers-prev
 "zr" mu4e-headers-toggle-include-related
 "zt" mu4e-headers-toggle-threading
 "zd" mu4e-headers-toggle-skip-duplicates
 "gl" mu4e-show-log
 "gv" mu4e-select-other-view
 "T" (lambda ()
       (interactive)
       (mu4e-headers-mark-thread nil '(read))))

(mu4e-compose-mode-map
 "gg" mu4e-compose-goto-top
 "G" mu4e-compose-goto-bottom
 "ZD" message-dont-send
 "ZF" mml-attach-file
 "ZQ" mu4e-message-kill-buffer
 "ZZ" message-send-and-exit)

(mu4e-search-minor-mode-map
 "J" mu4e-search-maildir)

(mu4e-view-mode-map
 " " mu4e-view-scroll-up-or-next
 [tab] shr-next-link
 [backtab] shr-previous-link
 "q" mu4e-view-quit
 "gx" mu4e-view-go-to-url
 "gX" mu4e-view-fetch-url
 "C" mu4e-compose-new
 "H" mu4e-view-toggle-html
 ;; "E"               mu4e-compose-edit
 ;; "F"               mu4e-compose-forward
 "R" mu4e-compose-reply
 "cc" mu4e-compose-new
 "ce" mu4e-compose-edit
 "cf" mu4e-compose-forward
 "cr" mu4e-compose-reply
 "p" mu4e-view-save-attachments
 "O" mu4e-headers-change-sorting
 "A" mu4e-view-mime-part-action ; Since 1.6, uses gnus view by default
 "a" mu4e-view-action
 "J" mu4e~headers-jump-to-maildir
 "[[" mu4e-view-headers-prev-unread
 "]]" mu4e-view-headers-next-unread
 "gk" mu4e-view-headers-prev-unread
 "gj" mu4e-view-headers-next-unread
 "\C-j" mu4e-view-headers-next
 "\C-k" mu4e-view-headers-prev
 "x" mu4e-view-marked-execute
 "&" mu4e-view-mark-custom
 "*" mu4e-view-mark-for-something   ; TODO: Don't override "*".
 "m" mu4e-view-mark-for-move
 "r" mu4e-view-mark-for-refile
 "D" mu4e-view-mark-for-delete
 "d" mu4e-view-mark-for-trash
 "=" mu4e-view-mark-for-untrash
 "u" mu4e-view-unmark
 "U" mu4e-view-unmark-all
 "?" mu4e-view-mark-for-unread
 "!" mu4e-view-mark-for-read
 "%" mu4e-view-mark-pattern
 "+" mu4e-view-mark-for-flag
 "-" mu4e-view-mark-for-unflag
 "zr" mu4e-headers-toggle-include-related
 "zt" mu4e-headers-toggle-threading
 "za" mu4e-view-toggle-hide-cited
 "gl" mu4e-show-log
 "s" mu4e-view-search-edit
 "|" mu4e-view-pipe
 "." mu4e-view-raw-message
 ,(kbd "C--") mu4e-headers-split-view-shrink
 ,(kbd "C-+") mu4e-headers-split-view-grow
 "T" (lambda ()
       (interactive)
       (mu4e-headers-mark-thread nil '(read)))
 ,@(when evil-want-C-u-scroll
     '("\C-u" evil-scroll-up))))

"All evil-mu4e bindings.")

(defun evil-collection-mu4e-set-bindings () "Set the bindings." ;; WARNING: With lexical binding, lambdas from mapc' anddolist' become ;; closures in which we must use evil-define-key*' instead of ;;evil-define-key'. (dolist (binding evil-collection-mu4e-mode-map-bindings) (apply #'evil-collection-define-key 'normal binding)) (evil-collection-define-key 'visual 'mu4e-compose-mode-map "gg" 'mu4e-compose-goto-top "G" 'mu4e-compose-goto-bottom) (evil-set-command-property 'mu4e-compose-goto-bottom :keep-visual t) (evil-set-command-property 'mu4e-compose-goto-top :keep-visual t)

;; yu (evil-collection-define-operator-key 'yank 'mu4e-view-mode-map "u" 'mu4e-view-save-url))

(defun evil-collection-mu4e-setup () "Initialize evil-mu4e if necessary. If mu4e-main-mode is in evil-state-motion-modes, initialization is already done earlier." (evil-collection-mu4e-set-state) (evil-collection-mu4e-set-bindings) ;;(add-hook 'org-mode-hook #'evil-collection-mu4e-org-set-header-to-normal-mode) ;;(add-hook 'mu4e-compose-pre-hook #'evil-collection-mu4e-org-set-header-to-insert-mode) )

(provide 'tx-mu4e-evil)

meliache commented 1 year ago

The fix provided by @theophilusx works well for me without modifications on the latest git build for mu4e 1.9.8 :+1: .

Once things settle and assuming I can find the time, I will submit a PR. However, that may be a ways off.

From the mailing list it appears like a new stable release is just a couple of weeks away, see Towards 1.10. I would still wait until the next stable release is actually out, but I'd advice to be prepared to make a PR once it is.

I'm on the development branch of mu4e and am currently using my personal fork of evil-collection where I just replaced the evil-collection mu4e module with the above code by @theophilusx , see the mu4e-development branch at https://github.com/meliache/evil-collection/tree/mu4e-development. The branch is not meant for a PR but for personal use. After the next stable release of mu4e I would wait if @theophilusx does a PR, as he authered the code, but if he doesn't I could also do it.

If we make a PR, a question would be whether we want to support multiple versions, as e.g. some linux distributions might take a while to update their mu4e version. We could do a version check and load an evil mu4e-module based on the release.

Though I think it might be better to keep things simple an just update the module to the latest recommended mu4e version, and get rid of all main menu modifications code, as it's complex and not needed anymore since 1.9. We might still support some keybindings for old mu4e versions, e.g. change which function is called by a binding depending on the version. But personally I think it's fine if the main menu isn't adapted for the old 1.8 versions, in my opinion that's not essential for mu4e to be usable and eventually users should update.

condy0919 commented 1 year ago

:+1:

abougouffa commented 1 year ago

After today's release of mu/mu4e 1.10, evil-collection-mu4e completely broke my config.

I've disabled the evil-collection-mu4e-update-main-view hook, as it seems to be the main reason. There are also some obsolete keybindings (like mu4e~view-quit-buffer) which needs to be updated.

johnhamelink commented 1 year ago

I found I had to mock the function mu4e--main-action-str to get to the point where I had an issue related to evil-collection-mu4e-update-main-view.

I just did this:

(use-package mu4e
  :init

  (defun mu4e--main-action-str (name func)
    "This seems to be needed until evil-collection supports the latest
  version of mu4e."
    "mu4e-main-action")

  (require 'mu4e)

  (remove-hook 'mu4e-main-mode-hook 'evil-collection-mu4e-update-main-view)

  ;; etc.
Jousimies commented 1 year ago

Wait for update, totally delete evil-collection-mu4e currently.

tshu-w commented 1 year ago

Just need to override evil-collection-mu4e-update-main-view without requiring mu4e:

  (defun mu4e--main-action-str (str &optional func-or-shortcut))
  (defun evil-collection-mu4e-update-main-view@override())
  (advice-add 'evil-collection-mu4e-update-main-view :override #'evil-collection-mu4e-update-main-view@override)
theophilusx commented 1 year ago

Tianshu Wang @.***> writes:

Just need to override evil-collection-mu4e-update-main-view without requiring mu4e:

(defun mu4e--main-action-str (str &optional func-or-shortcut)) (defun @.()) (advice-add 'evil-collection-mu4e-update-main-view :override @.)

or just see the attachment I posted earlier in this thread. You can just drop it in to replace the evil-collection module code and all will work.

jojojames commented 1 year ago

Once things settle and assuming I can find the time, I will submit a PR. However, that may be a ways off.

Feel free to submit a PR earlier and help everyone. There might be a lot of churn (additional PRs required) but I'll be happy to pull them in as is. I don't really have opinions on the mu4e bindings at the moment since I don't use it.

meliache commented 1 year ago

I opened the PR #720 with the changes from @theophilusx . It's still very raw, I didn't work on backwards-compatibility, and there is key overlap due to the new menu for toggling threading/duplicates/related. So help welcome. But I thought better start the PR than just discuss.

hokomo commented 1 year ago

I think the main issue that has to be resolved is to decide on the backwards compatibility strategy -- do we want to maintain backwards compatibility, and if so, how do we do it?

I've opened #672 before to fix evil-collection for mu/mu4e 1.6.x., and have asked (https://github.com/emacs-evil/evil-collection/pull/672#issuecomment-1232563009) whether adding version checks would be worthwhile.

Shortly after, I upgraded to mu/mu4e 1.8.x and most of the issues I had went away as the then-current version of evil-collection was mostly tracking 1.8.x anyway. I still discovered a few minor issues for which I had opened #675. Those fixes are still relevant and could be merged depending on how we decide to go about backwards compatibility.

Since mu4e is maintained and packaged together with mu itself, upgrading mu4e necessarily means upgrading mu as well, which depending on your distro might or might not be a simple thing to do. This is the main reason why backwards compatibility would be useful, rather than just tracking the latest version of mu/mu4e (which users might have to then install from source manually).

@jojojames mentioned creating a separate file for different versions of mu/mu4e (https://github.com/emacs-evil/evil-collection/pull/720#issuecomment-1496569812) to handle backwards compatibility. Another option are explicit version checks (within a single file) that I mentioned before. I'm honestly not sure which is preferable -- perhaps it depends on how big the changes are between the versions? Automatically detecting the version and choosing the appropriate configuration would be a convenient thing to do in any case (i.e. even in the one-file-per-version case).

If we decide to be backwards-compatible (which I think would be useful), we should probably just concentrate on starting with 1.8.x, as it has been the stable release for a while now.

bergheim commented 1 year ago

This is great. The separate file approach without a lot of version logic branching will be the cleanest and easiest to follow solution I think. Also, later changes that only affect new versions would not require any testing on older versions, as that file would be left alone.

milanglacier commented 1 year ago

729 should fix this issue.

Please leave feedback here about #729 and let's see if there are any other compatibility issue with mu 1.10 or mu 1.8.

milanglacier commented 1 year ago

@milanglacier something has changed between @meliache branch and this merge, because my key bindings are bypassed. I use doom emacs, with the bepo layout which rebinds the standard evil keys for the bepo keyboard. It was working correctly when I used @meliache’s branch, but now I have the standard evil key bindings in mu4e like ji to go to INBOX, or j and k to navigate in the mails. So something is bypassing the remapping of the bepo layout. Do you have any ideas of how to fix this ?

how about showing the git diff content of the branch you are using that has no conflict with your layout and the master branch?

I have no idea what’s happening here, since I never touched the keybindings code base, especially keybindings like j, i.

milanglacier commented 1 year ago

Unfortunately, the branch no longer exists. I was using this branch : (package! evil-collection :recipe (:repo "meliache/evil-collection" :branch "mu4e-development"))

I suspect that the current master branch loads the evil keybindings differently, so that the remapping is not effective.

this problem is hard to solve as it is hard to reproduce. Firstly you don’t have the branch anymore and you can’t reproduce the problem; secondly I don’t use bepo layout keyboard.

Have you ever tried “emacs -Q” and only load evil, evil-collection, and mu4e and see if the problem is still there?

edgar-vincent commented 1 year ago

@jojojames @condy0919 This issue can probably be closed.