When cancelling an edit, why do we send USR2 instead of USR1?

[ackerleytng]

When I use magit over tramp (the sleeping editor gets used), I do a commit (c c) and then quit with (C-c C-k), and this ugly error appears:

error: There was a problem with the editor 'sh -c 'printf "\nWITH-EDITOR: $$ OPEN $0\037 IN $(pwd)\n"; sleep 604800 & sleep=$!; trap "kill $sleep; exit 0" USR1; trap "kill $sleep; exit 1" USR2; wait $sleep''.
Please supply the message using either -m or -F option.

I dug into this and was wondering why we signal USR2 at this line https://github.com/magit/with-editor/blob/cfcbc2731e402b9169c0dc03e89b5b57aa988502/lisp/with-editor.el#L393, when the user's intention is to cancel.

If the user's intention is to cancel, there should not be any error?

[tarsius]

I dug into this and was wondering why we signal USR2 [in when canceling and USR1 when finishing successfully]

Cancelling and finishing successfully are two distinct actions and we have to encode this distinction as two different signals so that we can later use it in the script that is the value of with-editor-sleeping-editor.

this ugly error appears

It is ugly, but there is nothing we can do about it. It is git that does this when the child exits with status 1. We have to exit with status 1 because if we use 0 then git would go a head and create the commit, which is exactly what we didn't want it to do. We could instead remove the file. git might or might not show the ugly error in that case. But even if it did show the error in that case, we wouldn't want to use this approach because we do want to keep the file, because if you later initiate another commit, the message from that file will be prefilled. If the message were completely discarded, we couldn't do that.

[ackerleytng]

What if we

1. Save the commit message file to a temporary buffer
2. Empty the commit message file
3. Exit without an error
4. Save the contents of the temporary buffer back to the commit message file.

This way, git will say "Aborting commit due to empty commit message", though.

Or perhaps a better fix is to override this particular error message on the magit side so the user isn't confused.

[tarsius]

This has come up a few times before, so I've decided to write a FAQ entry. It got a bit long, there aren't any other FAQ entries, and this does come up that often, so for now I am only posting it here:

** Canceling a commit results in errors; should I worry?

Canceling the creation of a commit, results in output similar to what
is shown below.  The exact output depends on what command was used to
initiate the commit and whether that was done using Magit or a shell.

#+begin_src example
  $ git commit
  hint: Waiting for your editor to close the file...
  Waiting for Emacs...
  *ERROR*: Canceled by user
  error: There was a problem with the editor '/usr/bin/emacsclient \
    --socket-name=/run/user/1000/emacs/server'.
  Please supply the message using either -m or -F option.
#+end_src

Of course canceling a commit is a perfectly reasonable thing to do.
When you do so, then ~with-editor~ has to somehow communicate that
decision to the process that invoked ~$EDITOR~.  It does so by
instructing the ~emacsclient~ to return with a non-zero exit status.

Additionally it prints the message =Canceled by user=.  Unfortunately
the ~emacsclient~ insists on prefixing that message with =*ERROR*:=.  ~git~
also feels the need to make some noise about it.  Please do not worry
about these error messages, they do not apply in this case.  You
canceled the creation of a commit and that is okay.  Nothing went
wrong in the process of doing so.

It has been suggested that instead of returning with a non-zero exit
status, we remove the file or its content.  In some cases we would
have to restore the file or its content, once ~git~ has returned.  In
other cases we very much would not want to do that.  Something along
these lines might work for ~git~, but implementing it would be a lot
harder than you likely imagine it to be and I am certain there would
be bugs.

Furthermore, ~with-editor~ isn't limited to ~git~ and Magit.  For other
commands it would be highly inappropriate to remove or empty the file.

The only reliable way to indicate to the caller that the user canceled
is to return with a non-zero exit status.  Most callers won't be able
to tell the difference between failure and an intentional abort, and
will choose to make some noise about it.  That misleading noise is the
price we have to pay for this feature to work reliably.

[tarsius]

Magit is now filtering out these bogus errors. See https://github.com/magit/magit/commit/fa0997797bb0965763f416a17415ec2d23a955ea.