{"thread":{"id":"36478","subject":"[PATCH] git-rebase: fix probable reflog typo","startedAt":"2014-04-23T00:14:44Z","lastAt":"2014-04-24T08:40:38Z","messageCount":6,"participants":["Felipe Contreras","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"239403","messageId":"1398212084-16304-1-git-send-email-felipe.contreras@gmail.com","threadId":"36478","inReplyTo":null,"subject":"[PATCH] git-rebase: fix probable reflog typo","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-23T00:14:44Z","receivedAt":"2014-04-23T00:14:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Commit 26cd160 (rebase -i: use a better reflog message) tried to produce\na better reflog message, however, it seems a statement was introduced by\nmistake.\n\n'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we\njust set.\n\nSigned-off-by: Felipe Contreras \n---\n git-rebase--interactive.sh | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 43631b4..5f1d8c9 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -893,8 +893,6 @@ then\n \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n \toutput git checkout \"$switch_to\" -- ||\n \tdie \"Could not checkout $switch_to\"\n-\n-\tcomment_for_reflog start\n fi\n \n orig_head=$(git rev-parse --verify HEAD) || die \"No HEAD?\"\n-- \n1.9.2+fc1.1.g5c924db\n"},{"id":"239416","messageId":"vpqy4ywxoos.fsf@anie.imag.fr","threadId":"36478","inReplyTo":"1398212084-16304-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] git-rebase: fix probable reflog typo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-23T11:36:35Z","receivedAt":"2014-04-23T11:36:35Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras writes:\n\n> Commit 26cd160 (rebase -i: use a better reflog message) tried to produce\n> a better reflog message, however, it seems a statement was introduced by\n> mistake.\n>\n> 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we\n> just set.\n\nI think this is more complex than this. To give a bit more context, the\ncode you're changing is:\n\ncomment_for_reflog start\n\nif test ! -z \"$switch_to\"\nthen\n\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n\toutput git checkout \"$switch_to\" -- ||\n\tdie \"Could not checkout $switch_to\"\n\n\tcomment_for_reflog start\nfi\n\nThe GIT_REFLOG_ACTION= changes the reflog message for the git checkout\ncommand.\n\nSince we use the construct\n\nGIT_REFLOG_ACTION=\"...\" shell_function\n\nthe shell may keep $GIT_REFLOG_ACTION set after calling the function (I\ndon't remember what POSIX says, but dash does it:\n$ f () { echo $X; }\n$ X=42 f\n42\n$ echo $X\n42\n$ X=y f \ny\n$ echo $X\ny\n).\n\nSo, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it\nto be used later. However, it seems to me that the \"comment_for_reflog\nstart\" is used only for this checkout command. If so, there's no need\nfor the \"comment_for_reflog start\" before the if statement either.\n\nIn any case, this should be clarified with at least a comment in the\ncode.\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 43631b4..5f1d8c9 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -893,8 +893,6 @@ then\n> \tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n> \toutput git checkout \"$switch_to\" -- ||\n> \tdie \"Could not checkout $switch_to\"\n> -\n> -\tcomment_for_reflog start\n> fi\n> \n> orig_head=$(git rev-parse --verify HEAD) || die \"No HEAD?\"\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"239500","messageId":"53582f33385bc_24448772ec57@nysa.notmuch","threadId":"36478","inReplyTo":"vpqy4ywxoos.fsf@anie.imag.fr","subject":"Re: [PATCH] git-rebase: fix probable reflog typo","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-23T21:22:59Z","receivedAt":"2014-04-23T21:22:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Matthieu Moy wrote:\n> Felipe Contreras writes:\n> \n> > Commit 26cd160 (rebase -i: use a better reflog message) tried to produce\n> > a better reflog message, however, it seems a statement was introduced by\n> > mistake.\n> >\n> > 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we\n> > just set.\n\n> So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it\n> to be used later. However, it seems to me that the \"comment_for_reflog\n> start\" is used only for this checkout command. If so, there's no need\n> for the \"comment_for_reflog start\" before the if statement either.\n\nExactly. But if this variable is only meant for this command, it should be\n`VAR=VAL command`, that would make it clear without the need of a comment.\n\n-- \nFelipe Contreras\n"},{"id":"239538","messageId":"vpqzjjb1c4m.fsf@anie.imag.fr","threadId":"36478","inReplyTo":"53582f33385bc_24448772ec57@nysa.notmuch","subject":"Re: [PATCH] git-rebase: fix probable reflog typo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-24T06:21:13Z","receivedAt":"2014-04-24T06:21:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras writes:\n\n> Matthieu Moy wrote:\n>> Felipe Contreras writes:\n>> \n>> > Commit 26cd160 (rebase -i: use a better reflog message) tried to produce\n>> > a better reflog message, however, it seems a statement was introduced by\n>> > mistake.\n>> >\n>> > 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we\n>> > just set.\n>\n>> So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it\n>> to be used later. However, it seems to me that the \"comment_for_reflog\n>> start\" is used only for this checkout command. If so, there's no need\n>> for the \"comment_for_reflog start\" before the if statement either.\n>\n> Exactly. But if this variable is only meant for this command, it should be\n> `VAR=VAL command`, that would make it clear without the need of a comment.\n\nI don't understand. Are you suggesting to replace the shell function\n\"output\" with an external command? If not, which command would you want\nto call here?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"239541","messageId":"5358bb9ea7d42_1f7b143d310c6@nysa.notmuch","threadId":"36478","inReplyTo":"vpqzjjb1c4m.fsf@anie.imag.fr","subject":"Re: [PATCH] git-rebase: fix probable reflog typo","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-04-24T07:22:06Z","receivedAt":"2014-04-24T07:22:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Matthieu Moy wrote:\n> Felipe Contreras writes:\n> > Matthieu Moy wrote:\n> >> Felipe Contreras writes:\n> >> \n> >> > Commit 26cd160 (rebase -i: use a better reflog message) tried to produce\n> >> > a better reflog message, however, it seems a statement was introduced by\n> >> > mistake.\n> >> >\n> >> > 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we\n> >> > just set.\n> >\n> >> So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it\n> >> to be used later. However, it seems to me that the \"comment_for_reflog\n> >> start\" is used only for this checkout command. If so, there's no need\n> >> for the \"comment_for_reflog start\" before the if statement either.\n> >\n> > Exactly. But if this variable is only meant for this command, it should be\n> > `VAR=VAL command`, that would make it clear without the need of a comment.\n> \n> I don't understand. Are you suggesting to replace the shell function\n> \"output\" with an external command? If not, which command would you want\n> to call here?\n\nRecently some code was changed to do 'test_must_fail env VAR=VAL command', why\ncan't we do the same?\n\n-- \nFelipe Contreras\n"},{"id":"239546","messageId":"vpqiopzyvax.fsf@anie.imag.fr","threadId":"36478","inReplyTo":"5358bb9ea7d42_1f7b143d310c6@nysa.notmuch","subject":"Re: [PATCH] git-rebase: fix probable reflog typo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-04-24T08:40:38Z","receivedAt":"2014-04-24T08:40:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Felipe Contreras writes:\n\n> Recently some code was changed to do 'test_must_fail env VAR=VAL command', why\n> can't we do the same?\n\nI guess we can.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}