{"thread":{"id":"37039","subject":"[PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","startedAt":"2014-07-02T08:51:53Z","lastAt":"2014-07-03T14:08:33Z","messageCount":7,"participants":["Avi Kivity","Torsten Bögershausen","Fabian Ruch","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"245285","messageId":"1404291113-4424-1-git-send-email-avi@cloudius-systems.com","threadId":"37039","inReplyTo":null,"subject":"[PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Avi Kivity","fromEmail":"avi@cloudius-systems.com","sentAt":"2014-07-02T08:51:53Z","receivedAt":"2014-07-02T08:51:53Z","isPatch":true,"sender":{"key":"avi@cloudius-systems.com","avatar":null},"body":"Some workflows prefer to track exactly which email message was used to\ngenerate a commit.  This can be used, for example, to generate an\nautomated acknowledgement when a patch is committed as a response to\nthe patch email, or as a reference to the thread which introduced the\npatch.\n\nSupport this by adding a --message-id option (abbreviated as -m) to\ngit-am, which will then extract the message ID and append it to the\nemail commit log.\n\nSigned-off-by: Avi Kivity <avi@cloudius-systems.com>\n---\n\nv2: adjust to pass test suite (t5100)\n\n Documentation/git-am.txt |  6 ++++++\n builtin/mailinfo.c       |  2 +-\n git-am.sh                | 10 +++++++++-\n t/t5100/info0004         |  1 +\n t/t5100/info0005         |  1 +\n t/t5100/info0012         |  1 +\n 6 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 9adce37..8a251a1 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -15,6 +15,7 @@ SYNOPSIS\n \t [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]\n \t [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]\n \t [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]\n+\t [--message-id]\n \t [(<mbox> | <Maildir>)...]\n 'git am' (--continue | --skip | --abort)\n \n@@ -121,6 +122,11 @@ default.   You can use `--no-utf8` to override this.\n \tuser to lie about the author date by using the same\n \tvalue as the committer date.\n \n+-m::\n+--message-id::\n+\tExtract the Message-Id: header from the e-mail and\n+\tappend it to the commit message's tag stanza.\n+\n --skip::\n \tSkip the current patch.  This is only meaningful when\n \trestarting an aborted patch.\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex cf11c8d..f1e1fed 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)\n \n static void decode_header(struct strbuf *line);\n static const char *header[MAX_HDR_PARSED] = {\n-\t\"From\",\"Subject\",\"Date\",\n+\t\"From\",\"Subject\",\"Date\",\"Message-Id\"\n };\n \n static inline int cmp_header(const struct strbuf *line, const char *hdr)\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..c0e7bdd 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -39,6 +39,7 @@ committer-date-is-author-date    lie about committer date\n ignore-date     use current timestamp for author date\n rerere-autoupdate update the index with reused conflict resolution if possible\n S,gpg-sign?     GPG-sign commits\n+m,message-id    copy the Message-Id: header to the commit's tag stanza\n rebasing*       (internal use for git-rebase)\"\n \n . git-sh-setup\n@@ -371,7 +372,7 @@ split_patches () {\n prec=4\n dotest=\"$GIT_DIR/rebase-apply\"\n sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=\n-resolvemsg= resume= scissors= no_inbody_headers=\n+resolvemsg= resume= scissors= no_inbody_headers= message_id=\n git_apply_opt=\n committer_date_is_author_date=\n ignore_date=\n@@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.\"\n \t\tgpg_sign_opt=-S ;;\n \t--gpg-sign=*)\n \t\tgpg_sign_opt=\"-S${1#--gpg-sign=}\" ;;\n+\t-m|--message-id)\n+\t\tmessage_id=t ;;\n \t--)\n \t\tshift; break ;;\n \t*)\n@@ -565,6 +568,7 @@ Use \\\"git am --abort\\\" to remove it.\")\"\n \techo \" $git_apply_opt\" >\"$dotest/apply-opt\"\n \techo \"$threeway\" >\"$dotest/threeway\"\n \techo \"$sign\" >\"$dotest/sign\"\n+\techo \"$message_id\" > \"$dotest/message-id\"\n \techo \"$utf8\" >\"$dotest/utf8\"\n \techo \"$keep\" >\"$dotest/keep\"\n \techo \"$scissors\" >\"$dotest/scissors\"\n@@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n \t\tthen\n \t\t\tcat \"$dotest/msg-clean\"\n \t\tfi\n+\t\tif test 't' == \"$message_id\"\n+\t\tthen\n+\t\t\tgrep ^Message-Id: \"$dotest/info\" || true\n+\t\tfi\n \t\tif test '' != \"$ADD_SIGNOFF\"\n \t\tthen\n \t\t\techo \"$ADD_SIGNOFF\"\ndiff --git a/t/t5100/info0004 b/t/t5100/info0004\nindex 616c309..f7e2983 100644\n--- a/t/t5100/info0004\n+++ b/t/t5100/info0004\n@@ -2,4 +2,5 @@ Author: YOSHIFUJI Hideaki / 吉藤英明\n Email: yoshfuji@linux-ipv6.org\n Subject: GIT: Try all addresses for given remote name\n Date: Thu, 21 Jul 2005 09:10:36 -0400 (EDT)\n+Message-Id: <20050721.091036.01119516.yoshfuji@linux-ipv6.org>\n \ndiff --git a/t/t5100/info0005 b/t/t5100/info0005\nindex 46a46fc..592388f 100644\n--- a/t/t5100/info0005\n+++ b/t/t5100/info0005\n@@ -2,4 +2,5 @@ Author: David Kågedal\n Email: davidk@lysator.liu.se\n Subject: Fixed two bugs in git-cvsimport-script.\n Date: Mon, 15 Aug 2005 20:18:25 +0200\n+Message-Id: <u5tacjjdpxq.fsf@lysator.liu.se>\n \ndiff --git a/t/t5100/info0012 b/t/t5100/info0012\nindex ac1216f..b5d89a1 100644\n--- a/t/t5100/info0012\n+++ b/t/t5100/info0012\n@@ -2,4 +2,5 @@ Author: Dmitriy Blinov\n Email: bda@mnsspb.ru\n Subject: Изменён список пакетов необходимых для сборки\n Date: Wed, 12 Nov 2008 17:54:41 +0300\n+Message-Id: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>\n \n-- \n1.9.3\n"},{"id":"245288","messageId":"53B3D7AB.6090207@web.de","threadId":"37039","inReplyTo":"1404291113-4424-1-git-send-email-avi@cloudius-systems.com","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-07-02T09:58:03Z","receivedAt":"2014-07-02T09:58:03Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -39,6 +39,7 @@ committer-date-is-author-date    lie about committer date\n>  ignore-date     use current timestamp for author date\n>  rerere-autoupdate update the index with reused conflict resolution if possible\n>  S,gpg-sign?     GPG-sign commits\n> +m,message-id    copy the Message-Id: header to the commit's tag stanza\n>  rebasing*       (internal use for git-rebase)\"\n>  \n>  . git-sh-setup\n> @@ -371,7 +372,7 @@ split_patches () {\n>  prec=4\n>  dotest=\"$GIT_DIR/rebase-apply\"\n>  sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=\n> -resolvemsg= resume= scissors= no_inbody_headers=\n> +resolvemsg= resume= scissors= no_inbody_headers= message_id=\n>  git_apply_opt=\n>  committer_date_is_author_date=\n>  ignore_date=\n> @@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.\"\n>  \t\tgpg_sign_opt=-S ;;\n>  \t--gpg-sign=*)\n>  \t\tgpg_sign_opt=\"-S${1#--gpg-sign=}\" ;;\n> +\t-m|--message-id)\n> +\t\tmessage_id=t ;;\n>  \t--)\n>  \t\tshift; break ;;\n>  \t*)\n> @@ -565,6 +568,7 @@ Use \\\"git am --abort\\\" to remove it.\")\"\n>  \techo \" $git_apply_opt\" >\"$dotest/apply-opt\"\n>  \techo \"$threeway\" >\"$dotest/threeway\"\n>  \techo \"$sign\" >\"$dotest/sign\"\n> +\techo \"$message_id\" > \"$dotest/message-id\"\n>  \techo \"$utf8\" >\"$dotest/utf8\"\n>  \techo \"$keep\" >\"$dotest/keep\"\n>  \techo \"$scissors\" >\"$dotest/scissors\"\n> @@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n>  \t\tthen\n>  \t\t\tcat \"$dotest/msg-clean\"\n>  \t\tfi\n> +\t\tif test 't' == \"$message_id\"\n\nThe == is bash special, please use = instead\nAnd the 't' can be written as t, so that the whole line looks like this:\n\t\tif test t = \"$message_id\"\n"},{"id":"245292","messageId":"53B40F04.8000304@cloudius-systems.com","threadId":"37039","inReplyTo":"53B3D7AB.6090207@web.de","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Avi Kivity","fromEmail":"avi@cloudius-systems.com","sentAt":"2014-07-02T13:54:12Z","receivedAt":"2014-07-02T13:54:12Z","isPatch":true,"sender":{"key":"avi@cloudius-systems.com","avatar":null},"body":"\nOn 07/02/2014 12:58 PM, Torsten Bögershausen wrote:\n>> @@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n>>   \t\tthen\n>>   \t\t\tcat \"$dotest/msg-clean\"\n>>   \t\tfi\n>> +\t\tif test 't' == \"$message_id\"\n> The == is bash special, please use = instead\n> And the 't' can be written as t, so that the whole line looks like this:\n> \t\tif test t = \"$message_id\"\n>\n\nThanks.  v3 posted with these changes.\n"},{"id":"245294","messageId":"53B414A4.2060909@gmail.com","threadId":"37039","inReplyTo":"1404291113-4424-1-git-send-email-avi@cloudius-systems.com","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-07-02T14:18:12Z","receivedAt":"2014-07-02T14:18:12Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"Hi Avi,\n\nOn 07/02/2014 10:51 AM, Avi Kivity wrote:\n> Some workflows prefer to track exactly which email message was used to\n> generate a commit.  This can be used, for example, to generate an\n> automated acknowledgement when a patch is committed as a response to\n> the patch email, or as a reference to the thread which introduced the\n> patch.\n> \n> Support this by adding a --message-id option (abbreviated as -m) to\n> git-am, which will then extract the message ID and append it to the\n> email commit log.\n> \n> Signed-off-by: Avi Kivity <avi@cloudius-systems.com>\n> ---\n> \n> v2: adjust to pass test suite (t5100)\n> \n>  Documentation/git-am.txt |  6 ++++++\n>  builtin/mailinfo.c       |  2 +-\n>  git-am.sh                | 10 +++++++++-\n>  t/t5100/info0004         |  1 +\n>  t/t5100/info0005         |  1 +\n>  t/t5100/info0012         |  1 +\n>  6 files changed, 19 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\n> index 9adce37..8a251a1 100644\n> --- a/Documentation/git-am.txt\n> +++ b/Documentation/git-am.txt\n> @@ -15,6 +15,7 @@ SYNOPSIS\n>  \t [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]\n>  \t [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]\n>  \t [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]\n> +\t [--message-id]\n>  \t [(<mbox> | <Maildir>)...]\n>  'git am' (--continue | --skip | --abort)\n>  \n> @@ -121,6 +122,11 @@ default.   You can use `--no-utf8` to override this.\n>  \tuser to lie about the author date by using the same\n>  \tvalue as the committer date.\n>  \n> +-m::\n> +--message-id::\n> +\tExtract the Message-Id: header from the e-mail and\n> +\tappend it to the commit message's tag stanza.\n> +\n>  --skip::\n>  \tSkip the current patch.  This is only meaningful when\n>  \trestarting an aborted patch.\n> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\n> index cf11c8d..f1e1fed 100644\n> --- a/builtin/mailinfo.c\n> +++ b/builtin/mailinfo.c\n> @@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)\n>  \n>  static void decode_header(struct strbuf *line);\n>  static const char *header[MAX_HDR_PARSED] = {\n> -\t\"From\",\"Subject\",\"Date\",\n> +\t\"From\",\"Subject\",\"Date\",\"Message-Id\"\n>  };\n>  \n>  static inline int cmp_header(const struct strbuf *line, const char *hdr)\n> diff --git a/git-am.sh b/git-am.sh\n> index ee61a77..c0e7bdd 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -39,6 +39,7 @@ committer-date-is-author-date    lie about committer date\n>  ignore-date     use current timestamp for author date\n>  rerere-autoupdate update the index with reused conflict resolution if possible\n>  S,gpg-sign?     GPG-sign commits\n> +m,message-id    copy the Message-Id: header to the commit's tag stanza\n>  rebasing*       (internal use for git-rebase)\"\n>  \n>  . git-sh-setup\n> @@ -371,7 +372,7 @@ split_patches () {\n>  prec=4\n>  dotest=\"$GIT_DIR/rebase-apply\"\n>  sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=\n> -resolvemsg= resume= scissors= no_inbody_headers=\n> +resolvemsg= resume= scissors= no_inbody_headers= message_id=\n>  git_apply_opt=\n>  committer_date_is_author_date=\n>  ignore_date=\n> @@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.\"\n>  \t\tgpg_sign_opt=-S ;;\n>  \t--gpg-sign=*)\n>  \t\tgpg_sign_opt=\"-S${1#--gpg-sign=}\" ;;\n> +\t-m|--message-id)\n> +\t\tmessage_id=t ;;\n\nDoesn't the message-id line in OPTIONS_SPEC make the negated long\noption --no-message-id available as well? If that's the case, then\nthe corresponding case arm is missing from here.\n\n>  \t--)\n>  \t\tshift; break ;;\n>  \t*)\n> @@ -565,6 +568,7 @@ Use \\\"git am --abort\\\" to remove it.\")\"\n>  \techo \" $git_apply_opt\" >\"$dotest/apply-opt\"\n>  \techo \"$threeway\" >\"$dotest/threeway\"\n>  \techo \"$sign\" >\"$dotest/sign\"\n> +\techo \"$message_id\" > \"$dotest/message-id\"\n\nTo match the local style conventions, the space character after the\nredirection operator should be removed.\n\nAlso, isn't the patch missing the bits where the state of message-id\nis read? Like so:\n\n    if test \"$(cat \"$dotest/message-id\")\" = t\n    then\n    \tmessage_id=t\n    fi\n\n>  \techo \"$utf8\" >\"$dotest/utf8\"\n>  \techo \"$keep\" >\"$dotest/keep\"\n>  \techo \"$scissors\" >\"$dotest/scissors\"\n> @@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n>  \t\tthen\n>  \t\t\tcat \"$dotest/msg-clean\"\n>  \t\tfi\n> +\t\tif test 't' == \"$message_id\"\n> +\t\tthen\n> +\t\t\tgrep ^Message-Id: \"$dotest/info\" || true\n\nWhy is the true guard needed here? The exit status of grep seems to\nnever be checked.\n\nAlthough I cannot come up with an example where this would matter,\nyou might want to consider using the grep wrapper sane_grep from\ngit-sh-setup.sh instead. It resets the environment variable\nGREP_OPTIONS before calling grep so that no unexpected user options\ncome into play.\n\n> +\t\tfi\n>  \t\tif test '' != \"$ADD_SIGNOFF\"\n>  \t\tthen\n>  \t\t\techo \"$ADD_SIGNOFF\"\n> \n> [..]\n\n   Fabian\n"},{"id":"245296","messageId":"53B41AC4.5000308@cloudius-systems.com","threadId":"37039","inReplyTo":"53B414A4.2060909@gmail.com","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Avi Kivity","fromEmail":"avi@cloudius-systems.com","sentAt":"2014-07-02T14:44:20Z","receivedAt":"2014-07-02T14:44:20Z","isPatch":true,"sender":{"key":"avi@cloudius-systems.com","avatar":null},"body":"\nOn 07/02/2014 05:18 PM, Fabian Ruch wrote:\n> Hi Avi,\n>\n> On 07/02/2014 10:51 AM, Avi Kivity wrote:\n>> Some workflows prefer to track exactly which email message was used to\n>> generate a commit.  This can be used, for example, to generate an\n>> automated acknowledgement when a patch is committed as a response to\n>> the patch email, or as a reference to the thread which introduced the\n>> patch.\n>>\n>> Support this by adding a --message-id option (abbreviated as -m) to\n>> git-am, which will then extract the message ID and append it to the\n>> email commit log.\n>>\n>> Signed-off-by: Avi Kivity <avi@cloudius-systems.com>\n>> ---\n>>\n>> v2: adjust to pass test suite (t5100)\n>>\n>>   Documentation/git-am.txt |  6 ++++++\n>>   builtin/mailinfo.c       |  2 +-\n>>   git-am.sh                | 10 +++++++++-\n>>   t/t5100/info0004         |  1 +\n>>   t/t5100/info0005         |  1 +\n>>   t/t5100/info0012         |  1 +\n>>   6 files changed, 19 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\n>> index 9adce37..8a251a1 100644\n>> --- a/Documentation/git-am.txt\n>> +++ b/Documentation/git-am.txt\n>> @@ -15,6 +15,7 @@ SYNOPSIS\n>>   \t [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]\n>>   \t [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]\n>>   \t [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]\n>> +\t [--message-id]\n>>   \t [(<mbox> | <Maildir>)...]\n>>   'git am' (--continue | --skip | --abort)\n>>   \n>> @@ -121,6 +122,11 @@ default.   You can use `--no-utf8` to override this.\n>>   \tuser to lie about the author date by using the same\n>>   \tvalue as the committer date.\n>>   \n>> +-m::\n>> +--message-id::\n>> +\tExtract the Message-Id: header from the e-mail and\n>> +\tappend it to the commit message's tag stanza.\n>> +\n>>   --skip::\n>>   \tSkip the current patch.  This is only meaningful when\n>>   \trestarting an aborted patch.\n>> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\n>> index cf11c8d..f1e1fed 100644\n>> --- a/builtin/mailinfo.c\n>> +++ b/builtin/mailinfo.c\n>> @@ -278,7 +278,7 @@ static void cleanup_space(struct strbuf *sb)\n>>   \n>>   static void decode_header(struct strbuf *line);\n>>   static const char *header[MAX_HDR_PARSED] = {\n>> -\t\"From\",\"Subject\",\"Date\",\n>> +\t\"From\",\"Subject\",\"Date\",\"Message-Id\"\n>>   };\n>>   \n>>   static inline int cmp_header(const struct strbuf *line, const char *hdr)\n>> diff --git a/git-am.sh b/git-am.sh\n>> index ee61a77..c0e7bdd 100755\n>> --- a/git-am.sh\n>> +++ b/git-am.sh\n>> @@ -39,6 +39,7 @@ committer-date-is-author-date    lie about committer date\n>>   ignore-date     use current timestamp for author date\n>>   rerere-autoupdate update the index with reused conflict resolution if possible\n>>   S,gpg-sign?     GPG-sign commits\n>> +m,message-id    copy the Message-Id: header to the commit's tag stanza\n>>   rebasing*       (internal use for git-rebase)\"\n>>   \n>>   . git-sh-setup\n>> @@ -371,7 +372,7 @@ split_patches () {\n>>   prec=4\n>>   dotest=\"$GIT_DIR/rebase-apply\"\n>>   sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=\n>> -resolvemsg= resume= scissors= no_inbody_headers=\n>> +resolvemsg= resume= scissors= no_inbody_headers= message_id=\n>>   git_apply_opt=\n>>   committer_date_is_author_date=\n>>   ignore_date=\n>> @@ -442,6 +443,8 @@ it will be removed. Please do not use it anymore.\"\n>>   \t\tgpg_sign_opt=-S ;;\n>>   \t--gpg-sign=*)\n>>   \t\tgpg_sign_opt=\"-S${1#--gpg-sign=}\" ;;\n>> +\t-m|--message-id)\n>> +\t\tmessage_id=t ;;\n> Doesn't the message-id line in OPTIONS_SPEC make the negated long\n> option --no-message-id available as well? If that's the case, then\n> the corresponding case arm is missing from here.\n\nI don't know, but some other booleans don't supply negations, for \nexample --reject.\n\n>>   \t--)\n>>   \t\tshift; break ;;\n>>   \t*)\n>> @@ -565,6 +568,7 @@ Use \\\"git am --abort\\\" to remove it.\")\"\n>>   \techo \" $git_apply_opt\" >\"$dotest/apply-opt\"\n>>   \techo \"$threeway\" >\"$dotest/threeway\"\n>>   \techo \"$sign\" >\"$dotest/sign\"\n>> +\techo \"$message_id\" > \"$dotest/message-id\"\n> To match the local style conventions, the space character after the\n> redirection operator should be removed.\n\nSure.\n\n> Also, isn't the patch missing the bits where the state of message-id\n> is read? Like so:\n>\n>      if test \"$(cat \"$dotest/message-id\")\" = t\n>      then\n>      \tmessage_id=t\n>      fi\n\nGood catch, I guess this fixes a restarted am.\n\n>\n>>   \techo \"$utf8\" >\"$dotest/utf8\"\n>>   \techo \"$keep\" >\"$dotest/keep\"\n>>   \techo \"$scissors\" >\"$dotest/scissors\"\n>> @@ -757,6 +761,10 @@ To restore the original branch and stop patching run \\\"\\$cmdline --abort\\\".\"\n>>   \t\tthen\n>>   \t\t\tcat \"$dotest/msg-clean\"\n>>   \t\tfi\n>> +\t\tif test 't' == \"$message_id\"\n>> +\t\tthen\n>> +\t\t\tgrep ^Message-Id: \"$dotest/info\" || true\n> Why is the true guard needed here? The exit status of grep seems to\n> never be checked.\n\nI usually code scripts with -e, but here it's unneeded.\n\n>\n> Although I cannot come up with an example where this would matter,\n> you might want to consider using the grep wrapper sane_grep from\n> git-sh-setup.sh instead. It resets the environment variable\n> GREP_OPTIONS before calling grep so that no unexpected user options\n> come into play.\n\nGood idea.  Will post a v4 shortly.\n"},{"id":"245315","messageId":"xmqq1tu3isti.fsf@gitster.dls.corp.google.com","threadId":"37039","inReplyTo":"1404291113-4424-1-git-send-email-avi@cloudius-systems.com","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-02T17:17:13Z","receivedAt":"2014-07-02T17:17:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avi Kivity <avi@cloudius-systems.com> writes:\n\n> +\t\tif test 't' == \"$message_id\"\n> +\t\tthen\n> +\t\t\tgrep ^Message-Id: \"$dotest/info\" || true\n> +\t\tfi\n>  \t\tif test '' != \"$ADD_SIGNOFF\"\n>  \t\tthen\n>  \t\t\techo \"$ADD_SIGNOFF\"\n\nSeeing how existing code carefully makes sure that ADD_SIGNOFF has\nan empty line before it when and only when necessary to ensure that\nthere is a blank after the existing log message, I would suspect\nthat this patch that blindly inserts a line is doubly wrong.  The\noutput from \"grep\" may be appended without adding a blank when\nnecessary, and appending of ADD_SIGNOFF may end up adding an extra\nblank after Message-Id.  Am I reading the patch wrong?\n"},{"id":"245373","messageId":"53B563E1.8080201@cloudius-systems.com","threadId":"37039","inReplyTo":"xmqq1tu3isti.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] git-am: add option to extract email Message-Id: tag into commit log","fromName":"Avi Kivity","fromEmail":"avi@cloudius-systems.com","sentAt":"2014-07-03T14:08:33Z","receivedAt":"2014-07-03T14:08:33Z","isPatch":true,"sender":{"key":"avi@cloudius-systems.com","avatar":null},"body":"\nOn 07/02/2014 08:17 PM, Junio C Hamano wrote:\n> Avi Kivity <avi@cloudius-systems.com> writes:\n>\n>> +\t\tif test 't' == \"$message_id\"\n>> +\t\tthen\n>> +\t\t\tgrep ^Message-Id: \"$dotest/info\" || true\n>> +\t\tfi\n>>   \t\tif test '' != \"$ADD_SIGNOFF\"\n>>   \t\tthen\n>>   \t\t\techo \"$ADD_SIGNOFF\"\n> Seeing how existing code carefully makes sure that ADD_SIGNOFF has\n> an empty line before it when and only when necessary to ensure that\n> there is a blank after the existing log message, I would suspect\n> that this patch that blindly inserts a line is doubly wrong.  The\n> output from \"grep\" may be appended without adding a blank when\n> necessary, and appending of ADD_SIGNOFF may end up adding an extra\n> blank after Message-Id.  Am I reading the patch wrong?\n\nYes, you're right.  Will have to redo the logic for deciding whether we \nalready have a tag stanza or not.\n"}]}