threads / patch / 21990

patchrebase -i: abort cleanly if the editor fails to launch

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

## tl;dr

5 messages between Dec 19, 2009 and Dec 20, 2009. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Björn Gustavsson· Dec 19, 2009, 12:04 UTC · lore

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(-)

Show changes to git-rebase--interactive.sh +1 −1

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

Junio C Hamano· Dec 19, 2009, 20:44 UTC · re: Björn Gustavsson · lore

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

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

Show 15 quoted lines

> 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.

Johannes Schindelin· Dec 19, 2009, 20:57 UTC · re: Junio C Hamano · lore

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

Hi,

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

Show 23 quoted lines

> 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

Junio C Hamano· Dec 20, 2009, 07:24 UTC · re: Johannes Schindelin · lore

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

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 ;-)

Björn Gustavsson· Dec 20, 2009, 07:31 UTC · re: Junio C Hamano · lore

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

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

Show 5 quoted lines

> 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).

Show 9 quoted lines

>  - 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