Hi,
On Tue, 25 Sep 2007, Johannes Sixt wrote:
> 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!
> > > > # 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
