Re: [PATCH] git-rebase: fix probable reflog typo
Felipe Contreras felipe.contre...@gmail.com writes: Matthieu Moy wrote: Felipe Contreras felipe.contre...@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/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Matthieu Moy wrote: Felipe Contreras felipe.contre...@gmail.com writes: Matthieu Moy wrote: Felipe Contreras felipe.contre...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Felipe Contreras felipe.contre...@gmail.com writes: Recently some code was changed to do 'test_must_fail env VAR=VAL command', why can't we do the same? I guess we can. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Felipe Contreras felipe.contre...@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: fix probable reflog typo
Matthieu Moy wrote: Felipe Contreras felipe.contre...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-rebase: fix probable reflog typo
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.contre...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
