Re: [PATCH] git-rebase: fix probable reflog typo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> Commit 26cd160 (rebase -i: use a better reflog message) tried to produce
> a better reflog message, however, it seems a statement was introduced by
> mistake.
>
> 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we
> just set.

I think this is more complex than this. To give a bit more context, the
code you're changing is:

comment_for_reflog start

if test ! -z "$switch_to"
then
	GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
	output git checkout "$switch_to" -- ||
	die "Could not checkout $switch_to"

	comment_for_reflog start
fi

The GIT_REFLOG_ACTION= changes the reflog message for the git checkout
command.

Since we use the construct

GIT_REFLOG_ACTION="..." shell_function

the shell may keep $GIT_REFLOG_ACTION set after calling the function (I
don't remember what POSIX says, but dash does it:
$ f () { echo $X; }
$ X=42 f
42
$ echo $X
42
$ X=y f               
y
$ echo $X
y
).

So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it
to be used later. However, it seems to me that the "comment_for_reflog
start" is used only for this checkout command. If so, there's no need
for the "comment_for_reflog start" before the if statement either.

In any case, this should be clarified with at least a comment in the
code.

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 43631b4..5f1d8c9 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -893,8 +893,6 @@ then
>  	GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
>  	output git checkout "$switch_to" -- ||
>  	die "Could not checkout $switch_to"
> -
> -	comment_for_reflog start
>  fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]