Enabling explain-pause-mode causes committing via magit/with-editor over TRAMP to stop working (Root cause: `with-editor` checks for itself with `process-filter`, which fails because we wrapped it)

[tomfitzhenry]

Steps to reproduce

explain-pause-mode: version 318dace
magit: magit-20200108.532
emacs: 26.3
NixOS 20.03

Set up a git repo on a remote machine:

SERVER=srv
ssh $SERVER "mkdir tmp-git && cd tmp-git && git init

Now try to commit to that repo via magit over TRAMP, when explain-pause-mode is called.

(add-to-list 'load-path "~/.emacs.d/lisp/")

(require 'package)
(require 'explain-pause-mode)
(explain-pause-mode t)
(require 'magit)

(magit-status "/ssh:$SERVER:tmp-git")

To commit, switch to the "magit: tmp-git" buffer, and hit "c c"

Expected

magit's mode for writing commit message is displayed.

Actual

magit's mode for writing commit message is not displayed. The current buffer is still "magit: tmp-git". There is a buffer named "magit-process: git-tmp" which is typically read by with-editor's process filter (which doesn't get executed, when explain-pause-mode is enabled).

Some diagnosis

If I don't execute explain-pause-mode, then I get the expected behaviour.

Attempt at figuring root cause

Using edebug-defun I set edebug to trace calls to explain--wrap-set-process-filter-callback and with-editor-process-filter .

When explain-pause-mode is enabled, I observe only is executed explain--wrap-set-process-filter-callback (notably, with-editor-process-filter is not executed). I expect with-editor-process-filter to also execute, however.

When explain-pause-mode is disabled, I observe with-editor-process-filter is executed, as expected.

More context

#44 (comment)
https://github.com/tomfitzhenry/dotfiles/commit/09d05c4d16a9421b9cb8b9efde550f4549fec4e0

[tomfitzhenry]

I am able to reproduce this issue with b667822 .

[lastquestion]

Hey!! @tomfitzhenry thank you so much for going the extra mile and checking out #42 and seeing what happens.

disappointedpikachu.jpg 😞

I'll investigate this today. I was really hoping I got this fixed, but I must have missed something. with-editor is a really good stress test, and is used by magit aka a lot of people, so I expect to add an regression test suite for whatever with-editor is doing...

[lastquestion]

Repro:

(setq proc (make-process
            :name "foo"
            :command '("cat")
            :buffer nil
            :filter nil))

(defun my-filter (process string)
  t)

(set-process-filter proc 'my-filter)

(eq (process-filter proc) 'my-filter)
nil

[lastquestion]

I'm going to fix this in the "development branch" #42. If you are OK with using that branch (I'm pretty close to merging it), I won't bother backporting this fix to master. But if you need it, it's not too much effort, let me know if you need that. ^_^

[lastquestion]

The code in with-editor: https://github.com/magit/with-editor/blob/master/with-editor.el#L625-L640

One thing I notice is, that you can't call with-editor-set-process-filter twice, even normally, because the second time, the lambda doesn't eq with-editor-process-filter anymore. So anyway I think this could be written differently in with-editor.

[lastquestion]

So somehow github got all screwed up yesterday night. I had to repush and manually mark. Anyway, the commit above will fix your issue :_) I tested via TRAMP and SSH. It's a feature I very very rarely use... so if you use it often, please let me know if you find more bugs in this area!

[tomfitzhenry]

Anyway, the commit above will fix your issue :_) I tested via TRAMP and SSH. It's a feature I very very rarely use... so if you use it often, please let me know if you find more bugs in this area!

Great, this fixes the issue! Thanks a lot!