Re: [PATCH] rebase -i: commit when continuing after "edit"

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 25, 2007, 13:50 UTC
Message-ID: <Pine.LNX.4.64.0709251439070.28395@racer.site>
In-Reply-To: <46F90C95.5060903@viscovery.net>

Hi,

On Tue, 25 Sep 2007, Johannes Sixt wrote:

Show 19 quoted lines

> Johannes Schindelin schrieb:
> > On Mon, 24 Sep 2007, Junio C Hamano wrote:
> > > >  do_next () {
> > > >  	test -f "$DOTEST"/message && rm "$DOTEST"/message
> > > >  	test -f "$DOTEST"/author-script && rm "$DOTEST"/author-script
> > > > +	test -f "$DOTEST"/amend && rm "$DOTEST"/amend
> > > As you do not check the error from "rm", how are these different from rm
> > > -f "$DOTEST/frotz"?
> > 
> > The difference: the user will not see many irritating error messages.
> > 
> > I changed this code to use a newly written function "remove_if_exists",
> > which die()s if the file exists and could not be removed.
> 
> Why? rm -f does nothing if the file does not exist, and fails if it cannot
> remove an existing file. It all boils down to:
> 
> 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
> 		"$DOTEST"/amend || exit

You're completely right. I somehow assumed that it would print an annoying message, but I was wrong.

BTW I am continually amazed at the ease of rebase -i to fix issues like these in a patch series. Thanks Eric!

Show 28 quoted lines

> > > >  			# This is like --amend, but with a different message
> > > >  			eval "$author_script"
> > > >  			export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
> > > > GIT_AUTHOR_DATE
> > > >  			$USE_OUTPUT git commit -F "$MSG" $EDIT_COMMIT
> > > >  			;;
> > > The "export" here makes me somewhat nervous -- no chance these
> > > leak into the next round?
> > 
> > I am somewhat wary: I get quoting wrong all the time.  Would
> > 
> > 	$USE_OUTPUT $author_script git commit -F "$MSG" $EDIT_COMMIT
> > 
> > work?  I have the slight suspicion that it would not, since
> > 
> > 	eval "$author_script"
> > 
> > needs extra quoting in $author_script, no?
> 
> How about:
> 
> 	eval "$author_script"
> 	GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
> 	GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
> 	GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
> 	$USE_OUTPUT git commit -F "$MSG" $EDIT_COMMIT
> 
> and if you dislike that, put the two questionable lines in parenthesis.

That looks ugly.  I'd rather have something like

	eval "$USE_OUTPUT $author_script git commit -F \"$MSG\" $EDIT_COMMIT"

but I'm not quite certain if that is enough, what with the funny characters people put into path names these days ($MSG points to "$DOTEST"/message).

BTW I just realised that the _same_ issue should have occurred in the "squash" case, but there I _forgot_ to export the environment variables.

Ciao, Dscho