* [PATCH] git-rebase: fix probable reflog typo
@ 2014-04-23 0:14 Felipe Contreras
2014-04-23 11:36 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Felipe Contreras @ 2014-04-23 0:14 UTC (permalink / raw)
To: git; +Cc: Ramkumar Ramachandra, Felipe Contreras
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.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-rebase--interactive.sh | 2 --
1 file changed, 2 deletions(-)
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?"
--
1.9.2+fc1.1.g5c924db
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] git-rebase: fix probable reflog typo
2014-04-23 0:14 [PATCH] git-rebase: fix probable reflog typo Felipe Contreras
@ 2014-04-23 11:36 ` Matthieu Moy
2014-04-23 21:22 ` Felipe Contreras
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2014-04-23 11:36 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
Felipe Contreras <felipe.contreras@gmail.com> 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/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] git-rebase: fix probable reflog typo
2014-04-23 11:36 ` Matthieu Moy
@ 2014-04-23 21:22 ` Felipe Contreras
2014-04-24 6:21 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Felipe Contreras @ 2014-04-23 21:22 UTC (permalink / raw)
To: Matthieu Moy, Felipe Contreras; +Cc: git, Ramkumar Ramachandra
Matthieu Moy wrote:
> Felipe Contreras <felipe.contreras@gmail.com> 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.
> 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.
Exactly. But if this variable is only meant for this command, it should be
`VAR=VAL command`, that would make it clear without the need of a comment.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-rebase: fix probable reflog typo
2014-04-23 21:22 ` Felipe Contreras
@ 2014-04-24 6:21 ` Matthieu Moy
2014-04-24 7:22 ` Felipe Contreras
0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2014-04-24 6:21 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Ramkumar Ramachandra
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Matthieu Moy wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> 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.
>
>> 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.
>
> Exactly. But if this variable is only meant for this command, it should be
> `VAR=VAL command`, that would make it clear without the need of a comment.
I don't understand. Are you suggesting to replace the shell function
"output" with an external command? If not, which command would you want
to call here?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-rebase: fix probable reflog typo
2014-04-24 6:21 ` Matthieu Moy
@ 2014-04-24 7:22 ` Felipe Contreras
2014-04-24 8:40 ` Matthieu Moy
0 siblings, 1 reply; 6+ messages in thread
From: Felipe Contreras @ 2014-04-24 7:22 UTC (permalink / raw)
To: Matthieu Moy, Felipe Contreras; +Cc: git, Ramkumar Ramachandra
Matthieu Moy wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> > Matthieu Moy wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> 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.
> >
> >> 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.
> >
> > Exactly. But if this variable is only meant for this command, it should be
> > `VAR=VAL command`, that would make it clear without the need of a comment.
>
> I don't understand. Are you suggesting to replace the shell function
> "output" with an external command? If not, which command would you want
> to call here?
Recently some code was changed to do 'test_must_fail env VAR=VAL command', why
can't we do the same?
--
Felipe Contreras
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-24 8:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-23 0:14 [PATCH] git-rebase: fix probable reflog typo Felipe Contreras
2014-04-23 11:36 ` Matthieu Moy
2014-04-23 21:22 ` Felipe Contreras
2014-04-24 6:21 ` Matthieu Moy
2014-04-24 7:22 ` Felipe Contreras
2014-04-24 8:40 ` Matthieu Moy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).