[PATCH] rebase -i: abort cleanly if the editor fails to launch

[PATCH] rebase -i: abort cleanly if the editor fails to launch

[Björn Gustavsson]

If the user's configured editor is emacsclient, the editor
will fail to launch if emacs is not running and the git
command that tried to lanuch the editor will abort. For most
commands, all you have to do is to start emacs and repeat
the command.

The "git rebase -i" command, however, aborts without cleaning
the "$GIT_DIR/rebase-merge" directory if it fails to launch the
editor, so you'll need to do "git rebase --abort" before
repeating the rebase command.

Change "git rebase -i" to terminate using "die_abort" (instead of
with "die") if the initial launch of the editor fails.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
 git-rebase--interactive.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..d529328 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -779,7 +779,7 @@ EOF
 
 		cp "$TODO" "$TODO".backup
 		git_editor "$TODO" ||
-			die "Could not execute editor"
+			die_abort "Could not execute editor"
 
 		has_action "$TODO" ||
 			die_abort "Nothing to do"
-- 
1.6.6.rc3.dirty

Re: [PATCH] rebase -i: abort cleanly if the editor fails to launch

[Junio C Hamano]

Björn Gustavsson <bgustavsson@gmail.com> writes:

> If the user's configured editor is emacsclient, the editor
> will fail to launch if emacs is not running and the git
> command that tried to lanuch the editor will abort. For most
> commands, all you have to do is to start emacs and repeat
> the command.
>
> The "git rebase -i" command, however, aborts without cleaning
> the "$GIT_DIR/rebase-merge" directory if it fails to launch the
> editor, so you'll need to do "git rebase --abort" before
> repeating the rebase command.
>
> Change "git rebase -i" to terminate using "die_abort" (instead of
> with "die") if the initial launch of the editor fails.
>
> Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>

Two questions:

 - Is emacsclient the _only_ editor that can exit with non-zero status to
   signal an error condition "the user invoked me to edit an existing
   file, but I ended up not letting the user edit it"?

 - Are there editors that let you exit with non-zero status on demand?

The above is not a complaint to your log message, but is a genuine request
for information.

An editor that can exit with non-zero status on demand could use this
codepath to abort the rebase, when a user realizes that the edit s/he has
done so far was completely bogus [*1*].  In such a case, the existing
error message "Could not execute editor" may sound funny.

Regardless of that small issue, I think the patch to clean-up makes sense.

Thanks.


[Footnote]

*1* We interpret a completely empty $TODO as a signal from the user to
abort the sequencer (the literal interpretation of such a $TODO would be
to not pick any commits, which could be a source of potential confusion),
so it is not an issue in practice if your editor does not allow you to
exit with non-zero status to signal an exception.  You can remove all
lines and exit the editor normally instead.

Re: [PATCH] rebase -i: abort cleanly if the editor fails to launch

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1282 bytes --]

Hi,

On Sat, 19 Dec 2009, Junio C Hamano wrote:

> Björn Gustavsson <bgustavsson@gmail.com> writes:
> 
> > If the user's configured editor is emacsclient, the editor
> > will fail to launch if emacs is not running and the git
> > command that tried to lanuch the editor will abort. For most
> > commands, all you have to do is to start emacs and repeat
> > the command.
> >
> > The "git rebase -i" command, however, aborts without cleaning
> > the "$GIT_DIR/rebase-merge" directory if it fails to launch the
> > editor, so you'll need to do "git rebase --abort" before
> > repeating the rebase command.
> >
> > Change "git rebase -i" to terminate using "die_abort" (instead of
> > with "die") if the initial launch of the editor fails.
> >
> > Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
> 
> Two questions:
> 
>  - Is emacsclient the _only_ editor that can exit with non-zero status to
>    signal an error condition "the user invoked me to edit an existing
>    file, but I ended up not letting the user edit it"?

The non-existent editor also aborts with a non-zero status.

> An editor that can exit with non-zero status on demand could use this
> codepath to abort the rebase,

Removing all lines of the edit script will abort an interactive rebase.

Ciao,
Dscho

Re: [PATCH] rebase -i: abort cleanly if the editor fails to launch

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The non-existent editor also aborts with a non-zero status.

Thanks; that is a good point.  Perhaps we might want to say what editor we
tried to run in the error message.

>> An editor that can exit with non-zero status on demand could use this
>> codepath to abort the rebase,
>
> Removing all lines of the edit script will abort an interactive rebase.

I guess you replied too fast without reading the message to the end ;-)

Re: [PATCH] rebase -i: abort cleanly if the editor fails to launch

[Björn Gustavsson]

2009/12/19 Junio C Hamano <gitster@pobox.com>:

> Two questions:
>
>  - Is emacsclient the _only_ editor that can exit with non-zero status to
>   signal an error condition "the user invoked me to edit an existing
>   file, but I ended up not letting the user edit it"?

I have no idea, but my guess is that it would only happen in
exceptional circumstances (e.g. a file system that has run out
of space).

>  - Are there editors that let you exit with non-zero status on demand?
>
> The above is not a complaint to your log message, but is a genuine request
> for information.
>
> An editor that can exit with non-zero status on demand could use this
> codepath to abort the rebase, when a user realizes that the edit s/he has
> done so far was completely bogus [*1*].  In such a case, the existing
> error message "Could not execute editor" may sound funny.

Again, no idea, but I have never heard of such a feature. I think
that anyone advanced enough to use such a feature in an editor
would not be troubled by "Could not execute editor".


I mentioned emacsclient because it is a real annoyance
that has bitten me more than once (usually because I have
closed my local emacs while working on a remote computer
because I prefer to only keep windows that I am actively
working with), but my patch will help if the user has configured
a non-existent editor too (but such a problem usually
only happens once).

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB