magit/with-editor

with-editor fails in term-mode.

Closed this issue · 10 comments

The change in commit 8d52f93 makes with-editor fails in term-mode.

What I expect when running:

  • M-x term
  • M-x with-editor-export-editor
  • git commit (assuming there are changes to commit).
    is that the editor opens.

What I observe is that the pressed keys are not printed anymore to the screen and the editor does not open but the string that should have been intercepted by with-editor is visible. On top of that, the editor does not open.

Note that it works fine with if M-x shell is used instead of M-x term. The regression is specific to the term-mode.

Repeating the same steps with 8e0c358c94f3649ec122fd03f5c8bc4602dce611, everything works fine.

Let me know if you need more details.

If it matters, I am using GNU Emacs 27.2.

I cannot reproduce this on 28.0.60. I tried all the supported terminal/shells. I haven't tried with 27.2.

I did some nasty force pushing when creating and fixing that commit. You might not actually use that commit if you installed from an *Elpa. Try uninstalling, restarting emacs and then reinstalling. That might fix it.

Thanks, I'll try to compile Emacs from source with both 27.2 and 28 to check that on my side.

Regarding Elpa, unfortunately I get with-editor from a company specific process and not from Elpa. So what I did is:

  • sync my clone of with-editor to 8e0c358
  • ran emacs -q with-editor.el
  • M-x eval-buffer
  • tested
  • sync my clone to 8d52f93
  • repeat the process

So I am fairly sure I used this revision if I made no mistake.

But I'll test with Emacs 28 first to rule this out.

I just rebuild GNU Emacs 28.0.60 (build 1, x86_64-pc-linux-gnu) from source (rev 7e2b973d60cfd30f1828fabd8d9f33127f24e54a of the Emacs's Git) and I reproduce the issue with with-editor revision 8d52f93.

To be cristal clear, the steps are:

  • ~/devel/emacs-28/builddir/src/emacs -Q ~/devel/with-editor/with-editor.el
  • M-x eval-buffer
  • M-x term; validate the default value which for me is /bin/bash
  • C-c M-x with-editor-export-editor

After those steps; keys pressed don't echo on screen (they do if I don't run with-editor-export-editor of course).

If I then type (without seeing the typed command since the keys are not echoed anymore):

  • git commit (assuming there is a change to commit in the current directory)

I get (I redacted my personal data):

$ git status                                                                                                           
On branch master                                                                                                                                                      
Your branch is up to date with 'origin/master'.                                                                                                                       
                                                                                                                                                                      
Changes to be committed:                                                                                                                                              
  (use "git restore --staged <file>..." to unstage)                                                                                                                   
        modified:   with-editor.el                                                                                                                                    
                                                                                                                                                                      
$  export EDITOR=sh\ -c\ \'printf\ \"WITH-EDITOR\:\ \$\$\ OPEN\ \$0\\037\ IN\ \$\(pwd\)\\n\"\;\ sleep\ 604800\ \&\ sle 
ep\=\$\!\;\ trap\ \"kill\ \$sleep\;\ exit\ 0\"\ USR1\;\ trap\ \"kill\ \$sleep\;\ exit\ 1\"\ USR2\;\ wait\ \$sleep\'                                                   
$ WITH-EDITOR: 1923739 OPEN /.../devel/with-editor/.git/COMMIT_EDITMSG IN /.../devel/with-editor                                                                                                                                       

As you can see the git commit is not visible in the buffer since there is no echo anymore after export EDITOR.

To complete the previous comment I redid the test and added commands to dump the exact version of with-editor I am using:

<redacted>$ git rev-parse HEAD
8d52f933e50624c7bca3880f57297ac17ba4ac2d
<redacted>$ git --no-pager diff --staged
diff --git a/with-editor.el b/with-editor.el
index ba856e6..8c1a3a1 100644
--- a/with-editor.el
+++ b/with-editor.el
@@ -1,3 +1,4 @@
+
 ;;; with-editor.el --- Use the Emacsclient as $EDITOR -*- lexical-binding: t -*-                                                                         

 ;; Copyright (C) 2014-2021  The Magit Project Contributors                                                                                               
<redacted>$  export EDITOR=sh\ -c\ \'printf\ \"WITH-EDITOR\:\ \$\$\ OPEN\ \$0\\037\ IN\ \$\(pwd\)\\n\"\;\ sleep\ 604800\ \&\ sleep\=\$\!\;\ trap\ \"kill\\
 \$sleep\;\ exit\ 0\"\ USR1\;\ trap\ \"kill\ \$sleep\;\ exit\ 1\"\ USR2\;\ wait\ \$sleep\'
<redacted>$ WITH-EDITOR: 1950807 OPEN /<redacted>/devel/with-editor/.git/COMMIT_EDITMSG^_ IN /<redacted>/devel/with-editor

The staged diff here is just a blank line at the beginning of with-editor.el so that git commit has something to try to commit.

I can reproduce it now too. Maybe I did something wrong before.

That (plus another term specific issue) should be fixed now.

(I recommend you use vterm instead.)

Thanks for the quick fix.

With the latest change (521f75e) though I get an error with the same scenario:

Debugger entered--Lisp error: (void-variable buffer)
  (set-buffer buffer)
  (save-current-buffer (set-buffer buffer) (message (substitute-command-keys with-editor-usage-message)))
  (lambda nil (save-current-buffer (set-buffer buffer) (message (substitute-command-keys with-editor-usage-message))))()
  apply((lambda nil (save-current-buffer (set-buffer buffer) (message (substitute-command-keys with-editor-usage-message)))) nil)
  timer-event-handler([t 24955 47226 116699 nil (lambda nil (save-current-buffer (set-buffer buffer) (message (substitute-command-keys with-editor-usage-message)))) nil nil 474000])

This error does not prevent the editor from opening and both C-c C-c and C-c C-k seem to work fine (but the help message that shows these shortcuts is not displayed; instead we see the error).

Do you also see this error on your side?

Somehow this code:

(defun with-editor-usage-message ()
  ;; Run after `server-execute', which is run using
  ;; a timer which starts immediately.
  (let ((buffer (current-buffer)))
    (run-with-timer
     0.05 nil
     (lambda ()
       (with-current-buffer buffer
         (message (substitute-command-keys with-editor-usage-message)))))))

ended up being compiled without lexical-binding (a local variable which is being set on the first-line of the file). lambda should not appear in the backtrace but closure (which captures to buffer). I don't know how that happened but restarting Emacs should fix it. Maybe the reinstallation dance again.

Oh I am really sorry but that is my fault.

To test with-editor.el I made a change to this file and try to git commit it... but the change I did was adding a blank line at the beginning of the file. This means that the lexical-binding was not on the first line anymore and thus got ignored.

Please accept my apologies, I hope you did not spent too much time on that one.

I can now confirm it works perfectly fine.