{"thread":{"id":"26523","subject":"Why does git-patch-id(1) sometimes print two lines, one of which has commit = 0000000000000000000000000000000000000000","startedAt":"2011-02-16T14:56:32Z","lastAt":"2011-02-18T14:29:09Z","messageCount":18,"participants":["Ævar Arnfjörð Bjarmason","Michael J Gruber","Junio C Hamano","Jakub Narebski","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"161305","messageId":"AANLkTimHvA_LsU4YaiDHDKoXnsk5WgGCXgbuSJ99emXw@mail.gmail.com","threadId":"26523","inReplyTo":null,"subject":"Why does git-patch-id(1) sometimes print two lines, one of which has commit = 0000000000000000000000000000000000000000","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-02-16T14:56:32Z","receivedAt":"2011-02-16T14:56:32Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This behaves as I'd expect on git.git, i.e. shows a patch id and the commit id:\n\n $ git show 24231e063f0f003f8ffd7b64c7ba6a0baaaa5283 | git patch-id\n f10c69e0e5b33da206f37bd93639875555ac9b79\n24231e063f0f003f8ffd7b64c7ba6a0baaaa5283\n\nBut what does this mean, also on git.git:\n\n $ git show 7d48e9e6f77d336376c1a554eeff0590f77e1ee1 | git patch-id\n 4ba8a248731c5fcbd09cacb248d3128e742d1c90\n7d48e9e6f77d336376c1a554eeff0590f77e1ee1\n d019b35e0b859cdd6907ee170927de1124c0ed6e\n0000000000000000000000000000000000000000\n\n7d48e9e6f77d336376c1a554eeff0590f77e1ee1 is just one of the commits\nthat results in this output:\n\n $ git log --pretty=format:%H -p | git patch-id | grep\n0000000000000000000000000000000000000000\n d019b35e0b859cdd6907ee170927de1124c0ed6e\n0000000000000000000000000000000000000000\n 3b23a2a11055aef557369971e825010879a8c4d7\n0000000000000000000000000000000000000000\n d498fbbad6f1374d952925df699da237c3e8f2df\n0000000000000000000000000000000000000000\n b0c930dc1926ffae9cca022797856762fa908be6\n0000000000000000000000000000000000000000\n\nAnd on another repository where I'm dealing with this I have a bunch\nmore of them.\n\nWhy are they there and what do they mean? Maybe it's a bug, or\nsomething I can explain in the manual page.\n"},{"id":"161308","messageId":"4D5BF73E.50002@drmicha.warpmail.net","threadId":"26523","inReplyTo":"AANLkTimHvA_LsU4YaiDHDKoXnsk5WgGCXgbuSJ99emXw@mail.gmail.com","subject":"Re: Why does git-patch-id(1) sometimes print two lines, one of which has commit = 0000000000000000000000000000000000000000","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-16T16:11:42Z","receivedAt":"2011-02-16T16:11:42Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Ævar Arnfjörð Bjarmason venit, vidit, dixit 16.02.2011 15:56:\n> This behaves as I'd expect on git.git, i.e. shows a patch id and the commit id:\n> \n> $ git show 24231e063f0f003f8ffd7b64c7ba6a0baaaa5283 | git patch-id\n> f10c69e0e5b33da206f37bd93639875555ac9b79\n> 24231e063f0f003f8ffd7b64c7ba6a0baaaa5283\n> \n> But what does this mean, also on git.git:\n> \n> $ git show 7d48e9e6f77d336376c1a554eeff0590f77e1ee1 | git patch-id\n> 4ba8a248731c5fcbd09cacb248d3128e742d1c90\n> 7d48e9e6f77d336376c1a554eeff0590f77e1ee1\n> d019b35e0b859cdd6907ee170927de1124c0ed6e\n> 0000000000000000000000000000000000000000\n> \n> 7d48e9e6f77d336376c1a554eeff0590f77e1ee1 is just one of the commits\n> that results in this output:\n> \n> $ git log --pretty=format:%H -p | git patch-id | grep\n> 0000000000000000000000000000000000000000\n> d019b35e0b859cdd6907ee170927de1124c0ed6e\n> 0000000000000000000000000000000000000000\n> 3b23a2a11055aef557369971e825010879a8c4d7\n> 0000000000000000000000000000000000000000\n> d498fbbad6f1374d952925df699da237c3e8f2df\n> 0000000000000000000000000000000000000000\n> b0c930dc1926ffae9cca022797856762fa908be6\n> 0000000000000000000000000000000000000000\n\nYou're telling us that patch-ids, not the sha1's of affected commits here.\n\n> \n> And on another repository where I'm dealing with this I have a bunch\n> more of them.\n> \n> Why are they there and what do they mean? Maybe it's a bug, or\n> something I can explain in the manual page.\n\nYou would have to explain that git-patch-id trips over \"\\ Now\nnewline...\" lines in our diffs. Alternatively, wait a few minutes for my\npatch (done) with a test (the boring part...).\n\nCheers,\nMichael\n"},{"id":"161312","messageId":"d27aa07556df763b34b980d3706320216094d592.1297875236.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"4D5BF73E.50002@drmicha.warpmail.net","subject":"[PATCH 1/2] git-patch-id: test for \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-16T16:55:37Z","receivedAt":"2011-02-16T16:55:37Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over our very own diff extension for marking\nthe absence of newline at EOF.\n\nExpose this in a test.\n\nSigned-off-by: Michael J Gruber \n---\n t/t4204-patch-id.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex 68e2652..db96064 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -63,4 +63,40 @@ test_expect_success 'patch-id supports git-format-patch MIME output' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+cat >nonl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..2e65efe 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+\\ No newline at end of file\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+cat >withnl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..7898192 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+\tcat nonl | calc_patch_id nonl &&\n+\tcat withnl | calc_patch_id withnl &&\n+\ttest_cmp patch-id_nonl patch-id_withnl\n+'\n test_done\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161313","messageId":"144f7a4824abecaf87f862ed9e60587456128b41.1297875236.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"4D5BF73E.50002@drmicha.warpmail.net","subject":"[PATCH 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-16T16:55:38Z","receivedAt":"2011-02-16T16:55:38Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over our very own diff extension for marking\nthe absence of newline at EOF.\n\nFix it. (Ignore it, it's whitespace.)\n\nSigned-off-by: Michael J Gruber \n---\n builtin/patch-id.c | 2 ++\n t/t4204-patch-id.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 5125300..653d958 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n \t\t\tp += 7;\n \t\telse if (!memcmp(line, \"From \", 5))\n \t\t\tp += 5;\n+\t\telse if (!memcmp(line, \"\\\\ No newline at end of file\", 27))\n+\t\t\tcontinue;\n \n \t\tif (!get_sha1_hex(p, next_sha1)) {\n \t\t\tfound_next = 1;\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex db96064..d2c930d 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -94,7 +94,7 @@ index e69de29..6178079 100644\n +b\n EOF\n \n-test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+test_expect_success 'patch-id handles no-nl-at-eof markers' '\n \tcat nonl | calc_patch_id nonl &&\n \tcat withnl | calc_patch_id withnl &&\n \ttest_cmp patch-id_nonl patch-id_withnl\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161333","messageId":"7vwrkziw2i.fsf@alter.siamese.dyndns.org","threadId":"26523","inReplyTo":"144f7a4824abecaf87f862ed9e60587456128b41.1297875236.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-16T20:05:57Z","receivedAt":"2011-02-16T20:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, patch-id trips over our very own diff extension for marking\n> the absence of newline at EOF.\n>\n> Fix it. (Ignore it, it's whitespace.)\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> builtin/patch-id.c | 2 ++\n> t/t4204-patch-id.sh | 2 +-\n> 2 files changed, 3 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin/patch-id.c b/builtin/patch-id.c\n> index 5125300..653d958 100644\n> --- a/builtin/patch-id.c\n> +++ b/builtin/patch-id.c\n> @@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n> \t\t\tp += 7;\n> \t\telse if (!memcmp(line, \"From \", 5))\n> \t\t\tp += 5;\n> +\t\telse if (!memcmp(line, \"\\\\ No newline at end of file\", 27))\n> +\t\t\tcontinue;\n\nGood spotting, except that this string is a bit too specific to git and\nGNU diff run in the C or ??_en locales. An early draft of \"git apply\"\nused to have the same string but the string was removed before the command\nwas released to the wild with a lot weaker condition (grep for \"l10n\" in\nbuiltin/apply.c).\n"},{"id":"161387","messageId":"d27aa07556df763b34b980d3706320216094d592.1297928549.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"7vwrkziw2i.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 1/2] git-patch-id: test for \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-17T07:44:41Z","receivedAt":"2011-02-17T07:44:41Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over our very own diff extension for marking\nthe absence of newline at EOF.\n\nExpose this in a test.\n\nSigned-off-by: Michael J Gruber \n---\n t/t4204-patch-id.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex 68e2652..db96064 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -63,4 +63,40 @@ test_expect_success 'patch-id supports git-format-patch MIME output' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+cat >nonl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..2e65efe 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+\\ No newline at end of file\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+cat >withnl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..7898192 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+\tcat nonl | calc_patch_id nonl &&\n+\tcat withnl | calc_patch_id withnl &&\n+\ttest_cmp patch-id_nonl patch-id_withnl\n+'\n test_done\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161388","messageId":"536855b39d0ab5bb657ee6117a7e7c6bb0de0027.1297928549.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"7vwrkziw2i.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-17T07:44:42Z","receivedAt":"2011-02-17T07:44:42Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over our very own diff extension for marking\nthe absence of newline at EOF.\n\nFix it. (Ignore it, it's whitespace.)\n\nSigned-off-by: Michael J Gruber \n---\nv2 has the more fuzzy marker detection from \"apply\" as suggested by Junio.\n\n builtin/patch-id.c | 2 ++\n t/t4204-patch-id.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 5125300..e1c3cb9 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n \t\t\tp += 7;\n \t\telse if (!memcmp(line, \"From \", 5))\n \t\t\tp += 5;\n+\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n+\t\t\tcontinue;\n \n \t\tif (!get_sha1_hex(p, next_sha1)) {\n \t\t\tfound_next = 1;\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex db96064..d2c930d 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -94,7 +94,7 @@ index e69de29..6178079 100644\n +b\n EOF\n \n-test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+test_expect_success 'patch-id handles no-nl-at-eof markers' '\n \tcat nonl | calc_patch_id nonl &&\n \tcat withnl | calc_patch_id withnl &&\n \ttest_cmp patch-id_nonl patch-id_withnl\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161404","messageId":"m3ei7698qf.fsf@localhost.localdomain","threadId":"26523","inReplyTo":"d27aa07556df763b34b980d3706320216094d592.1297928549.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2 1/2] git-patch-id: test for \"no newline\" markers","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-17T11:55:46Z","receivedAt":"2011-02-17T11:55:46Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, patch-id trips over our very own diff extension for marking\n> the absence of newline at EOF.\n\nThis is not *our* diff extension; it is either GNU diff extension, or\nit is defined in diff/patch format standard.\n\ndiff.info states in chapter \"Incomplete Lines\":\n\n When an input file ends in a non-newline character, its last line is\n called an \"incomplete line\" because its last character is not a\n newline. All other lines are called \"full lines\" and end in a newline\n character. Incomplete lines do not match full lines unless differences\n in white space are ignored (*note White Space::).\n\n An incomplete line is normally distinguished on output from a full\n line by a following line that starts with `\\'. [...]\n\n For example, suppose `F' and `G' are one-byte files that contain\n just `f' and `g', respectively. Then `diff F G' outputs\n\n 1c1\n < f\n \\ No newline at end of file\n ---\n > g\n \\ No newline at end of file\n\n (The exact message may differ in non-English locales.)\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"161409","messageId":"4D5D108F.7060606@drmicha.warpmail.net","threadId":"26523","inReplyTo":"m3ei7698qf.fsf@localhost.localdomain","subject":"Re: [PATCHv2 1/2] git-patch-id: test for \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-17T12:11:59Z","receivedAt":"2011-02-17T12:11:59Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jakub Narebski venit, vidit, dixit 17.02.2011 12:55:\n> Michael J Gruber writes:\n> \n>> Currently, patch-id trips over our very own diff extension for marking\n>> the absence of newline at EOF.\n> \n> This is not *our* diff extension; it is either GNU diff extension, or\n> it is defined in diff/patch format standard.\n\nSo, which one is it?\n\nThe point is that our very own code produces that extension, so we'd\nbetter support what our \"diff\" and \"log -p\" produce.\n\nMichael\n"},{"id":"161498","messageId":"20110218041609.GA12348@sigill.intra.peff.net","threadId":"26523","inReplyTo":"536855b39d0ab5bb657ee6117a7e7c6bb0de0027.1297928549.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-18T04:16:09Z","receivedAt":"2011-02-18T04:16:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 17, 2011 at 08:44:42AM +0100, Michael J Gruber wrote:\n\n> --- a/builtin/patch-id.c\n> +++ b/builtin/patch-id.c\n> @@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n> \t\t\tp += 7;\n> \t\telse if (!memcmp(line, \"From \", 5))\n> \t\t\tp += 5;\n> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n> +\t\t\tcontinue;\n\nWow, that's pretty obscure. I wonder if the test should be factored out\ninto line_is_no_newline_at_end_of_file() (or surely there is some more\nsensible name), and used by both apply and patch-id. Along with a nice\ncomment (which I see apply already has) describing what in the world the\nmagic number 12 means.\n\n-Peff\n"},{"id":"161518","messageId":"4D5E278F.2010509@drmicha.warpmail.net","threadId":"26523","inReplyTo":"20110218041609.GA12348@sigill.intra.peff.net","subject":"Re: [PATCHv2 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-18T08:02:23Z","receivedAt":"2011-02-18T08:02:23Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 18.02.2011 05:16:\n> On Thu, Feb 17, 2011 at 08:44:42AM +0100, Michael J Gruber wrote:\n> \n>> --- a/builtin/patch-id.c\n>> +++ b/builtin/patch-id.c\n>> @@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n>> \t\t\tp += 7;\n>> \t\telse if (!memcmp(line, \"From \", 5))\n>> \t\t\tp += 5;\n>> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n>> +\t\t\tcontinue;\n> \n> Wow, that's pretty obscure. I wonder if the test should be factored out\n> into line_is_no_newline_at_end_of_file() (or surely there is some more\n> sensible name), and used by both apply and patch-id. Along with a nice\n> comment (which I see apply already has) describing what in the world the\n> magic number 12 means.\n\nSo, where do you suggest it should go? As far as I can see, we have two\nplaces reading these markers (the above builtins) and two places writing\nthem (diff.c, xdiff/xutils.c), at least for git-core. (git-gui's\ndiff.tcl has its own strict check etc.)\n\nI would opt for putting a more detailed explanation in the commit\nmessage (reasoning + ref. to apply.c) rather then trying to factor this.\n\nAfter all, our code is largely undocumented as far as inline\ndocumentation goes (it has frustrated me a few times), and the more\ncomprehensive documentation are the commit messages which git blame\npoints you to.\n\nMichael\n"},{"id":"161527","messageId":"26bc8a07f1b965a3022c3a1542f95422a8462e42.1298023827.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"4D5E278F.2010509@drmicha.warpmail.net","subject":"[PATCHv3 1/2] git-patch-id: test for \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-18T10:12:41Z","receivedAt":"2011-02-18T10:12:41Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over the diff extension for marking\nthe absence of newline at EOF.\n\nExpose this in a test.\n\nSigned-off-by: Michael J Gruber \n---\n t/t4204-patch-id.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex 68e2652..db96064 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -63,4 +63,40 @@ test_expect_success 'patch-id supports git-format-patch MIME output' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+cat >nonl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..2e65efe 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+\\ No newline at end of file\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+cat >withnl <<\\EOF\n+diff --git i/a w/a\n+index e69de29..7898192 100644\n+--- i/a\n++++ w/a\n+@@ -0,0 +1 @@\n++a\n+diff --git i/b w/b\n+index e69de29..6178079 100644\n+--- i/b\n++++ w/b\n+@@ -0,0 +1 @@\n++b\n+EOF\n+\n+test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+\tcat nonl | calc_patch_id nonl &&\n+\tcat withnl | calc_patch_id withnl &&\n+\ttest_cmp patch-id_nonl patch-id_withnl\n+'\n test_done\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161526","messageId":"c0ee81b29d902e773dc851aaf86b99c9337079c9.1298023827.git.git@drmicha.warpmail.net","threadId":"26523","inReplyTo":"4D5E278F.2010509@drmicha.warpmail.net","subject":"[PATCHv3 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-18T10:12:42Z","receivedAt":"2011-02-18T10:12:42Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, patch-id trips over the diff extension for marking\nthe absence of newline at EOF.\n\nFix it. (Ignore it, it's whitespace.)\n\nThis uses the same detection rationale as in buitlin/apply.c, which was\nintroduced in\n\n433ef8a ([PATCH] Make git-apply understand incomplete lines in non-C locales, 2005-09-04)\n\nSigned-off-by: Michael J Gruber \n---\n builtin/patch-id.c | 2 ++\n t/t4204-patch-id.sh | 2 +-\n 2 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 5125300..e1c3cb9 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n \t\t\tp += 7;\n \t\telse if (!memcmp(line, \"From \", 5))\n \t\t\tp += 5;\n+\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n+\t\t\tcontinue;\n \n \t\tif (!get_sha1_hex(p, next_sha1)) {\n \t\t\tfound_next = 1;\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex db96064..d2c930d 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -94,7 +94,7 @@ index e69de29..6178079 100644\n +b\n EOF\n \n-test_expect_failure 'patch-id handles no-nl-at-eof markers' '\n+test_expect_success 'patch-id handles no-nl-at-eof markers' '\n \tcat nonl | calc_patch_id nonl &&\n \tcat withnl | calc_patch_id withnl &&\n \ttest_cmp patch-id_nonl patch-id_withnl\n-- \n1.7.4.1.74.gf39475.dirty\n"},{"id":"161529","messageId":"20110218104035.GA7262@sigill.intra.peff.net","threadId":"26523","inReplyTo":"4D5E278F.2010509@drmicha.warpmail.net","subject":"Re: [PATCHv2 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-18T10:40:35Z","receivedAt":"2011-02-18T10:40:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 18, 2011 at 09:02:23AM +0100, Michael J Gruber wrote:\n\n> > Wow, that's pretty obscure. I wonder if the test should be factored out\n> > into line_is_no_newline_at_end_of_file() (or surely there is some more\n> > sensible name), and used by both apply and patch-id. Along with a nice\n> > comment (which I see apply already has) describing what in the world the\n> > magic number 12 means.\n> \n> So, where do you suggest it should go? As far as I can see, we have two\n> places reading these markers (the above builtins) and two places writing\n> them (diff.c, xdiff/xutils.c), at least for git-core.\n\nI think you could ignore the writers, as they are much more\nstraightforward. But...\n\n> (git-gui's diff.tcl has its own strict check etc.)\n\nUgh. Factoring that out with the C code would not be easy. :)\n\n> I would opt for putting a more detailed explanation in the commit\n> message (reasoning + ref. to apply.c) rather then trying to factor this.\n\nI think that's OK. It's not a lot of code, it's just very non-obvious.\nSo a comment is probably as good as a refactor.\n\n> After all, our code is largely undocumented as far as inline\n> documentation goes (it has frustrated me a few times), and the more\n> comprehensive documentation are the commit messages which git blame\n> points you to.\n\nYeah, it is sometimes annoying. On the other hand, you know that a\ncomment in the commit log cannot be stale, because you are viewing it in\ncontext (well, it can be, but theoretically you have the tools to\ndetermine that).\n\nI'm not sure what the right balance is. I tend to write inline comments\nfor small non-obvious things (e.g., why a variable is being strdup'd),\nand describe the bigger picture in the commit message (e.g., why a\nparticular algorithm was chosen).\n\nBut it has only been a few years that we've had version control systems\nwhere history browsing wasn't absolutely horrendous. Maybe in another\ndecade or so there will be best practices for this sort of thing. :)\n\n-Peff\n"},{"id":"161530","messageId":"20110218104151.GB7262@sigill.intra.peff.net","threadId":"26523","inReplyTo":"c0ee81b29d902e773dc851aaf86b99c9337079c9.1298023827.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-18T10:41:51Z","receivedAt":"2011-02-18T10:41:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 18, 2011 at 11:12:42AM +0100, Michael J Gruber wrote:\n\n> Currently, patch-id trips over the diff extension for marking\n> the absence of newline at EOF.\n> \n> Fix it. (Ignore it, it's whitespace.)\n> \n> This uses the same detection rationale as in buitlin/apply.c, which was\n> introduced in\n> \n> 433ef8a ([PATCH] Make git-apply understand incomplete lines in non-C locales, 2005-09-04)\n\nThanks, this looks good to me, with one style nit:\n\n> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n> +\t\t\tcontinue;\n\nWhitespace around relational operators.\n\n-Peff\n"},{"id":"161531","messageId":"4D5E4D44.6000203@drmicha.warpmail.net","threadId":"26523","inReplyTo":"20110218104151.GB7262@sigill.intra.peff.net","subject":"Re: [PATCHv3 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-02-18T10:43:16Z","receivedAt":"2011-02-18T10:43:16Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 18.02.2011 11:41:\n> On Fri, Feb 18, 2011 at 11:12:42AM +0100, Michael J Gruber wrote:\n> \n>> Currently, patch-id trips over the diff extension for marking\n>> the absence of newline at EOF.\n>>\n>> Fix it. (Ignore it, it's whitespace.)\n>>\n>> This uses the same detection rationale as in buitlin/apply.c, which was\n>> introduced in\n>>\n>> 433ef8a ([PATCH] Make git-apply understand incomplete lines in non-C locales, 2005-09-04)\n> \n> Thanks, this looks good to me, with one style nit:\n> \n>> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n>> +\t\t\tcontinue;\n> \n> Whitespace around relational operators.\n\nYou think\n\n+\t\telse if (!memcmp(line, \"\\\\ \", 2)&&strlen(line)>=12)\n\nis that much better?\n\nNitpickers need to brace for smartass responses ;)\n\nJunio, should I resend with two extra spaces?\n\nMichael\n"},{"id":"161532","messageId":"20110218104741.GA11888@sigill.intra.peff.net","threadId":"26523","inReplyTo":"4D5E4D44.6000203@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-02-18T10:47:41Z","receivedAt":"2011-02-18T10:47:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 18, 2011 at 11:43:16AM +0100, Michael J Gruber wrote:\n\n> >> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n> >> +\t\t\tcontinue;\n> > \n> > Whitespace around relational operators.\n> \n> You think\n> \n> +\t\telse if (!memcmp(line, \"\\\\ \", 2)&&strlen(line)>=12)\n> \n> is that much better?\n\nYeah, it's starting to look like perl. :)\n\n-Peff\n"},{"id":"161549","messageId":"201102181529.18497.jnareb@gmail.com","threadId":"26523","inReplyTo":"c0ee81b29d902e773dc851aaf86b99c9337079c9.1298023827.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3 2/2] git-patch-id: do not trip over \"no newline\" markers","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-02-18T14:29:09Z","receivedAt":"2011-02-18T14:29:09Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michael J Gruber wrote:\n\n> --- a/builtin/patch-id.c\n> +++ b/builtin/patch-id.c\n> @@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx)\n> \t\t\tp += 7;\n> \t\telse if (!memcmp(line, \"From \", 5))\n> \t\t\tp += 5;\n> +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line)>=12)\n> +\t\t\tcontinue;\n> \n> \t\tif (!get_sha1_hex(p, next_sha1)) {\n> \t\t\tfound_next = 1;\n\nAll of those are minor issues:\n\n1. Whitespace (coding style) - it should be\n\n +\t\telse if (!memcmp(line, \"\\\\ \", 2) && strlen(line) >= 12)\n\n2. Is \"\\\\ \" needed, or \"\\\\\" would be enough (line[0] == '\\')?\n\n3. What is this 12 in \"strlen(line) >= 12\" about? Note that \n strlen() is optimized out during compiling.\n\n-- \nJakub Narebski\nPoland\n"}]}