{"thread":{"id":"37477","subject":"[RFC PATCH 1/1] am: add gitk patch format","startedAt":"2014-09-03T09:35:18Z","lastAt":"2014-09-05T21:54:26Z","messageCount":14,"participants":["Chris Packham","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"248778","messageId":"1409736919-22341-1-git-send-email-judge.packham@gmail.com","threadId":"37477","inReplyTo":null,"subject":"[RFC PATCH 0/1] am: bug report and new patch format support","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T09:35:18Z","receivedAt":"2014-09-03T09:35:18Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi List,\n\nWhen I first tried to apply a patch someone gave me I got the following\nerrors.\n\n  $ git am patch.patch\n  tr: write error: Broken pipe\n  tr: write error\n  Patch format detection failed.\n\nThe patch itself was generated from gitk and while I'd seen the format\ndetection problem before but the tr errors were new to me. I haven't\nlooked at the tr problem yet but the gitk thing had been bugging me for\na while so here's a patch that understands the format generated by gitk.\n\n Documentation/git-am.txt |    3 ++-\n git-am.sh                |   35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 37 insertions(+), 1 deletion(-)\n\nChris Packham (1):\n      am: add gitk patch format\n"},{"id":"248777","messageId":"1409736919-22341-2-git-send-email-judge.packham@gmail.com","threadId":"37477","inReplyTo":"1409736919-22341-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCH 1/1] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T09:35:19Z","receivedAt":"2014-09-03T09:35:19Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Patches created using gitk's \"write commit to file\" functionality (which\nuses 'git diff-tree -p --pretty' under the hood) need some massaging in\norder to apply cleanly. This consists of dropping the 'commit' line\nautomatically determining the subject and removing leading whitespace.\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\n Documentation/git-am.txt |    3 ++-\n git-am.sh                |   35 +++++++++++++++++++++++++++++++++++\n 2 files changed, 37 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 9adce37..b59d2b3 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -101,7 +101,8 @@ default.   You can use `--no-utf8` to override this.\n \tBy default the command will try to detect the patch format\n \tautomatically. This option allows the user to bypass the automatic\n \tdetection and specify the patch format that the patch(es) should be\n-\tinterpreted as. Valid formats are mbox, stgit, stgit-series and hg.\n+\tinterpreted as. Valid formats are mbox, stgit, stgit-series, hg and\n+\tgitk.\n \n -i::\n --interactive::\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..73b0a86 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -227,6 +227,9 @@ check_patch_format () {\n \t\t\"# HG changeset patch\")\n \t\t\tpatch_format=hg\n \t\t\t;;\n+\t\t'commit '*)\n+\t\t\tpatch_format=gitk\n+\t\t\t;;\n \t\t*)\n \t\t\t# if the second line is empty and the third is\n \t\t\t# a From, Author or Date entry, this is very\n@@ -357,6 +360,38 @@ split_patches () {\n \t\tthis=\n \t\tmsgnum=\n \t\t;;\n+\tgitk)\n+\t\t# These patches are generates with 'git diff-tree -p --pretty'\n+\t\t# we discard the 'commit' line, after that the first line not\n+\t\t# starting with 'Author:' or 'Date:' is the subject. We also\n+\t\t# need to strip leading whitespace from the message body.\n+\t\tthis=0\n+\t\tfor gitk in \"$@\"\n+\t\tdo\n+\t\t\tthis=$(expr \"$this\" + 1)\n+\t\t\tmsgnum=$(printf \"%0${prec}d\" $this)\n+\t\t\t@@PERL@@ -ne 'BEGIN { $subject = 0 }\n+\t\t\t\ts/^    // ;\n+\t\t\t\tif ($subject > 1) { print ; }\n+\t\t\t\telsif (/^commit\\s.*$/) { next ; }\n+\t\t\t\telsif (/^\\s+$/) { next ; }\n+\t\t\t\telsif (/^Author:/) { s/Author/From/ ; print ;}\n+\t\t\t\telsif (/^Date:/) { print ;}\n+\t\t\t\telsif ($subject) {\n+\t\t\t\t\t$subject = 2 ;\n+\t\t\t\t\tprint \"\\n\" ;\n+\t\t\t\t\tprint ;\n+\t\t\t\t} else {\n+\t\t\t\t\tprint \"Subject: \", $_ ;\n+\t\t\t\t\t$subject = 1;\n+\t\t\t\t}\n+\t\t\t' <\"$gitk\" >\"$dotest/$msgnum\" || clean_abort\n+\n+\t\tdone\n+\t\techo \"$this\" >\"$dotest/last\"\n+\t\tthis=\n+\t\tmsgnum=\n+\t\t;;\n \t*)\n \t\tif test -n \"$patch_format\"\n \t\tthen\n-- \n1.7.9.5\n"},{"id":"248779","messageId":"CAFOYHZAFvqv49oFAv5w5S2Eaw_fpbvucuNvSs7O9rMw367AwXw@mail.gmail.com","threadId":"37477","inReplyTo":"1409736919-22341-2-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC PATCH 1/1] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T09:59:26Z","receivedAt":"2014-09-03T09:59:26Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Reviewing my own code.\n\nOn Wed, Sep 3, 2014 at 9:35 PM, Chris Packham <judge.packham@gmail.com> wrote:\n> Patches created using gitk's \"write commit to file\" functionality (which\n> uses 'git diff-tree -p --pretty' under the hood) need some massaging in\n> order to apply cleanly. This consists of dropping the 'commit' line\n> automatically determining the subject and removing leading whitespace.\n>\n> Signed-off-by: Chris Packham <judge.packham@gmail.com>\n> ---\n>  Documentation/git-am.txt |    3 ++-\n>  git-am.sh                |   35 +++++++++++++++++++++++++++++++++++\n>  2 files changed, 37 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\n> index 9adce37..b59d2b3 100644\n> --- a/Documentation/git-am.txt\n> +++ b/Documentation/git-am.txt\n> @@ -101,7 +101,8 @@ default.   You can use `--no-utf8` to override this.\n>         By default the command will try to detect the patch format\n>         automatically. This option allows the user to bypass the automatic\n>         detection and specify the patch format that the patch(es) should be\n> -       interpreted as. Valid formats are mbox, stgit, stgit-series and hg.\n> +       interpreted as. Valid formats are mbox, stgit, stgit-series, hg and\n> +       gitk.\n>\n>  -i::\n>  --interactive::\n> diff --git a/git-am.sh b/git-am.sh\n> index ee61a77..73b0a86 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -227,6 +227,9 @@ check_patch_format () {\n>                 \"# HG changeset patch\")\n>                         patch_format=hg\n>                         ;;\n> +               'commit '*)\n> +                       patch_format=gitk\n> +                       ;;\n>                 *)\n>                         # if the second line is empty and the third is\n>                         # a From, Author or Date entry, this is very\n> @@ -357,6 +360,38 @@ split_patches () {\n>                 this=\n>                 msgnum=\n>                 ;;\n> +       gitk)\n> +               # These patches are generates with 'git diff-tree -p --pretty'\n> +               # we discard the 'commit' line, after that the first line not\n> +               # starting with 'Author:' or 'Date:' is the subject. We also\n> +               # need to strip leading whitespace from the message body.\n> +               this=0\n> +               for gitk in \"$@\"\n> +               do\n> +                       this=$(expr \"$this\" + 1)\n> +                       msgnum=$(printf \"%0${prec}d\" $this)\n> +                       @@PERL@@ -ne 'BEGIN { $subject = 0 }\n> +                               s/^    // ;\n\nThis is a little too aggressive. It'll also chomp whitespace from the\ndiff context. We should probably check for the 'diff --git' line and\nstop stripping whitespace.\n\n> +                               if ($subject > 1) { print ; }\n> +                               elsif (/^commit\\s.*$/) { next ; }\n> +                               elsif (/^\\s+$/) { next ; }\n> +                               elsif (/^Author:/) { s/Author/From/ ; print ;}\n> +                               elsif (/^Date:/) { print ;}\n> +                               elsif ($subject) {\n> +                                       $subject = 2 ;\n> +                                       print \"\\n\" ;\n> +                                       print ;\n> +                               } else {\n> +                                       print \"Subject: \", $_ ;\n> +                                       $subject = 1;\n> +                               }\n> +                       ' <\"$gitk\" >\"$dotest/$msgnum\" || clean_abort\n> +\n> +               done\n> +               echo \"$this\" >\"$dotest/last\"\n> +               this=\n> +               msgnum=\n> +               ;;\n>         *)\n>                 if test -n \"$patch_format\"\n>                 then\n> --\n> 1.7.9.5\n>\n"},{"id":"248780","messageId":"CAFOYHZCJhGtJcBUDdQXkF6SMCmQ=XDk2D+tQ3D6e4+i9VaqGew@mail.gmail.com","threadId":"37477","inReplyTo":"1409736919-22341-1-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC PATCH 0/1] am: bug report and new patch format support","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T10:18:19Z","receivedAt":"2014-09-03T10:18:19Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Wed, Sep 3, 2014 at 9:35 PM, Chris Packham <judge.packham@gmail.com> wrote:\n> Hi List,\n>\n> When I first tried to apply a patch someone gave me I got the following\n> errors.\n>\n>   $ git am patch.patch\n>   tr: write error: Broken pipe\n>   tr: write error\n>   Patch format detection failed.\n\nI can't do this with every patch. It seems to be size related, if I\nfeed it a big enough patch it triggers the error.\nSomewhere around 3000 lines triggers it for me. I think the cause is\nthe following construct in check_patch_format\n\n  {\n    ...\n    tr -d '\\015' <\"$1\" |\n    ...\n  } < \"$1\" || clean_abort\n\n>\n> The patch itself was generated from gitk and while I'd seen the format\n> detection problem before but the tr errors were new to me. I haven't\n> looked at the tr problem yet but the gitk thing had been bugging me for\n> a while so here's a patch that understands the format generated by gitk.\n>\n>  Documentation/git-am.txt |    3 ++-\n>  git-am.sh                |   35 +++++++++++++++++++++++++++++++++++\n>  2 files changed, 37 insertions(+), 1 deletion(-)\n>\n> Chris Packham (1):\n>       am: add gitk patch format\n>\n>\n"},{"id":"248816","messageId":"1409782918-26133-1-git-send-email-judge.packham@gmail.com","threadId":"37477","inReplyTo":"1409736919-22341-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCHv2 0/2] am: bug fix and new patch format support","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T22:21:56Z","receivedAt":"2014-09-03T22:21:56Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"I've done some digging into the tr error message. I think the extra\nre-direction of stdin is unnecessary. I've removed it and the tests\nstill pass. The error message I was seeing is no longer seen but I've\nfound it hard to actually reproduce (even with different sized patches).\n\nI've also updated 'am: avoid re-directing stdin twice' to add some tests\nand only strip the description part of the patch.\n\nChris Packham (2):\n  am: add gitk patch format\n  am: avoid re-directing stdin twice\n\n Documentation/git-am.txt |  3 ++-\n git-am.sh                | 38 +++++++++++++++++++++++++++++++++++++-\n t/t4150-am.sh            | 13 +++++++++++++\n 3 files changed, 52 insertions(+), 2 deletions(-)\n\n-- \n2.0.4.2.gadd452d\n"},{"id":"248818","messageId":"1409782918-26133-2-git-send-email-judge.packham@gmail.com","threadId":"37477","inReplyTo":"1409782918-26133-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T22:21:57Z","receivedAt":"2014-09-03T22:21:57Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Patches created using gitk's \"write commit to file\" functionality (which\nuses 'git diff-tree -p --pretty' under the hood) need some massaging in\norder to apply cleanly. This consists of dropping the 'commit' line\nautomatically determining the subject and removing leading whitespace.\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\n Documentation/git-am.txt |  3 ++-\n git-am.sh                | 36 ++++++++++++++++++++++++++++++++++++\n t/t4150-am.sh            | 13 +++++++++++++\n 3 files changed, 51 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 9adce37..b59d2b3 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -101,7 +101,8 @@ default.   You can use `--no-utf8` to override this.\n \tBy default the command will try to detect the patch format\n \tautomatically. This option allows the user to bypass the automatic\n \tdetection and specify the patch format that the patch(es) should be\n-\tinterpreted as. Valid formats are mbox, stgit, stgit-series and hg.\n+\tinterpreted as. Valid formats are mbox, stgit, stgit-series, hg and\n+\tgitk.\n \n -i::\n --interactive::\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..f979925 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -227,6 +227,9 @@ check_patch_format () {\n \t\t\"# HG changeset patch\")\n \t\t\tpatch_format=hg\n \t\t\t;;\n+\t\t'commit '*)\n+\t\t\tpatch_format=gitk\n+\t\t\t;;\n \t\t*)\n \t\t\t# if the second line is empty and the third is\n \t\t\t# a From, Author or Date entry, this is very\n@@ -357,6 +360,39 @@ split_patches () {\n \t\tthis=\n \t\tmsgnum=\n \t\t;;\n+\tgitk)\n+\t\t# These patches are generates with 'git diff-tree -p --pretty'\n+\t\t# we discard the 'commit' line, after that the first line not\n+\t\t# starting with 'Author:' or 'Date:' is the subject. We also\n+\t\t# need to strip leading whitespace from the message body.\n+\t\tthis=0\n+\t\tfor gitk in \"$@\"\n+\t\tdo\n+\t\t\tthis=$(expr \"$this\" + 1)\n+\t\t\tmsgnum=$(printf \"%0${prec}d\" $this)\n+\t\t\t@@PERL@@ -ne 'BEGIN { $subject = 0; $diff = 0; }\n+\t\t\t\tif (!$diff) { s/^    // ; }\n+\t\t\t\tif ($subject > 1) { print ; }\n+\t\t\t\telsif (/^commit\\s.*$/) { next ; }\n+\t\t\t\telsif (/^\\s+$/) { next ; }\n+\t\t\t\telsif (/^Author:/) { s/Author/From/ ; print ; }\n+\t\t\t\telsif (/^Date:/) { print ;}\n+\t\t\t\telsif (/^diff --git/) { $diff = 1 ; print ;}\n+\t\t\t\telsif ($subject) {\n+\t\t\t\t\t$subject = 2 ;\n+\t\t\t\t\tprint \"\\n\" ;\n+\t\t\t\t\tprint ;\n+\t\t\t\t} else {\n+\t\t\t\t\tprint \"Subject: \", $_ ;\n+\t\t\t\t\t$subject = 1;\n+\t\t\t\t}\n+\t\t\t' <\"$gitk\" >\"$dotest/$msgnum\" || clean_abort\n+\n+\t\tdone\n+\t\techo \"$this\" >\"$dotest/last\"\n+\t\tthis=\n+\t\tmsgnum=\n+\t\t;;\n \t*)\n \t\tif test -n \"$patch_format\"\n \t\tthen\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 5edb79a..e36cd0b 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -103,6 +103,7 @@ test_expect_success setup '\n \t\techo \"X-Fake-Field: Line Three\" &&\n \t\tgit format-patch --stdout first | sed -e \"1d\"\n \t} > patch1-ws.eml &&\n+\tgit diff-tree -p --pretty second >patch1-gitk.eml &&\n \n \tsed -n -e \"3,\\$p\" msg >file &&\n \tgit add file &&\n@@ -186,6 +187,18 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' '\n \ttest \"$(git rev-parse second^)\" = \"$(git rev-parse HEAD^)\"\n '\n \n+test_expect_success 'am applies patch generated by gitk' '\n+\tcat patch1-gitk.eml &&\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am patch1-gitk.eml &&\n+\ttest_path_is_missing .git/rebase-apply &&\n+\tgit diff --exit-code second &&\n+\ttest \"$(git rev-parse second)\" = \"$(git rev-parse HEAD)\" &&\n+\ttest \"$(git rev-parse second^)\" = \"$(git rev-parse HEAD^)\"\n+'\n+\n test_expect_success 'setup: new author and committer' '\n \tGIT_AUTHOR_NAME=\"Another Thor\" &&\n \tGIT_AUTHOR_EMAIL=\"a.thor@example.com\" &&\n-- \n2.0.4.2.gadd452d\n"},{"id":"248817","messageId":"1409782918-26133-3-git-send-email-judge.packham@gmail.com","threadId":"37477","inReplyTo":"1409782918-26133-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCHv2 2/2] am: avoid re-directing stdin twice","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-03T22:21:58Z","receivedAt":"2014-09-03T22:21:58Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"In check_patch_format we feed $1 to a block that attempts to determine\nthe patch format. Since we've already redirected $1 to stdin there is no\nneed to redirect it again when we invoke tr. This prevents the following\nerrors when invoking git am\n\n  $ git am patch.patch\n  tr: write error: Broken pipe\n  tr: write error\n  Patch format detection failed.\n\nCc: Stephen Boyd <bebarino@gmail.com>\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\n git-am.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex f979925..5d69c89 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -253,7 +253,7 @@ check_patch_format () {\n \t\t\t# discarding the indented remainder of folded lines,\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n-\t\t\ttr -d '\\015' <\"$1\" |\n+\t\t\ttr -d '\\015' |\n \t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p |\n \t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n-- \n2.0.4.2.gadd452d\n"},{"id":"248822","messageId":"xmqq38c8waub.fsf@gitster.dls.corp.google.com","threadId":"37477","inReplyTo":"1409782918-26133-2-git-send-email-judge.packham@gmail.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-03T23:19:56Z","receivedAt":"2014-09-03T23:19:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Packham <judge.packham@gmail.com> writes:\n\n> Patches created using gitk's \"write commit to file\" functionality (which\n> uses 'git diff-tree -p --pretty' under the hood) need some massaging in\n> order to apply cleanly.\n\nShouldn't that output routine be the one to be corrected, then?  We\nreally do not need yet another format to express the same thing,\nespecially from the same suite of programs.\n"},{"id":"248824","messageId":"CAFOYHZCcAwHwRy50kE8=rRwEOtrXovNkkKSQo2Gwcfvbve1Qwg@mail.gmail.com","threadId":"37477","inReplyTo":"xmqq38c8waub.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-04T00:46:43Z","receivedAt":"2014-09-04T00:46:43Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Thu, Sep 4, 2014 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Chris Packham <judge.packham@gmail.com> writes:\n>\n>> Patches created using gitk's \"write commit to file\" functionality (which\n>> uses 'git diff-tree -p --pretty' under the hood) need some massaging in\n>> order to apply cleanly.\n>\n> Shouldn't that output routine be the one to be corrected, then?  We\n> really do not need yet another format to express the same thing,\n> especially from the same suite of programs.\n\nThat's an option. It shouldn't be too hard to make gitk use 'git\nformat-patch --stdout' instead. The problem for me is that it's easier\nfor me to update my git installation to get git am to accept the\ncurrent format than it is for me to ask the people generating these\npatches to change their git/gitk installation to generate a different\nformat.\n\nAnother thing that I've since realised is that this 'gitk' format is\nalso what you've get from git show or git log -p. So this is actually\nallowing (for better or worse) things like 'git show $sha1 | git am\n--patch-format=gitk'[*1*]. That may mean that we should call the\nformat something else (\"pretty\" perhaps?) and note that this is what\ngitk, git show and some incantations of git log generate.\n\n--\n[*1*] - Although I've just found a bug that affects the existing\n--patch-format=hg|stgit where reading from stdin is not currently\nsupported. I'll send out a v3 of this series that includes some tests\nfor those a bit later.\n"},{"id":"248843","messageId":"xmqqiol3uwr5.fsf@gitster.dls.corp.google.com","threadId":"37477","inReplyTo":"CAFOYHZCcAwHwRy50kE8=rRwEOtrXovNkkKSQo2Gwcfvbve1Qwg@mail.gmail.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-04T17:21:50Z","receivedAt":"2014-09-04T17:21:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Packham <judge.packham@gmail.com> writes:\n\n> Another thing that I've since realised is that this 'gitk' format is\n> also what you've get from git show or git log -p. So this is actually\n> allowing (for better or worse) things like 'git show $sha1 | git am\n> --patch-format=gitk'[*1*]. That may mean that we should call the\n> format something else (\"pretty\" perhaps?) and note that this is what\n> gitk, git show and some incantations of git log generate.\n\nI would not call it \"pretty\", because \"--pretty\" is merely a\nshort-hand to \"--pretty=<some format name>\".\n\nThe output format indents the log message text by four spaces for\nhuman reading to make it stand out from the patch text, and not\nmeant for machine consumption.  I doubt that a patchset that does\nnot update mailinfo and mailsplit to extract information and to undo\nthe indentation could be a right solution.  \"am\" itself should not\nbe mucking with the input files.\n"},{"id":"248881","messageId":"CAFOYHZC5pWadJiqY=F3gP4DKcNzhogfWH76jAcez5AjW7FJrVQ@mail.gmail.com","threadId":"37477","inReplyTo":"xmqqiol3uwr5.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-04T22:47:39Z","receivedAt":"2014-09-04T22:47:39Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi Junio,\n\nOn Fri, Sep 5, 2014 at 5:21 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Chris Packham <judge.packham@gmail.com> writes:\n>\n>> Another thing that I've since realised is that this 'gitk' format is\n>> also what you've get from git show or git log -p. So this is actually\n>> allowing (for better or worse) things like 'git show $sha1 | git am\n>> --patch-format=gitk'[*1*]. That may mean that we should call the\n>> format something else (\"pretty\" perhaps?) and note that this is what\n>> gitk, git show and some incantations of git log generate.\n>\n> I would not call it \"pretty\", because \"--pretty\" is merely a\n> short-hand to \"--pretty=<some format name>\".\n>\n> The output format indents the log message text by four spaces for\n> human reading to make it stand out from the patch text, and not\n> meant for machine consumption.\n\nFair enough.\n\n> I doubt that a patchset that does\n> not update mailinfo and mailsplit to extract information and to undo\n> the indentation could be a right solution.\n\nI've read this sentence a couple of times and I can't understand it. I\nget that you are against adding yet more special cases to 'git am' to\nhandle patches that weren't generated by 'git format-patch'. Are you\nsaying that this won't go in or that the solution should be\nimplemented differently.\n\n> \"am\" itself should not\n> be mucking with the input files.\n>\n\nAt the very least we need to drop the first line and replace \"Author\"\nwith \"From\". Which would still leave the commit message indented.\nSomething like the following allows the patch to be applied\n\n  sed -e '1d' -e 's/^Author:/From:/' <patch.patch | git am\n\nBut it'd be nice if am could do that for me to save my fingers some work.\n"},{"id":"248886","messageId":"CAFOYHZDfpZPvuE_BZQHajc65fZNKoyqvFf+UZyf0LyLwrooqzA@mail.gmail.com","threadId":"37477","inReplyTo":"CAPc5daWip1dQ5Or6hzmdjoBUStusvs-jK0ODNuzAotNfM5BLbQ@mail.gmail.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T01:23:17Z","receivedAt":"2014-09-05T01:23:17Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"(added back git ml because I accidentally dropped the Cc when replying\nto Junio).\n\nOn Fri, Sep 5, 2014 at 10:57 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> I doubt that a patchset that does\n>>> not update mailinfo and mailsplit to extract information and to undo\n>>> the indentation could be a right solution.\n>>\n>> I've read this sentence a couple of times and I can't understand it.\n>\n> \"am\" uses mailsplit on the input file to separate it into one or more\n> input files, each of which represents a single change. On each of\n> them, it uses mailinfo to extract the meta information (e.g.\n> authorship), log message and the patch into separate files.\n\nExcept when --patch-format=hg|stgit is specified (or detected). In\nthese cases mailsplit is avoided.\n\n> It does not make any sense for a support for a new input format that\n> does not teach mailinfo how to handle that format. Transforming it\n> into a pseudo e-mail format is not the way to go. If that approach were\n> acceptable, that format conversion filter can be run completely outside\n> \"am\" in the first place, no?\n\nSo teaching git mailinfo to do s/^    // (either when asked to or\nusing some heuristic) would be a better approach? I also think we\nshould accept \"Author:\" as an acceptable fallback if \"From:\" is not\npresent.\n"},{"id":"248903","messageId":"xmqq8ulxrkeq.fsf@gitster.dls.corp.google.com","threadId":"37477","inReplyTo":"CAFOYHZDfpZPvuE_BZQHajc65fZNKoyqvFf+UZyf0LyLwrooqzA@mail.gmail.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-05T18:29:01Z","receivedAt":"2014-09-05T18:29:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Packham <judge.packham@gmail.com> writes:\n\n> So teaching git mailinfo to do s/^    // (either when asked to or\n> using some heuristic) would be a better approach? I also think we\n> should accept \"Author:\" as an acceptable fallback if \"From:\" is not\n> present.\n\nNot as \"a fallback\" in the sense that \"Author:\" should not be\ntreated any specially when \"am\" (which stands for \"apply mail\") is\noperating on the patches in e-mails.  Whatever wants to convert the\noutput from \"log --pretty\" as if it came from \"log --pretty=email\"\nwould certainly need to flip \"Author:\" to \"From:\" (what should\nhappen when it sees \"From:\" in the input, though???), and whatgver\nwannts to pick metainfo from \"log --pretty\" output like mailinfo\ndoes for \"log --pretty=email\" output would certainly need to pick\nthe authorship from \"Author:\" (without paying any attention to\n\"From:\" if one exists).\n"},{"id":"248934","messageId":"CAFOYHZDeasZMFfupoXXV7ZKox-E2iDer_kEzQuZ9OhexQ=99aQ@mail.gmail.com","threadId":"37477","inReplyTo":"xmqq8ulxrkeq.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv2 1/2] am: add gitk patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T21:54:26Z","receivedAt":"2014-09-05T21:54:26Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Sat, Sep 6, 2014 at 6:29 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Chris Packham <judge.packham@gmail.com> writes:\n>\n>> So teaching git mailinfo to do s/^    // (either when asked to or\n>> using some heuristic) would be a better approach? I also think we\n>> should accept \"Author:\" as an acceptable fallback if \"From:\" is not\n>> present.\n>\n> Not as \"a fallback\" in the sense that \"Author:\" should not be\n> treated any specially when \"am\" (which stands for \"apply mail\") is\\\n> operating on the patches in e-mails.\n\nI was proposing we avoid the \"Patch does not have a valid e-mail\naddress.\" error when we can dwim and determine the email address from\n\"Author:\". I originally was going to say \"From:\" should take\nprecedence but it would be another way to indicate that the true\nauthor is not necessarily the person who sent the email.\n\n> Whatever wants to convert the\n> output from \"log --pretty\" as if it came from \"log --pretty=email\"\n> would certainly need to flip \"Author:\" to \"From:\" (what should\n> happen when it sees \"From:\" in the input, though???), and whatgver\n> wannts to pick metainfo from \"log --pretty\" output like mailinfo\n> does for \"log --pretty=email\" output would certainly need to pick\n> the authorship from \"Author:\" (without paying any attention to\n> \"From:\" if one exists).\n>\n\nWow. I didn't know --pretty=email existed. Better yet it works for\ndiff-tree so gitk should probably be using that to produce something\nthat can be exported/imported easily.\n\nI do wonder what the original use-case for \"write commit to file\" was.\nOnce it's been written to a file what is one supposed to do with it?\nIt's not something that 'git am' can consume (currently). Using 'git\napply' or 'patch' would lose the commit message plus you have to\nmanually stage/commit the changes.\n"}]}