evil-collection-diff-mode breaks magit-commit

[Deewiant]

The way in which evil-collection-diff-mode unconditionally enables read-only-mode breaks committing in Magit (similarly to the way diff-default-read-only used to: magit/magit#3170).

Should be easy to reproduce by attempting to commit anything with Magit after calling evil-collection-init but here are from-scratch reproduction instructions anyway, starting in a terminal:

$ mkdir test
$ cd test
$ git init
$ cat > file
(package-initialize)
(setq evil-want-integration nil)
(require 'evil)
(evil-mode 1)
(require 'evil-collection)
(evil-collection-init 'diff-mode)
(require 'magit)
$ git add file
$ emacs -Q file

And now in emacs:

M-: (eval-buffer)
M-x magit
; (EDIT: updated this to include --verbose)
; Call magit-commit with --verbose to trigger the error
c -v c
; At this point, instead of the commit message buffer opening properly there's an error,
; so let's view the process buffer to see it:
$

At this point the process buffer should look like:

  1 git … commit --
hint: Waiting for your editor to close the file...
Waiting for Emacs...
*ERROR*: Buffer is read-only: #<killed buffer>
error: There was a problem with the editor '/bin/emacsclient --socket-name=/tmp/emacs1000/server9279'.
Please supply the message using either -m or -F option.

Which pointed me to magit/magit#3170. The fix there — magit/magit@746f236 — overrides diff-default-read-only but that's not something that affects the behaviour of evil-collection-diff-mode in any way. Maybe it should? I could also see this being considered as another Magit bug but I'm not sure if it's reasonable for Magit to detect an arbitrary hook that sets read-only-mode.

[jojojames]

Thanks for the awesome report! I'll leave @Ambrevar for this one as I think he's the expert on diff-mode. I'll give it a shot to reproduce at the very least once I have some time.

Any suggestions or PRs would work too. :)

[Ambrevar]

I'll look into it soon, thanks for reporting!

[Ambrevar]

I could not reproduce but you've highlighted a weakness in the read-only switch.
I've added a check so that only diff-mode is affected.

@Deewiant: Let me know if that works for you.
By the way, you can use emacs -Q -l file to save the M-: (eval-buffer) step :)

[Deewiant]

@Ambrevar I'm afraid it doesn't fix the issue, but I think I know why you couldn't reproduce it. My mistake for not properly checking vanilla git configuration — I have commit.verbose = true enabled and that's apparently key. I updated the reproduction instructions to call magit-commit with c -v c instead of c c, thereby doing git commit --verbose under the hood.

Another thing I forgot to mention is version numbers. I'm running the latest stuff from MELPA so M-x magit-version says Magit 20180620.1224, Git 2.18.0, Emacs 26.1, gnu/linux, while evil-collection is at 20180625.642 (plus that minor patch to evil-collection-diff-read-only-state-switch which apparently didn't make it to MELPA yet).

(Thanks for the tip about emacs -l, didn't know about that.)

[Ambrevar]

OK, I could reproduce this time.
I've decided not to switch to read-only mode by default. See 5affbdb. This is how vanilla Emacs behave, so it should work. Let me know if you can come up with a better fix.

[Deewiant]

Thanks, that indeed works. I think it makes sense for evil-collection to stick with that and let read-onlyness be handled by other things like the vanilla diff-default-read-only setting (which Magit also understands).