{"thread":{"id":"20376","subject":"Help/Advice needed on diff bug in xutils.c","startedAt":"2009-08-04T23:33:24Z","lastAt":"2009-08-27T10:49:44Z","messageCount":63,"participants":["Thell Fowler","Johannes Schindelin","Alex Riesen","Junio C Hamano","Nanako Shiraishi","Nicolas Sebrecht","Don Zickus","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"119543","messageId":"1249428804.2774.52.camel@GWPortableVCS","threadId":"20376","inReplyTo":null,"subject":"Help/Advice needed on diff bug in xutils.c","fromName":"Thell Fowler","fromEmail":"tbfowler4@gmail.com","sentAt":"2009-08-04T23:33:24Z","receivedAt":"2009-08-04T23:33:24Z","isPatch":false,"sender":{"key":"tbfowler4@gmail.com","avatar":"https://gravatar.com/avatar/1b038b543b3facd3ae8cbfcc05ae06547e9f2015b5e661efbec68d662955a826?d=mp&s=160"},"body":"Hi all! Please give me a sanity check before I go crazy...\n\nThere is a bug in git diff (ignoring whitespace) that does not take into\naccount a trailing space at the end of a line at the end of a file when\nno new line follows.\n\nHere is the example of the bug:\nmkdir test_ws_eof\ncd test_ws_eof\ngit init\necho -n \"Test\" > test.txt\ngit add .\ngit commit -m'test'\ngit symbolic-ref HEAD refs/heads/with_space\nrm .git/index\ngit clean -f\necho -n \"Test \">test.txt\ngit add .\ngit commit -m'test'\n# Ignoring all whitespace there shouldn't be a diff.\ngit diff -w master -- test.txt\n# Ignoring space at eol there shouldn't be a diff\ngit diff --ignore-space-at-eol master -- test.txt\n# Ignoring with -b might have a case for a diff showing.\ngit diff -b master -- test.txt\n\n\nIn the xutils.c xdl_hash_record_with_whitespace function the trailing\nspace prior to eof was being calculated into the hash, I fixed that\nwith the change below, but there is still a difference being noted in\nxdl_recmatch because of the size difference.\n\nBefore I go changing something that shouldn't be changed could someone\nprovide some input please?\n\nThanks for reading,\nThell\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 04ad468..623da92 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -243,17 +243,17 @@ static unsigned long\nxdl_hash_record_with_whitespace(char\nconst **data,\n if (isspace(*ptr)) {\n const char *ptr2 = ptr;\n while (ptr + 1 < top && isspace(ptr[1])\n- && ptr[1] != '\\n')\n+ && ( ptr[1] != '\\n' && ptr[1] !=\n'\\0' ) )\n ptr++;\n if (flags & XDF_IGNORE_WHITESPACE)\n ; /* already handled */\n else if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n- && ptr[1] != '\\n') {\n+ && ( ptr[1] != '\\n' && ptr[1] !=\n'\\0' ) ) {\n ha += (ha << 5);\n ha ^= (unsigned long) ' ';\n }\n else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n- && ptr[1] != '\\n') {\n+ && ( ptr[1] != '\\n' && ptr[1] !=\n'\\0' ) ) {\n while (ptr2 != ptr + 1) {\n"},{"id":"119684","messageId":"alpine.DEB.1.00.0908052239180.8306@pacific.mpi-cbg.de","threadId":"20376","inReplyTo":"1249428804.2774.52.camel@GWPortableVCS","subject":"Re: Help/Advice needed on diff bug in xutils.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-05T20:45:20Z","receivedAt":"2009-08-05T20:45:20Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 4 Aug 2009, Thell Fowler wrote:\n\n> There is a bug in git diff (ignoring whitespace) that does not take into \n> account a trailing space at the end of a line at the end of a file when \n> no new line follows.\n> \n> Here is the example of the bug:\n> mkdir test_ws_eof\n> cd test_ws_eof\n> git init\n> echo -n \"Test\" > test.txt\n> git add .\n> git commit -m'test'\n> git symbolic-ref HEAD refs/heads/with_space\n> rm .git/index\n> git clean -f\n> echo -n \"Test \">test.txt\n> git add .\n> git commit -m'test'\n> # Ignoring all whitespace there shouldn't be a diff.\n> git diff -w master -- test.txt\n> # Ignoring space at eol there shouldn't be a diff\n> git diff --ignore-space-at-eol master -- test.txt\n> # Ignoring with -b might have a case for a diff showing.\n> git diff -b master -- test.txt\n\nIf you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding \na test_expect_failure for a known bug), it is much easier to convince \ndevelopers to work on the issue.\n\n> In the xutils.c xdl_hash_record_with_whitespace function the trailing \n> space prior to eof was being calculated into the hash, I fixed that with \n> the change below, but there is still a difference being noted in \n> xdl_recmatch because of the size difference.\n> \n> Before I go changing something that shouldn't be changed could someone\n> provide some input please?\n> \n> Thanks for reading,\n> Thell\n> \n> diff --git a/xdiff/xutils.c b/xdiff/xutils.c\n> index 04ad468..623da92 100644\n> --- a/xdiff/xutils.c\n> +++ b/xdiff/xutils.c\n> @@ -243,17 +243,17 @@ static unsigned long\n> xdl_hash_record_with_whitespace(char\n> const **data,\n> if (isspace(*ptr)) {\n> const char *ptr2 = ptr;\n> while (ptr + 1 < top && isspace(ptr[1])\n> - && ptr[1] != '\\n')\n> + && ( ptr[1] != '\\n' && ptr[1] !=\n> '\\0' ) )\n\nFirst, your coding style is different from the surrounding code. I think \nit goes without saying that this should be fixed.\n\nSecond, you do not need the parentheses at all (and therefore they should \ngo).\n\nThird, libxdiff does not assume to be fed NUL delimited strings.\n\nFourth, that condition \"ptr + 1 < top\" is already doing what you tried to \naccomplish here.\n\nSo I guess that you need to do add \"ptr + 1 < top\" checks \ninstead.\n\nThanks,\nDscho\n"},{"id":"120204","messageId":"alpine.DEB.2.00.0908101211590.32213@GWPortableVCS","threadId":"20376","inReplyTo":"alpine.DEB.1.00.0908052239180.8306@pacific.mpi-cbg.de","subject":"Re: Help/Advice needed on diff bug in xutils.c","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-10T18:54:44Z","receivedAt":"2009-08-10T18:54:44Z","isPatch":false,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Johannes Schindelin (Johannes.Schindelin@gmx.de) wrote on Aug 5, 2009:\n\n> On Tue, 4 Aug 2009, Thell Fowler wrote:\n>\n>> There is a bug in git diff (ignoring whitespace) that does not take into\n>> account a trailing space at the end of a line at the end of a file when\n>> no new line follows.\n\nFirst, please forgive my hubris at thinking I had _found_ a bug when my \nignorance of the issue was so obvious. I am definitely humbled after \nreading every post in the archive having to do with whitespace and diff.\n\n>>\n>> Here is the example of the bug:\n>> mkdir test_ws_eof\n>> cd test_ws_eof\n>> git init\n>> echo -n \"Test\" > test.txt\n>> git add .\n>> git commit -m'test'\n>> git symbolic-ref HEAD refs/heads/with_space\n>> rm .git/index\n>> git clean -f\n>> echo -n \"Test \">test.txt\n>> git add .\n>> git commit -m'test'\n>> # Ignoring all whitespace there shouldn't be a diff.\n>> git diff -w master -- test.txt\n>> # Ignoring space at eol there shouldn't be a diff\n>> git diff --ignore-space-at-eol master -- test.txt\n>> # Ignoring with -b might have a case for a diff showing.\n>> git diff -b master -- test.txt\n>\n> If you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding\n> a test_expect_failure for a known bug), it is much easier to convince\n> developers to work on the issue.\n>\n\nThank you. In progress.\n\nI am curious if t4015 is planned to be to be rewritten to follow what \nJunio had outlined and Giuseppe implemented for \nt4107-apply-ignore-whitespace.sh to make the spaces more obvious to the reader:\n\nOne other question on the making of the test in regards to the \nfollowing quote from:\nhttp://article.gmane.org/gmane.comp.version-control.git/124765\nwhere Junio C Hamano wrote:\n\n>>>>\tsed -e 's/Z/ /g' >patch3.patch <<\\EOF\n>>>> ...\n>>>> +Z \tprint_int(func(i));Z\n>>>> EOF\n>>>>\n>>>> to make invisible SP stand out more for the benefit of people reading \n>>>> the test script (I know you did not have leading SP before HT in \n>>>> yours, but the above illustrates the visibility issues). For other \n>>>> tests with test vector patches, visibility of whitespace is not much \n>>>> an issue, but this script is _all about_ whitespace, so anything that \n>>>> clarifies what is going on better would help.\n\nThe test being implemented was t4107-apply-ignore-whitespace.sh.\n\nAre there any plans to have t4015-diff-whitespace.sh tests rewritten in \nthe same fashion?\n\n> First, your coding style is different from the surrounding code. I think\n> it goes without saying that this should be fixed.\n>\n> Second, you do not need the parentheses at all (and therefore they should\n> go).\n>\n> Third, libxdiff does not assume to be fed NUL delimited strings.\n>\n\nYou're absolutely right, I'll be more aware in the future.\n\n> Fourth, that condition \"ptr + 1 < top\" is already doing what you tried to\n> accomplish here.\n>\n> So I guess that you need to do add \"ptr + 1 < top\" checks\n> instead.\n>\n\nI'll give it another go.\n\nThank you for the advice Dscho.\n\n-- \nThell\n"},{"id":"120339","messageId":"alpine.DEB.2.00.0908111942020.15481@GWPortableVCS","threadId":"20376","inReplyTo":"alpine.DEB.1.00.0908052239180.8306@pacific.mpi-cbg.de","subject":"[PATCH/RFC] Add diff tests for trailing-space and now newline","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-12T00:47:29Z","receivedAt":"2009-08-12T00:47:29Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Test each diff whitespace ignore option on trailing-space at eof\n\nSigned-off-by: Thell Fowler \n---\n\nJohannes Schindelin (Johannes.Schindelin@gmx.de) wrote on Aug 5, 2009:\n>On Tue, 4 Aug 2009, Thell Fowler wrote:\n>\n>> mkdir test_ws_eof\n>> cd test_ws_eof\n>> git init\n>> echo -n \"Test\" > test.txt\n>> git add .\n>> git commit -m'test'\n>> git symbolic-ref HEAD refs/heads/with_space\n>> rm .git/index\n>> git clean -f\n>> echo -n \"Test \">test.txt\n>> git add .\n>> git commit -m'test'\n>> # Ignoring all whitespace there shouldn't be a diff.\n>> git diff -w master -- test.txt\n>> # Ignoring space at eol there shouldn't be a diff\n>> git diff --ignore-space-at-eol master -- test.txt\n>> # Ignoring with -b might have a case for a diff showing.\n>> git diff -b master -- test.txt\n>\n>If you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding \n>a test_expect_failure for a known bug), it is much easier to convince \n>developers to work on the issue.\n\nIs this more along the right line?\n\nThell\n\n\n t/t4015-diff-whitespace.sh | 23 +++++++++++++++++++++++\n 1 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 6d13da3..fddbf20 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -395,4 +395,27 @@ test_expect_success 'combined diff with autocrlf conversion' '\n \n '\n \n+test_expect_failure 'diff -w on trailing-space with no newline' '\n+\n+\tgit reset --hard &&\n+\tprintf \"foo \" >x &&\n+\tgit commit -m \"trailing-space @ eof test\" x &&\n+\tprintf \"foo\" >x &&\n+\tgit commit -m \"trailing-space @ eof test\" x &&\n+\ttest_must_pass git diff -w HEAD^ -- x | grep \"foo \"\n+\n+'\n+\n+test_expect_failure 'diff -b on trailing-space with no newline' '\n+\n+\ttest_must_pass git diff -b HEAD^ -- x | grep \"foo \"\n+\n+'\n+\n+test_expect_failure 'diff --ignore-space-at-eol on trailing-space with no newline' '\n+\n+\ttest_must_pass git diff -ignore-space-at-eol HEAD^ -- x | grep \"foo \"\n+\n+'\n+\n test_done\n-- \n1.6.4.240.g4cd31\n"},{"id":"121294","messageId":"alpine.DEB.2.00.0908191713070.2012@GWPortableVCS","threadId":"20376","inReplyTo":"1249428804.2774.52.camel@GWPortableVCS","subject":"[PATCH 0/6 RFC] Series to correct xutils incomplete line handling.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:05:16Z","receivedAt":"2009-08-19T23:05:16Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"\n[1/6]\nAdd supplemental test for trailing-whitespace on incomplete lines.\n\n-- This patch is for illustrative purposes only. It exposes the current \nfailures of git diff whitespace ignore options when dealing with trailing-\nspaces on incomplete lines.\n\n[2/6] through [5/6]\nMake xdl_hash_record_with_whitespace ignore eof\nMake diff -w handle trailing-spaces on incomplete lines.\nMake diff -b handle trailing-spaces on incomplete lines.\nMake diff --ignore-space-at-eol handle incomplete lines.\n\n-- These alter the record ptr loops to go to the end of the record and to \ntreat the terminator as it would '\\n'.\n\n[6/6]\nAdd diff tests for trailing-space on incomplete lines\n\n-- Just the seven test cases that would identify future breakage.\n\n\n t/t4015-diff-trailing-whitespace.sh | 95 +++++++++++++++++++++++++++++++++++\n t/t4015-diff-whitespace.sh | 33 ++++++++++++\n xdiff/xutils.c | 20 ++++----\n 3 files changed, 138 insertions(+), 10 deletions(-)\n create mode 100755 t/t4015-diff-trailing-whitespace.sh\n"},{"id":"121295","messageId":"alpine.DEB.2.00.0908191710570.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 1/6] Add supplemental test for trailing-whitespace on incomplete lines.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:06:16Z","receivedAt":"2009-08-19T23:06:16Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"*** For illustrative purposes only and not meant for upstream ***\n\n - Adds a stand-alone test that loops through A-side B-side with\n and without new-lines from 0 to 3 spaces per side.\n This is a draft test meant to expose the issue with xutils.c\n handling of incomplete lines and trailing-spaces.\n\nSigned-off-by: Thell Fowler \n---\n t/t4015-diff-trailing-whitespace.sh | 95 +++++++++++++++++++++++++++++++++++\n 1 files changed, 95 insertions(+), 0 deletions(-)\n create mode 100755 t/t4015-diff-trailing-whitespace.sh\n\ndiff --git a/t/t4015-diff-trailing-whitespace.sh b/t/t4015-diff-trailing-whitespace.sh\nnew file mode 100755\nindex 0000000000000000000000000000000000000000..c4937c1b457c24b35565b09e7b262443a05f9795\n--- /dev/null\n+++ b/t/t4015-diff-trailing-whitespace.sh\n@@ -0,0 +1,95 @@\n+#!/bin/sh\n+\n+test_description='Test trailing whitespace in diff engine.\n+\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n+\n+# Trailing-space testing with and without newlines.\n+prepare_diff_file () {\n+\tprintf \"%s%$2s\" foo \"\" >\"$1\"\n+\tif [ $3 = \"+nl\" ]\n+\tthen\n+\t\tprintf \"\\n\" >>\"$1\"\n+\tfi\n+}\n+\n+diff_trailing () {\n+\tfoo=\"foo___\"\n+\tprepare_diff_file \"left\" \"$2\" \"$3\"\n+\tlfoo=$( expr substr $foo 1 $((3+$2)) )\n+\tlfoo=${lfoo}\"$3\"\n+\n+\tprepare_diff_file \"right\" \"$4\" \"$5\"\n+\trfoo=$( expr substr $foo 1 $((3+$4)) )\n+\trfoo=${rfoo}\"$5\"\n+\n+\tlabel=\"-$1 $lfoo $rfoo ($6)\"\n+\n+\tif [ \"$6\" != \"should_diff\" ]\n+\tthen\n+\t\tnegate='!'\n+\telse\n+\t\tnegate=''\n+\tfi\n+\n+\tif [ -z \"$7\" ]\n+\tthen\n+\t\ttest_expect_success \"$label\" \\\n+\t\t\"$negate git diff --no-index -$1 -- left right | grep -q foo\"\n+\telse\n+\t\ttest_expect_failure \"$label\" \\\n+\t\t\"$negate git diff --no-index -$1 -- left right | grep -q foo\"\n+\tfi\n+\n+\ttest_debug \"git diff --no-index -$1 -- left right | grep foo\"\n+}\n+\n+touch diffout\n+for arg in -ignore-all-space -ignore-space-at-eol -ignore-space-change\n+do\n+\tfor i1 in 0 1 2 3\n+\tdo\n+\t\tfor i2 in 0 1 2 3\n+\t\tdo\n+\t\t\tdiff_trailing $arg $i1 +nl $i2 -nl should_not_diff >> diffout\n+\t\t\tdiff_trailing $arg $i1 -nl $i2 +nl should_not_diff >> diffout\n+\n+\t\t\tif [ $i1 -ne $i2 ]\n+\t\t\tthen\n+\t\t\t\tdiff_trailing $arg $i1 +nl $i2 +nl should_not_diff >> diffout\n+\t\t\t\tdiff_trailing $arg $i1 -nl $i2 -nl should_not_diff >> diffout\n+\t\t\tfi\n+\t\tdone\n+\tdone\n+done\n+\n+test_debug 'grep \"FAIL\" diffout'\n+\n+for arg in all eol change\n+do\n+\tgrep \"FAIL\" diffout | \\\n+\tgrep \"$arg\" | \\\n+\tcut -d \" \" -f 4- | \\\n+\n+\t## Playing with filtering to isolate core issue.\n+\t#sort -k 2,2 -k 3,3 | \\\n+\t#awk '{ forward = $2 \" \" $3; reverse = $3 \" \" $2}\n+\t#\t!seen[forward]++ && !seen[reverse]++' | \\\n+\t#sort -k 2,2 | \\\n+\n+\t## Playing with filtering to isolate core issue.\n+\t## This seems like the most illustrative output...\n+\tawk '{ key=$3 ; gsub(/-/, \"+\", key) ; key=$2 \":\" key ; if ( hash[key]++ == 0 ) print ; }'\n+\t\n+\t## Playing with filtering to isolate core issue.\n+\t#awk '{ if ( $3 ~ /.*\\-/ )\n+\t#\t\tprint $0\n+\t#\telse\n+\t#\t\tprint $1 \" \" $3 \" \" $2 \" \" $4\n+\t#\t; }' | \\\n+\t#sort -k 2,2 -k 3,3\n+done\n+\n+test_done\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121297","messageId":"alpine.DEB.2.00.0908191725140.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 2/6] Make xdl_hash_record_with_whitespace ignore eof","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:06:53Z","receivedAt":"2009-08-19T23:06:53Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - When xdl_hash_record_with_whitespace encountered an incomplete\n line the hash would be different than the identical line with\n either --ignore-space-change or --ignore-space-at-eol on an\n incomplete line because they only terminated with a check for\n a new-line.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 04ad468702209b77427e635370d41001986042ce..c6512a53b08a8c9039614738310aa2786f4fbb1c 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -248,12 +248,12 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,\n \t\t\tif (flags & XDF_IGNORE_WHITESPACE)\n \t\t\t\t; /* already handled */\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n \t\t\t\tha += (ha << 5);\n \t\t\t\tha ^= (unsigned long) ' ';\n \t\t\t}\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n \t\t\t\twhile (ptr2 != ptr + 1) {\n \t\t\t\t\tha += (ha << 5);\n \t\t\t\t\tha ^= (unsigned long) *ptr2;\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121298","messageId":"alpine.DEB.2.00.0908191725440.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 3/6] Make diff -w handle trailing-spaces on incomplete lines.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:07:18Z","receivedAt":"2009-08-19T23:07:18Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - When processing trailing spaces with --ignore-all-space a diff\n would be found whenever one side had 0 spaces and either (or both)\n sides was an incomplete line. xdl_recmatch should process the\n full length of the record instead of assuming both sides have a\n terminator.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex c6512a53b08a8c9039614738310aa2786f4fbb1c..1f28f4fb4e0a8fdc6c9aa1904cf0362dd1e7b977 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -191,14 +191,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \tint i1, i2;\n \n \tif (flags & XDF_IGNORE_WHITESPACE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n+\t\tfor (i1 = i2 = 0; i1 <= s1 && i2 <= s2; ) {\n \t\t\tif (isspace(l1[i1]))\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n+\t\t\t\twhile (isspace(l1[i1]) && i1 <= s1)\n \t\t\t\t\ti1++;\n \t\t\tif (isspace(l2[i2]))\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n+\t\t\t\twhile (isspace(l2[i2]) && i2 <= s2)\n \t\t\t\t\ti2++;\n-\t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])\n+\t\t\tif (i1 <= s1 && i2 <= s2 && l1[i1++] != l2[i2++])\n \t\t\t\treturn 0;\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121300","messageId":"alpine.DEB.2.00.0908191726090.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 4/6] Make diff -b handle trailing-spaces on incomplete lines.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:07:55Z","receivedAt":"2009-08-19T23:07:55Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - When processing trailing spaces with --ignore-space-change a diff\n would be found whenever an incomplete line terminated before the\n whitespace handling started regardless of actual trailing-spaces.\n xdl_recmatch should process the full length of the record instead\n of assuming both sides have a terminator, and should treat the\n terminator as a whitespace like it does with '\\n'.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 1f28f4fb4e0a8fdc6c9aa1904cf0362dd1e7b977..e126de450c99fb1e557c2cfc0ffe54e8e3e80394 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -203,9 +203,9 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1])) {\n-\t\t\t\tif (!isspace(l2[i2]))\n+\t\tfor (i1 = i2 = 0; i1 <= s1 && i2 <= s2; ) {\n+\t\t\tif (isspace(l1[i1]) || (i1 == s1 && i2 < s2)) {\n+\t\t\t\tif (!isspace(l2[i2]) && i2 != s2)\n \t\t\t\t\treturn 0;\n \t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n \t\t\t\t\ti1++;\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121301","messageId":"alpine.DEB.2.00.0908191726280.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 5/6] Make diff --ignore-space-at-eol handle incomplete lines.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:08:26Z","receivedAt":"2009-08-19T23:08:26Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - When processing with --ignore-space-change a diff would be found\n whenever an incomplete line was encountered. xdl_recmatch should\n process the full length of the record instead of assuming both\n sides have a terminator.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex e126de450c99fb1e557c2cfc0ffe54e8e3e80394..a8ed102d528bdb5d8f0839eb392b35dc1c534fba 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -216,7 +216,7 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n+\t\tfor (i1 = i2 = 0; i1 <= s1 && i2 <= s2; ) {\n \t\t\tif (l1[i1] != l2[i2]) {\n \t\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n \t\t\t\t\ti1++;\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121302","messageId":"alpine.DEB.2.00.0908191726470.2012@GWPortableVCS","threadId":"20376","inReplyTo":"cover.1250719760.git.git@tbfowler.name","subject":"[PATCH 6/6] Add diff tests for trailing-space on incomplete lines","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-19T23:09:07Z","receivedAt":"2009-08-19T23:09:07Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Adds 7 --no-index tests to t4015-diff-whitespace.sh specifically\n to ensure that xutils.c xdl_hash_record_with_whitespace and\n xdl_recmatch process to the end of the record and handle an\n incomplete line terminator the same as a new-line.\n\nSigned-off-by: Thell Fowler \n---\n t/t4015-diff-whitespace.sh | 33 +++++++++++++++++++++++++++++++++\n 1 files changed, 33 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 6d13da30dad5a78fb17a01e86ef33072ea9e6250..193ddbe0659ede17154ffda3b25ebc5e6c686d6e 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -395,4 +395,37 @@ test_expect_success 'combined diff with autocrlf conversion' '\n \n '\n \n+# Ignore trailing-space testing on incomplete lines.\n+prepare_diff_file () {\n+\tprintf \"%s%$2s\" foo \"\" >\"$1\"\n+\tif [ $3 = \"+nl\" ]\n+\tthen\n+\t\tprintf \"\\n\" >>\"$1\"\n+\tfi\n+}\n+\n+diff_trailing () {\n+\tfoo=\"foo___\"\n+\tprepare_diff_file \"left\" \"$2\" \"$3\"\n+\tlfoo=$( expr substr $foo 1 $((3+$2)) )\n+\tlfoo=${lfoo}\"$3\"\n+\n+\tprepare_diff_file \"right\" \"$4\" \"$5\"\n+\trfoo=$( expr substr $foo 1 $((3+$4)) )\n+\trfoo=${rfoo}\"$5\"\n+\n+\tlabel=\"-$1 $lfoo $rfoo\"\n+\n+\ttest_expect_success \"$label\" \\\n+\t\"! git diff --no-index -$1 -- left right | grep -q foo\"\n+}\n+\n+diff_trailing w 0 +nl 1 -nl\n+diff_trailing w 0 -nl 1 -nl\n+diff_trailing b 0 +nl 0 -nl\n+diff_trailing b 1 +nl 0 -nl\n+diff_trailing b 1 -nl 0 -nl\n+diff_trailing -ignore-space-at-eol 0 +nl 0 -nl\n+diff_trailing -ignore-space-at-eol 2 +nl 2 -nl\n+\n test_done\n-- \n1.6.4.172.g5c0d0.dirty\n"},{"id":"121365","messageId":"alpine.DEB.2.00.0908201803170.2012@GWPortableVCS","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908191725440.2012@GWPortableVCS","subject":"Re: [PATCH 3/6] Make diff -w handle trailing-spaces on incomplete lines.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-20T23:09:26Z","receivedAt":"2009-08-20T23:09:26Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Thell Fowler (git@tbfowler.name) wrote on Aug 19, 2009:\n\n> - When processing trailing spaces with --ignore-all-space a diff\n> would be found whenever one side had 0 spaces and either (or both)\n> sides was an incomplete line. xdl_recmatch should process the\n> full length of the record instead of assuming both sides have a\n> terminator.\n> \n> @@ -191,14 +191,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n> \tint i1, i2;\n> \n> \tif (flags & XDF_IGNORE_WHITESPACE) {\n> -\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n> +\t\tfor (i1 = i2 = 0; i1 <= s1 && i2 <= s2; ) {\n> \t\t\tif (isspace(l1[i1]))\n> -\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n> +\t\t\t\twhile (isspace(l1[i1]) && i1 <= s1)\n\nThe change from '<' to <=' obviously did do jack-diddly-squat, which I \nshould have noticed earlier.\n\n> \t\t\t\t\ti1++;\n> \t\t\tif (isspace(l2[i2]))\n> -\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n> +\t\t\t\twhile (isspace(l2[i2]) && i2 <= s2)\n\nHere too.\n\n> \t\t\t\t\ti2++;\n> -\t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])\n> +\t\t\tif (i1 <= s1 && i2 <= s2 && l1[i1++] != l2[i2++])\n> \t\t\t\treturn 0;\n> \t\t}\n> \t\treturn (i1 >= s1 && i2 >= s2);\n> \n\n\nThose will be corrected for v2.\n\n-- \nThell\n"},{"id":"121444","messageId":"alpine.DEB.2.00.0908211228000.2012@GWPortableVCS","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908191713070.2012@GWPortableVCS","subject":"Re: [PATCH 0/6 RFC] Series to correct xutils incomplete line handling.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-21T17:39:37Z","receivedAt":"2009-08-21T17:39:37Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Thell Fowler (git@tbfowler.name) wrote on Aug 19, 2009:\n\n> t/t4015-diff-trailing-whitespace.sh | 95 +++++++++++++++++++++++++++++++++++\n> t/t4015-diff-whitespace.sh | 33 ++++++++++++\n> xdiff/xutils.c | 20 ++++----\n> 3 files changed, 138 insertions(+), 10 deletions(-)\n> create mode 100755 t/t4015-diff-trailing-whitespace.sh\n\nDon't bother trying this series. I tried it out on live data today and it \ndoes not work. It actually caused regression in the diffs for the \nconversion project I'm working on.\n\nWhat is _REALLY_ odd is that it didn't make any tests fail in the test \ndir using master, next, and pu.\n\n\nPerhaps someone can explain what I did wrong when testing?\n\ngit checkout master\nmake -s clean && make -s all && make -s install && cd t && make\n\nI really did do over 9 hours of testing using the test dir. First with \njust the branches with no modification, then with the modified \nt4015-diff-whitespace.sh, then with the xutils.c patch. And this was on \neach branch at about 40 minutes per run through.\n\n-- \nThell\n"},{"id":"121471","messageId":"81b0412b0908211516l4db79249h180e50ffb8e2c1ab@mail.gmail.com","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908211228000.2012@GWPortableVCS","subject":"Re: [PATCH 0/6 RFC] Series to correct xutils incomplete line handling.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-21T22:16:38Z","receivedAt":"2009-08-21T22:16:38Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Fri, Aug 21, 2009 at 19:39, Thell Fowler wrote:\n> What is _REALLY_ odd is that it didn't make any tests fail in the test\n> dir using master, next, and pu.\n\nThe test suite has a very good coverage, but it surely is not complete.\n\n> Perhaps someone can explain what I did wrong when testing?\n>\n> git checkout master\n> make -s clean && make -s all && make -s install && cd t && make\n\nThis should have worked. Although I prefer just:\n\n $ make -j4 && make test -j16\n"},{"id":"121489","messageId":"alpine.DEB.2.00.0908212307530.2012@GWPortableVCS","threadId":"20376","inReplyTo":"81b0412b0908211516l4db79249h180e50ffb8e2c1ab@mail.gmail.com","subject":"Re: [PATCH 0/6 RFC] Series to correct xutils incomplete line handling.","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-22T04:23:18Z","receivedAt":"2009-08-22T04:23:18Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Alex Riesen (raa.lkml@gmail.com) wrote on Aug 21, 2009:\n\n> On Fri, Aug 21, 2009 at 19:39, Thell Fowler wrote:\n> > What is _REALLY_ odd is that it didn't make any tests fail in the test\n> > dir using master, next, and pu.\n> \n> The test suite has a very good coverage, but it surely is not complete.\n> \n\nWell, two lessons learned...\n1) Don't do isolated tests and count on the bundled tests to catch the \ncorner cases.\n2) Write more tests.\n\n> > Perhaps someone can explain what I did wrong when testing?\n> >\n> > git checkout master\n> > make -s clean && make -s all && make -s install && cd t && make\n> \n> This should have worked. Although I prefer just:\n> \n> $ make -j4 && make test -j16\n> \n\nGood to know I was on the right track. Thanks.\n-- \nThell\n"},{"id":"121538","messageId":"1250999285-10683-1-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1249428804.2774.52.camel@GWPortableVCS","subject":"[PATCH-v2/RFC 0/6] improvements for trailing-space processing on incomplete lines","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:47:59Z","receivedAt":"2009-08-23T03:47:59Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"These patches are directly aimed at making it possible to validate the\nresults of git apply --whitespace=fix which is currently not possible\nin files with incomplete lines in various situations (demonstrated in\nthe test in PATCH 1).\n\nThe goal is to be able to validate the application of a patch generated\nfrom a dirty whitespace source to a clean target.\n\n D2 C2\n | \\/ |\n | /\\ |\n D1 C1\n\nwhere D1..D2, C1..C2, C1..D2, D1..C2 should all yield the same patch-id\nwhen passed a diff ignoring the applicable whitespace changes.\n\nApplying these patches does nothing to fix the case of differences when\napply --whitespace=fix limits new blank lines at eof as can happen when\nfoo\\r\\n\\r\\n\\r\\n is fixed to foo\\n\\n.\n\n\nThell Fowler (6):\n Add supplemental test for trailing-whitespace on incomplete lines\n xutils: fix hash with whitespace on incomplete line\n xutils: fix ignore-all-space on incomplete line\n xutils: fix ignore-space-change on incomplete line\n xutils: fix ignore-space-at-eol on incomplete line\n t4015: add tests for trailing-space on incomplete line\n\n t/t4015-diff-trailing-whitespace.sh | 95 +++++++++++++++++++++++++++++++++++\n t/t4015-diff-whitespace.sh | 33 ++++++++++++\n xdiff/xutils.c | 39 ++++++++++-----\n 3 files changed, 154 insertions(+), 13 deletions(-)\n create mode 100755 t/t4015-diff-trailing-whitespace.sh\n"},{"id":"121539","messageId":"1250999357-10827-1-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 1/6] Add supplemental test for trailing-whitespace on incomplete lines","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:12Z","receivedAt":"2009-08-23T03:49:12Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"*** For illustrative purposes only and not meant for upstream ***\n\n - Adds a stand-alone test that loops through A-side B-side with\n and without new-lines from 0 to 3 spaces per side.\n This is a draft test meant to expose the issue with xutils.c\n handling of incomplete lines and trailing-spaces.\n\nSigned-off-by: Thell Fowler \n---\n t/t4015-diff-trailing-whitespace.sh | 95 +++++++++++++++++++++++++++++++++++\n 1 files changed, 95 insertions(+), 0 deletions(-)\n create mode 100755 t/t4015-diff-trailing-whitespace.sh\n\ndiff --git a/t/t4015-diff-trailing-whitespace.sh b/t/t4015-diff-trailing-whitespace.sh\nnew file mode 100755\nindex 0000000..079fba5\n--- /dev/null\n+++ b/t/t4015-diff-trailing-whitespace.sh\n@@ -0,0 +1,95 @@\n+#!/bin/sh\n+\n+test_description='Test trailing whitespace in diff engine.\n+\n+'\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n+\n+# Trailing-space testing with and without newlines.\n+prepare_diff_file () {\n+\tprintf \"%s%$2s\" foo \"\" >\"$1\"\n+\tif [ $3 = \"+nl\" ]\n+\tthen\n+\t\tprintf \"\\n\" >>\"$1\"\n+\tfi\n+}\n+\n+diff_trailing () {\n+\tfoo=\"foo___\"\n+\tprepare_diff_file \"left\" \"$2\" \"$3\"\n+\tlfoo=$( expr substr $foo 1 $((3+$2)) )\n+\tlfoo=${lfoo}\"$3\"\n+\n+\tprepare_diff_file \"right\" \"$4\" \"$5\"\n+\trfoo=$( expr substr $foo 1 $((3+$4)) )\n+\trfoo=${rfoo}\"$5\"\n+\n+\tlabel=\"-$1 $lfoo $rfoo ($6)\"\n+\n+\tif [ \"$6\" != \"should_diff\" ]\n+\tthen\n+\t\tnegate='!'\n+\telse\n+\t\tnegate=''\n+\tfi\n+\n+\tif [ -z \"$7\" ]\n+\tthen\n+\t\ttest_expect_success \"$label\" \\\n+\t\t\"$negate git diff --no-index -$1 -- left right | grep -q foo\"\n+\telse\n+\t\ttest_expect_failure \"$label\" \\\n+\t\t\"$negate git diff --no-index -$1 -- left right | grep -q foo\"\n+\tfi\n+\n+\ttest_debug \"git diff --no-index -$1 -- left right | grep foo\"\n+}\n+\n+touch diffout\n+for arg in -ignore-all-space -ignore-space-at-eol -ignore-space-change\n+do\n+\tfor i1 in 0 1 2 3\n+\tdo\n+\t\tfor i2 in 0 1 2 3\n+\t\tdo\n+\t\t\tdiff_trailing $arg $i1 +nl $i2 -nl should_not_diff >> diffout\n+\t\t\tdiff_trailing $arg $i1 -nl $i2 +nl should_not_diff >> diffout\n+\n+\t\t\tif [ $i1 -ne $i2 ]\n+\t\t\tthen\n+\t\t\t\tdiff_trailing $arg $i1 +nl $i2 +nl should_not_diff >> diffout\n+\t\t\t\tdiff_trailing $arg $i1 -nl $i2 -nl should_not_diff >> diffout\n+\t\t\tfi\n+\t\tdone\n+\tdone\n+done\n+\n+test_debug 'grep \"FAIL\" diffout'\n+\n+for arg in all eol change\n+do\n+\tgrep \"FAIL\" diffout | \\\n+\tgrep \"$arg\" | \\\n+\tcut -d \" \" -f 4- | \\\n+\n+\t## Playing with filtering to isolate core issue.\n+\t#sort -k 2,2 -k 3,3 | \\\n+\t#awk '{ forward = $2 \" \" $3; reverse = $3 \" \" $2}\n+\t#\t!seen[forward]++ && !seen[reverse]++' | \\\n+\t#sort -k 2,2 | \\\n+\n+\t## Playing with filtering to isolate core issue.\n+\t## This seems like the most illustrative output...\n+\tawk '{ key=$3 ; gsub(/-/, \"+\", key) ; key=$2 \":\" key ; if ( hash[key]++ == 0 ) print ; }'\n+\n+\t## Playing with filtering to isolate core issue.\n+\t#awk '{ if ( $3 ~ /.*\\-/ )\n+\t#\t\tprint $0\n+\t#\telse\n+\t#\t\tprint $1 \" \" $3 \" \" $2 \" \" $4\n+\t#\t; }' | \\\n+\t#sort -k 2,2 -k 3,3\n+done\n+\n+test_done\n-- \n1.6.4.176.g556a4\n"},{"id":"121543","messageId":"1250999357-10827-2-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 2/6] xutils: fix hash with whitespace on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:13Z","receivedAt":"2009-08-23T03:49:13Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Make xdl_hash_record_with_whitespace stop hashing before the\n eof when ignoring space change or space at eol on an incomplete\n line.\n\n Resolves issue with a final trailing space being included in the\n hash on an incomplete line by treating the eof in the same fashion\n as a newline.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 04ad468..c6512a5 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -248,12 +248,12 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,\n \t\t\tif (flags & XDF_IGNORE_WHITESPACE)\n \t\t\t\t; /* already handled */\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n \t\t\t\tha += (ha << 5);\n \t\t\t\tha ^= (unsigned long) ' ';\n \t\t\t}\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n \t\t\t\twhile (ptr2 != ptr + 1) {\n \t\t\t\t\tha += (ha << 5);\n \t\t\t\t\tha ^= (unsigned long) *ptr2;\n-- \n1.6.4.176.g556a4\n"},{"id":"121541","messageId":"1250999357-10827-3-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:14Z","receivedAt":"2009-08-23T03:49:14Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Allow xdl_recmatch to recognize and continue processing when\n at the end of an incomplete line.\n\n Resolves issue with --ignore-all-space when either side 1 or 2\n has 0 trailing spaces and either (or both) are incomplete by\n allowing the processing loop to continue when one side has\n reached the end and includes a check for being at eof on an\n incomplete line.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex c6512a5..e22b4bb 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -191,12 +191,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \tint i1, i2;\n \n \tif (flags & XDF_IGNORE_WHITESPACE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n+\t\tfor (i1 = i2 = 0; i1 < s1 || i2 < s2; ) {\n \t\t\tif (isspace(l1[i1]))\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n+\t\t\t\twhile ((isspace(l1[i1]) && i1 < s1)\n+\t\t\t\t\t\t|| (i1 + 1 == s1 && l1[s1] != '\\n'))\n \t\t\t\t\ti1++;\n \t\t\tif (isspace(l2[i2]))\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n+\t\t\t\twhile ((isspace(l2[i2]) && i2 < s2)\n+\t\t\t\t\t\t|| (i2 + 1 == s2 && l2[s2] != '\\n'))\n \t\t\t\t\ti2++;\n \t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])\n \t\t\t\treturn 0;\n-- \n1.6.4.176.g556a4\n"},{"id":"121540","messageId":"1250999357-10827-4-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 4/6] xutils: fix ignore-space-change on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:15Z","receivedAt":"2009-08-23T03:49:15Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Allow xdl_recmatch to recognize and continue processing when at\n the end of an incomplete line.\n\n Resolves issue with --ignore-space-change when an incomplete line\n terminated before the eol whitespace handling started by allowing\n the processing loop to continue when one side has reached the end\n and includes a check for being at the eol on an incomplete line.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 25 ++++++++++++++++++-------\n 1 files changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex e22b4bb..54bb235 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -205,16 +205,27 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1])) {\n-\t\t\t\tif (!isspace(l2[i2]))\n+\t\tfor (i1 = i2 = 0; i1 < s1 || i2 < s2;) {\n+\t\t\tif (isspace(l1[i1]) || i1 == s1) {\n+\t\t\t\tif (!isspace(l2[i2]) && i2 != s2 && l2[s2] != '\\n')\n \t\t\t\t\treturn 0;\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n+\t\t\t\twhile ((isspace(l1[i1]) && i1 < s1)\n+\t\t\t\t\t\t|| (i1 + 1 == s1 && l1[s1] != '\\n'))\n \t\t\t\t\ti1++;\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n+\t\t\t\twhile ((isspace(l2[i2]) && i2 < s2)\n+\t\t\t\t\t\t|| (i2 + 1 == s2 && l2[s2] != '\\n'))\n \t\t\t\t\ti2++;\n-\t\t\t} else if (l1[i1++] != l2[i2++])\n-\t\t\t\treturn 0;\n+\t\t\t} else {\n+\t\t\t\tif (l1[i1] != l2[i2] && ((i1 != s1 && l1[s1] != '\\n')\n+\t\t\t\t\t\t|| (i2 != s2 && l2[s2] != '\\n')))\n+\t\t\t\t\treturn 0;\n+\t\t\t\telse {\n+\t\t\t\t\tif (i1 < s1)\n+\t\t\t\t\t\ti1++;\n+\t\t\t\t\tif (i2 < s2)\n+\t\t\t\t\t\ti2++;\n+\t\t\t\t}\n+\t\t\t}\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n-- \n1.6.4.176.g556a4\n"},{"id":"121542","messageId":"1250999357-10827-5-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 5/6] xutils: fix ignore-space-at-eol on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:16Z","receivedAt":"2009-08-23T03:49:16Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Allow xdl_recmatch to process to the eof.\n\n Resolves issue with --ignore-space-at-eol processing where an\n incomplete line terminated processing early before a final check\n could be done on the other side.\n\nSigned-off-by: Thell Fowler \n---\n xdiff/xutils.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 54bb235..3e26488 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -229,7 +229,7 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n \t\t}\n \t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n+\t\tfor (i1 = i2 = 0; i1 <= s1 && i2 <= s2; ) {\n \t\t\tif (l1[i1] != l2[i2]) {\n \t\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n \t\t\t\t\ti1++;\n-- \n1.6.4.176.g556a4\n"},{"id":"121544","messageId":"1250999357-10827-6-git-send-email-git@tbfowler.name","threadId":"20376","inReplyTo":"1250999285-10683-1-git-send-email-git@tbfowler.name","subject":"[PATCH-v2/RFC 6/6] t4015: add tests for trailing-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T03:49:17Z","receivedAt":"2009-08-23T03:49:17Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":" - Add 7 --no-index tests to t4015-diff-whitespace.sh to check\n that ignore options work on incomplete lines.\n\nSigned-off-by: Thell Fowler \n---\n t/t4015-diff-whitespace.sh | 33 +++++++++++++++++++++++++++++++++\n 1 files changed, 33 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 6d13da3..193ddbe 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -395,4 +395,37 @@ test_expect_success 'combined diff with autocrlf conversion' '\n \n '\n \n+# Ignore trailing-space testing on incomplete lines.\n+prepare_diff_file () {\n+\tprintf \"%s%$2s\" foo \"\" >\"$1\"\n+\tif [ $3 = \"+nl\" ]\n+\tthen\n+\t\tprintf \"\\n\" >>\"$1\"\n+\tfi\n+}\n+\n+diff_trailing () {\n+\tfoo=\"foo___\"\n+\tprepare_diff_file \"left\" \"$2\" \"$3\"\n+\tlfoo=$( expr substr $foo 1 $((3+$2)) )\n+\tlfoo=${lfoo}\"$3\"\n+\n+\tprepare_diff_file \"right\" \"$4\" \"$5\"\n+\trfoo=$( expr substr $foo 1 $((3+$4)) )\n+\trfoo=${rfoo}\"$5\"\n+\n+\tlabel=\"-$1 $lfoo $rfoo\"\n+\n+\ttest_expect_success \"$label\" \\\n+\t\"! git diff --no-index -$1 -- left right | grep -q foo\"\n+}\n+\n+diff_trailing w 0 +nl 1 -nl\n+diff_trailing w 0 -nl 1 -nl\n+diff_trailing b 0 +nl 0 -nl\n+diff_trailing b 1 +nl 0 -nl\n+diff_trailing b 1 -nl 0 -nl\n+diff_trailing -ignore-space-at-eol 0 +nl 0 -nl\n+diff_trailing -ignore-space-at-eol 2 +nl 2 -nl\n+\n test_done\n-- \n1.6.4.176.g556a4\n"},{"id":"121552","messageId":"7veir3ynma.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"1250999357-10827-2-git-send-email-git@tbfowler.name","subject":"Re: [PATCH-v2/RFC 2/6] xutils: fix hash with whitespace on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T07:51:09Z","receivedAt":"2009-08-23T07:51:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n> - Make xdl_hash_record_with_whitespace stop hashing before the\n> eof when ignoring space change or space at eol on an incomplete\n> line.\n>\n> Resolves issue with a final trailing space being included in the\n> hash on an incomplete line by treating the eof in the same fashion\n> as a newline.\n\nPlease study the style of existing commit messages and imitate them.\n\n> Signed-off-by: Thell Fowler \n> ---\n> xdiff/xutils.c | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/xdiff/xutils.c b/xdiff/xutils.c\n> index 04ad468..c6512a5 100644\n> --- a/xdiff/xutils.c\n> +++ b/xdiff/xutils.c\n> @@ -248,12 +248,12 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,\n> \t\t\tif (flags & XDF_IGNORE_WHITESPACE)\n> \t\t\t\t; /* already handled */\n> \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n> -\t\t\t\t\t&& ptr[1] != '\\n') {\n> +\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n> \t\t\t\tha += (ha << 5);\n> \t\t\t\tha ^= (unsigned long) ' ';\n> \t\t\t}\n> \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n> -\t\t\t\t\t&& ptr[1] != '\\n') {\n> +\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n> \t\t\t\twhile (ptr2 != ptr + 1) {\n> \t\t\t\t\tha += (ha << 5);\n> \t\t\t\t\tha ^= (unsigned long) *ptr2;\n\nThanks.\n\nThe issue you identified and tried to fix is a worthy one. But before the\npre-context of this hunk, I notice these lines:\n\n\t\tif (isspace(*ptr)) {\n\t\t\tconst char *ptr2 = ptr;\n\t\t\twhile (ptr + 1 < top && isspace(ptr[1])\n\t\t\t\t\t&& ptr[1] != '\\n')\n\t\t\t\tptr++;\n\nIf you have trailing whitespaces on an incomplete line, ptr initially\npoints at the first such whitespace, ptr2 points at the same location, and\nthen the while() loop advances ptr to point at the last byte on the line,\nwhich in turn will be the last byte of the file. And the codepath with\nyour updates still try to access ptr[1] that is beyond that last byte.\n\nI would write it like this patch instead.\n\nThe intent is the same as your patch, but it avoids accessing ptr[1] when\nthat is beyond the end of the buffer, and the logic is easier to follow as\nwell.\n\n-- >8 --\nSubject: xutils: fix hashing an incomplete line with whitespaces at the end\n\nUpon seeing a whitespace, xdl_hash_record_with_whitespace() first skipped\nthe run of whitespaces (excluding LF) that begins there, ensuring that the\npointer points the last whitespace character in the run, and assumed that\nthe next character must be LF at the end of the line. This does not work\nwhen hashing an incomplete line, that lacks the LF at the end.\n\nIntroduce \"at_eol\" variable that is true when either we are at the end of\nline (looking at LF) or at the end of an incomplete line, and use that\ninstead throughout the code.\n\nNoticed by Thell Fowler.\n\nSigned-off-by: Junio C Hamano \n---\n xdiff/xutils.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 04ad468..9411fa9 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -242,18 +242,20 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,\n \tfor (; ptr < top && *ptr != '\\n'; ptr++) {\n \t\tif (isspace(*ptr)) {\n \t\t\tconst char *ptr2 = ptr;\n+\t\t\tint at_eol;\n \t\t\twhile (ptr + 1 < top && isspace(ptr[1])\n \t\t\t\t\t&& ptr[1] != '\\n')\n \t\t\t\tptr++;\n+\t\t\tat_eol = (top <= ptr + 1 || ptr[1] == '\\n');\n \t\t\tif (flags & XDF_IGNORE_WHITESPACE)\n \t\t\t\t; /* already handled */\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t && !at_eol) {\n \t\t\t\tha += (ha << 5);\n \t\t\t\tha ^= (unsigned long) ' ';\n \t\t\t}\n \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n-\t\t\t\t\t&& ptr[1] != '\\n') {\n+\t\t\t\t && !at_eol) {\n \t\t\t\twhile (ptr2 != ptr + 1) {\n \t\t\t\t\tha += (ha << 5);\n \t\t\t\t\tha ^= (unsigned long) *ptr2;\n"},{"id":"121553","messageId":"7vvdkfx8rl.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"1250999357-10827-3-git-send-email-git@tbfowler.name","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T07:57:18Z","receivedAt":"2009-08-23T07:57:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n> @@ -191,12 +191,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n> \tint i1, i2;\n> \n> \tif (flags & XDF_IGNORE_WHITESPACE) {\n> -\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n> +\t\tfor (i1 = i2 = 0; i1 < s1 || i2 < s2; ) {\n> \t\t\tif (isspace(l1[i1]))\n> -\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n> +\t\t\t\twhile ((isspace(l1[i1]) && i1 < s1)\n> +\t\t\t\t\t\t|| (i1 + 1 == s1 && l1[s1] != '\\n'))\n\nThis is wrong. If you ran out l1/s1/i1 but you still have remaining\ncharacters in l2/s2/i2, you do not want to even look at l1[i1].\n\nYou can fudge this by sprinkling more \"(i1 < s1) &&\" in many places (and\nreordering how your inner while() loop checks (i1 < s1) and l1[i1]), but I\ndo not think that is the right direction.\n\nThe thing is, the loop control in this function is extremely hard to read\nto begin with, and now it is \"if we haven't run out both\", the complexity\nseeps into the inner logic.\n\nHow about doing it like this patch instead? This counterproposal replaces\nyour 3 patches starting from [3/6].\n\n-- >8 --\nSubject: xutils: Fix xdl_recmatch() on incomplete lines\n\nThell Fowler noticed that various \"ignore whitespace\" options to\ngit diff does not work well with whitespace glitches on an incomplete\nline.\n\nThe loop control of this function incorrectly handled incomplete lines,\nand it was extremely difficult to follow. This restructures the loops for\nthree variants of \"ignore whitespace\" logic.\n\nThe basic idea of the re-written logic is this.\n\n - An initial loop runs while the characters from both strings we are\n looking at match. We declare unmatch immediately when we find\n something that does not match and return false from the loop. And we\n break out of the loop if we ran out of either side of the string.\n\n The way we skip spaces inside this loop varies depending on the style\n of ignoring whitespaces.\n\n - After the loop, the lines can match only if the remainder consists of\n nothing but whitespaces. This part of the logic is shared across all\n three styles.\n\nThe new code is more obvious and should be much easier to follow.\n\nSigned-off-by: Junio C Hamano \n\n---\n xdiff/xutils.c | 111 +++++++++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 77 insertions(+), 34 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 9411fa9..dd8b7e7 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -186,50 +186,93 @@ long xdl_guess_lines(mmfile_t *mf) {\n \treturn nl + 1;\n }\n \n+static int remainder_all_ws(const char *l1, const char *l2,\n+\t\t\t int i1, int i2, long s1, long s2)\n+{\n+\tif (i1 < s1) {\n+\t\twhile (i1 < s1 && isspace(l1[i1]))\n+\t\t\ti1++;\n+\t\treturn (s1 == i1);\n+\t}\n+\tif (i2 < s2) {\n+\t\twhile (i2 < s2 && isspace(l2[i2]))\n+\t\t\ti2++;\n+\t\treturn (s2 == i2);\n+\t}\n+\treturn 1;\n+}\n+\n int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n {\n-\tint i1, i2;\n+\tint i1 = 0, i2 = 0;\n \n \tif (flags & XDF_IGNORE_WHITESPACE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1]))\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n-\t\t\t\t\ti1++;\n-\t\t\tif (isspace(l2[i2]))\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n-\t\t\t\t\ti2++;\n-\t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])\n-\t\t\t\treturn 0;\n+\t\twhile (1) {\n+\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n+\t\t\t\ti1++;\n+\t\t\twhile (i2 < s2 && isspace(l2[i2]))\n+\t\t\t\ti2++;\n+\t\t\tif (i1 < s1 && i2 < s2) {\n+\t\t\t\tif (l1[i1++] != l2[i2++])\n+\t\t\t\t\treturn 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tbreak;\n \t\t}\n-\t\treturn (i1 >= s1 && i2 >= s2);\n+\n+\t\t/*\n+\t\t * we ran out one side; the remaining side must be all\n+\t\t * whitespace to match.\n+\t\t */\n+\t\treturn remainder_all_ws(l1, l2, i1, i2, s1, s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1])) {\n-\t\t\t\tif (!isspace(l2[i2]))\n+\t\twhile (1) {\n+\t\t\tif (i1 < s1 && i2 < s2) {\n+\t\t\t\t/* Skip matching spaces */\n+\t\t\t\tif (isspace(l1[i1]) && isspace(l2[i2])) {\n+\t\t\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n+\t\t\t\t\t\ti1++;\n+\t\t\t\t\twhile (i2 < s2 && isspace(l2[i2]))\n+\t\t\t\t\t\ti2++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (i1 < s1 && i2 < s2) {\n+\t\t\t\t/*\n+\t\t\t\t * We still have both sides; do they match?\n+\t\t\t\t */\n+\t\t\t\tif (l1[i1++] != l2[i2++])\n \t\t\t\t\treturn 0;\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n-\t\t\t\t\ti1++;\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n-\t\t\t\t\ti2++;\n-\t\t\t} else if (l1[i1++] != l2[i2++])\n-\t\t\t\treturn 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tbreak;\n \t\t}\n-\t\treturn (i1 >= s1 && i2 >= s2);\n+\n+\t\t/*\n+\t\t * If we do not want -b to imply --ignore-space-at-eol\n+\t\t * then you would need to add this:\n+\t\t *\n+\t\t * if (!(flags & XDF_IGNORE_WHITESPACE_AT_EOL))\n+\t\t *\treturn (s1 <= i1 && s2 <= i2);\n+\t\t *\n+\t\t */\n+\n+\t\t/*\n+\t\t * we ran out one side; the remaining side must be all\n+\t\t * whitespace to match.\n+\t\t */\n+\t\treturn remainder_all_ws(l1, l2, i1, i2, s1, s2);\n+\n \t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (l1[i1] != l2[i2]) {\n-\t\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n-\t\t\t\t\ti1++;\n-\t\t\t\twhile (i2 < s2 && isspace(l2[i2]))\n-\t\t\t\t\ti2++;\n-\t\t\t\tif (i1 < s1 || i2 < s2)\n-\t\t\t\t\treturn 0;\n-\t\t\t\treturn 1;\n-\t\t\t}\n-\t\t\ti1++;\n-\t\t\ti2++;\n+\t\twhile (1) {\n+\t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] == l2[i2++])\n+\t\t\t\tcontinue;\n+\t\t\tbreak;\n \t\t}\n-\t\treturn i1 >= s1 && i2 >= s2;\n+\t\t/*\n+\t\t * we ran out one side; the remaining side must be all\n+\t\t * whitespace to match.\n+\t\t */\n+\t\treturn remainder_all_ws(l1, l2, i1, i2, s1, s2);\n \t} else\n \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n }\n"},{"id":"121554","messageId":"20090823171819.6117@nanako3.lavabit.com","threadId":"20376","inReplyTo":"7vvdkfx8rl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-08-23T08:18:19Z","receivedAt":"2009-08-23T08:18:19Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano \n\n> How about doing it like this patch instead? This counterproposal replaces\n> your 3 patches starting from [3/6].\n>\n> -- >8 --\n> Subject: xutils: Fix xdl_recmatch() on incomplete lines\n>\n> Thell Fowler noticed that various \"ignore whitespace\" options to\n> git diff does not work well with whitespace glitches on an incomplete\n> line.\n\nI think this should be \"options to git diff don't work\".\n\nI have two unrelated questions.\n\n(1) Why do you post patches to the list, instead of committing them yourself?\n(2) How do I apply a patch like this one to try to my tree? Am I expected to edit the mail message to remove everything before the shears mark before running the git-am command?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"121556","messageId":"7v1vn2yklo.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090823171819.6117@nanako3.lavabit.com","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T08:56:19Z","receivedAt":"2009-08-23T08:56:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Quoting Junio C Hamano \n>\n>> How about doing it like this patch instead? This counterproposal replaces\n>> your 3 patches starting from [3/6].\n>>\n>> -- >8 --\n>> Subject: xutils: Fix xdl_recmatch() on incomplete lines\n>>\n>> Thell Fowler noticed that various \"ignore whitespace\" options to\n>> git diff does not work well with whitespace glitches on an incomplete\n>> line.\n>\n> I think this should be \"options to git diff don't work\".\n\nSoory, I kant speel; thanks.\n\n> (1) Why do you post patches to the list, instead of committing them\n> yourself?\n\nSo that others can catch silly mistakes of mine, like the one you just\ncaught.\n\nI play three separate roles here, two of which I should send patches out\nwhile playing them.\n\n * Just like everybody else, I find itches to scratch from time to time,\n and I build my own topic branches locally for the changes to scratch\n them, just like other contributors.\n\n They are indeed committed and often immediately merged to 'pu', but I\n send out format-patch output for them, because I firmly believe that\n the development _process_, not just the end result, should be in the\n open. Everybody's patch should go through the list, get reviewed and\n improved by help from others. So should mine.\n\n * I read others' patches, review, comment, and suggest improvements and\n make counterproposals, just like others on the list.\n\n The \"how about\" patches when I am playing this role are often not meant\n as the final shape of the patch but to show the direction to improve\n upon. They are output from \"git diff\", not format-patch nor even \"git\n diff --cached\"---I do not commit, nor even add them to the index---and\n after I send out e-mails, I typically reset them away to work on\n something else, because they are usually not my itch.\n\n * I accept patches that were reviewed favorably on the list by running\n \"git am\" on them.\n\n> (2) How do I apply a patch like this one to try to my tree? Am I\n> expected to edit the mail message to remove everything before the shears\n> mark before running the git-am command?\n\nThat is how I have been doing it. My workflow is:\n\n (1) First read patches in my primary mailbox, while copying promising\n ones to a separate mailbox;\n\n (2) And then go through the separate mailbox as a separate pass, while\n fixing obvious typos and minor coding style violations still inside\n mailbox; and finally\n\n (3) Run \"git am\" on the (possibly edited) patch to apply.\n\nBecause I'll be editing the messages (both log and code) _anyway_,\nremoving everything before the scissors mark is not much of a trouble.\n\nHaving said that, I could use something like this.\n\n-- >8 -- cut here -- >8 -- \nSubject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n\nThis teaches mailinfo the scissors -- >8 -- mark; the command ignores\neverything before it in the message body.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-mailinfo.c | 37 ++++++++++++++++++++++++++++++++++++-\n 1 files changed, 36 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex b0b5d8f..461c47e 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -712,6 +712,34 @@ static inline int patchbreak(const struct strbuf *line)\n \treturn 0;\n }\n \n+static int scissors(const struct strbuf *line)\n+{\n+\tsize_t i, len = line->len;\n+\tint scissors_dashes_seen = 0;\n+\tconst char *buf = line->buf;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tif (isspace(buf[i]))\n+\t\t\tcontinue;\n+\t\tif (buf[i] == '-') {\n+\t\t\tscissors_dashes_seen |= 02;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (i + 1 < len && !memcmp(buf + i, \">8\", 2)) {\n+\t\t\tscissors_dashes_seen |= 01;\n+\t\t\ti++;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (i + 7 < len && !memcmp(buf + i, \"cut here\", 8)) {\n+\t\t\ti += 7;\n+\t\t\tcontinue;\n+\t\t}\n+\t\t/* everything else --- not scissors */\n+\t\tbreak;\n+\t}\n+\treturn scissors_dashes_seen == 03;\n+}\n+\n static int handle_commit_msg(struct strbuf *line)\n {\n \tstatic int still_looking = 1;\n@@ -723,10 +751,17 @@ static int handle_commit_msg(struct strbuf *line)\n \t\tstrbuf_ltrim(line);\n \t\tif (!line->len)\n \t\t\treturn 0;\n-\t\tif ((still_looking = check_header(line, s_hdr_data, 0)) != 0)\n+\t\tstill_looking = check_header(line, s_hdr_data, 0);\n+\t\tif (still_looking)\n \t\t\treturn 0;\n \t}\n \n+\tif (scissors(line)) {\n+\t\tfseek(cmitmsg, 0L, SEEK_SET);\n+\t\tstill_looking = 1;\n+\t\treturn 0;\n+\t}\n+\n \t/* normalize the log message to UTF-8. */\n \tif (metainfo_charset)\n \t\tconvert_to_utf8(line, charset.buf);\n-- \n1.6.4.1\n"},{"id":"121567","messageId":"alpine.DEB.2.00.0908231110500.29625@GWPortableVCS","threadId":"20376","inReplyTo":"7vvdkfx8rl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T17:01:25Z","receivedAt":"2009-08-23T17:01:25Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Junio C Hamano (gitster@pobox.com) wrote on Aug 23, 2009:\n\n> Thell Fowler writes:\n> \n> > @@ -191,12 +191,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n> > \tint i1, i2;\n> > \n> > \tif (flags & XDF_IGNORE_WHITESPACE) {\n> > -\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n> > +\t\tfor (i1 = i2 = 0; i1 < s1 || i2 < s2; ) {\n> > \t\t\tif (isspace(l1[i1]))\n> > -\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n> > +\t\t\t\twhile ((isspace(l1[i1]) && i1 < s1)\n> > +\t\t\t\t\t\t|| (i1 + 1 == s1 && l1[s1] != '\\n'))\n> \n> This is wrong. If you ran out l1/s1/i1 but you still have remaining\n> characters in l2/s2/i2, you do not want to even look at l1[i1].\n> \n> You can fudge this by sprinkling more \"(i1 < s1) &&\" in many places (and\n> reordering how your inner while() loop checks (i1 < s1) and l1[i1]), but I\n> do not think that is the right direction.\n> \n> The thing is, the loop control in this function is extremely hard to read\n> to begin with, and now it is \"if we haven't run out both\", the complexity\n> seeps into the inner logic.\n> \n\nI see what you're saying here and your absolutely right. Good thing you \ndidn't write a critique of the XDF_IGNORE_WHITESPACE_CHANGE case. ;)\n\n> How about doing it like this patch instead? This counterproposal replaces\n> your 3 patches starting from [3/6].\n[...snip...]\n> The basic idea of the re-written logic is this.\n> \n> - An initial loop runs while the characters from both strings we are\n> looking at match. We declare unmatch immediately when we find\n> something that does not match and return false from the loop. And we\n> break out of the loop if we ran out of either side of the string.\n> \n> The way we skip spaces inside this loop varies depending on the style\n> of ignoring whitespaces.\n> \n> - After the loop, the lines can match only if the remainder consists of\n> nothing but whitespaces. This part of the logic is shared across all\n> three styles.\n> \n> The new code is more obvious and should be much easier to follow.\n\nBecause the flow is much more direct it also makes the test additions to \nt4015 obsolete as they essentially tested for line end conditions instead \nof whitespace (like they should have).\n[...clip...]\n> +\t\t/*\n> +\t\t * If we do not want -b to imply --ignore-space-at-eol\n> +\t\t * then you would need to add this:\n> +\t\t *\n> +\t\t * if (!(flags & XDF_IGNORE_WHITESPACE_AT_EOL))\n> +\t\t *\treturn (s1 <= i1 && s2 <= i2);\n> +\t\t *\n> +\t\t */\n> +\n\nWhile it would be nice to have -b and --ignore-space-at-eol be two \ndifferent options that could be merged together the documentation states \nthat -b ignores spaces at eol, and there are scripts that depend on this \nbehavior.\n\nIMHO it is wrong to accept that new spaces where none existed before is \nakin to having one or more existing spaces coalesced. I seem to recall \nreading something about 1.7 having some changes in it that wouldn't be \nbackward compatible; perhaps -b and --ignore-space-at-eol could be \ndistinct options for that release.\n\nOn another item:\nRight now the xdl_recmatch() checks three distinct flags before having the \nopportunity to do the default behavior of a straight diff. In \nxdl_hash_record there is an initial check for whitespace flags.\n\n...\n\tif (flags & XDF_WHITESPACE_FLAGS)\n\t\treturn xdl_hash_record_with_whitespace(data, top, flags);\n...\n\nPerhaps a similar setup for xdl_rematch() and a \nxdl_recmatch_with_whitespace() ?\n\nLastly:\nSince your to counter-proposals give the same results, provide safer and \nfaster processing, eliminate the additional test, as well as being easier \nto read and comprehend I propose a v3 with just those two patches. I'll \nbe glad to post it, with or without a xdl_recmatch_with_whitespace, if \nneed be. And should I, or do I need to, add something to the commit (ie: \nack, tested, ...) ?\n\nThank you again for taking the time to look at this change!\n\n-- \nThell\n"},{"id":"121568","messageId":"alpine.DEB.2.00.0908231050240.29625@GWPortableVCS","threadId":"20376","inReplyTo":"7veir3ynma.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 2/6] xutils: fix hash with whitespace on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T17:02:14Z","receivedAt":"2009-08-23T17:02:14Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Junio C Hamano (gitster@pobox.com) wrote on Aug 23, 2009:\n> Thell Fowler writes:\n> \n> > - Make xdl_hash_record_with_whitespace stop hashing before the\n> > eof when ignoring space change or space at eol on an incomplete\n> > line.\n> >\n> > Resolves issue with a final trailing space being included in the\n> > hash on an incomplete line by treating the eof in the same fashion\n> > as a newline.\n> \n> Please study the style of existing commit messages and imitate them.\n> \n\nI'll keep trying.\n\n> > Signed-off-by: Thell Fowler \n> > ---\n> > xdiff/xutils.c | 4 ++--\n> > 1 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/xdiff/xutils.c b/xdiff/xutils.c\n> > index 04ad468..c6512a5 100644\n> > --- a/xdiff/xutils.c\n> > +++ b/xdiff/xutils.c\n> > @@ -248,12 +248,12 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,\n> > \t\t\tif (flags & XDF_IGNORE_WHITESPACE)\n> > \t\t\t\t; /* already handled */\n> > \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_CHANGE\n> > -\t\t\t\t\t&& ptr[1] != '\\n') {\n> > +\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n> > \t\t\t\tha += (ha << 5);\n> > \t\t\t\tha ^= (unsigned long) ' ';\n> > \t\t\t}\n> > \t\t\telse if (flags & XDF_IGNORE_WHITESPACE_AT_EOL\n> > -\t\t\t\t\t&& ptr[1] != '\\n') {\n> > +\t\t\t\t\t&& ptr[1] != '\\n' && ptr + 1 < top) {\n> > \t\t\t\twhile (ptr2 != ptr + 1) {\n> > \t\t\t\t\tha += (ha << 5);\n> > \t\t\t\t\tha ^= (unsigned long) *ptr2;\n> \n> Thanks.\n> \n> The issue you identified and tried to fix is a worthy one. But before the\n> pre-context of this hunk, I notice these lines:\n> \n> \t\tif (isspace(*ptr)) {\n> \t\t\tconst char *ptr2 = ptr;\n> \t\t\twhile (ptr + 1 < top && isspace(ptr[1])\n> \t\t\t\t\t&& ptr[1] != '\\n')\n> \t\t\t\tptr++;\n> \n> If you have trailing whitespaces on an incomplete line, ptr initially\n> points at the first such whitespace, ptr2 points at the same location, and\n> then the while() loop advances ptr to point at the last byte on the line,\n> which in turn will be the last byte of the file. And the codepath with\n> your updates still try to access ptr[1] that is beyond that last byte.\n> \n> I would write it like this patch instead.\n> \n> The intent is the same as your patch, but it avoids accessing ptr[1] when\n> that is beyond the end of the buffer, and the logic is easier to follow as\n> well.\n> \n\nI appreciate your taking the time to look at the issue and explaining the \nreasons for your change.\n\n> -- >8 --\n> Subject: xutils: fix hashing an incomplete line with whitespaces at the end\n> \n> Upon seeing a whitespace, xdl_hash_record_with_whitespace() first skipped\n> the run of whitespaces (excluding LF) that begins there, ensuring that the\n> pointer points the last whitespace character in the run, and assumed that\n> the next character must be LF at the end of the line. This does not work\n> when hashing an incomplete line, that lacks the LF at the end.\n> \n> Introduce \"at_eol\" variable that is true when either we are at the end of\n> line (looking at LF) or at the end of an incomplete line, and use that\n> instead throughout the code.\n> \n> Noticed by Thell Fowler.\n> \n> Signed-off-by: Junio C Hamano \n\nYeah... comparing this commit message to the original shows a pretty stark \ndifference. I'll get it 'the git way' eventually.\n\n-- \nThell\n"},{"id":"121572","messageId":"7vljlauxmk.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908231110500.29625@GWPortableVCS","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T19:40:51Z","receivedAt":"2009-08-23T19:40:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n> Because the flow is much more direct it also makes the test additions to \n> t4015 obsolete as they essentially tested for line end conditions instead \n> of whitespace (like they should have).\n\nYour patch 6/6 that added the tests were useful to find a bug I originally\nhad, which is the one below that is commented out.\n\n>> +\t\t/*\n>> +\t\t * If we do not want -b to imply --ignore-space-at-eol\n>> +\t\t * then you would need to add this:\n>> +\t\t *\n>> +\t\t * if (!(flags & XDF_IGNORE_WHITESPACE_AT_EOL))\n>> +\t\t *\treturn (s1 <= i1 && s2 <= i2);\n>> +\t\t *\n>> +\t\t */\n>> +\n>\n> While it would be nice to have -b and --ignore-space-at-eol be two \n> different options that could be merged together the documentation states \n> that -b ignores spaces at eol, and there are scripts that depend on this \n> behavior.\n\nAlso that is how \"diff -b\" behaves, and that is why I said your tests\nfound a _bug_ in my original. I'll drop the above large comment and\nreplace it with just a \"/* -b implies --ignore-space-at-eol */\".\n\n> Right now the xdl_recmatch() checks three distinct flags before having the \n> opportunity to do the default behavior of a straight diff. In \n> xdl_hash_record there is an initial check for whitespace flags.\n>\n> ...\n> \tif (flags & XDF_WHITESPACE_FLAGS)\n> \t\treturn xdl_hash_record_with_whitespace(data, top, flags);\n> ...\n>\n> Perhaps a similar setup for xdl_rematch() and a \n> xdl_recmatch_with_whitespace() ?\n\nOr we can just move the final else clause up and start the function like\nthis:\n\n\tint i1, i2;\n\n\tif (!(flags & XDF_WHITESPACE_FLAGS))\n \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n\n\ti1 = i2 = 0;\n \tif (flags & XDF_IGNORE_WHITESPACE) {\n\t\t...\n\nthat would get rid of two unnecessary clearing of variables (i1 and i2,\neven though I suspect that the compiler _could_ optimize them out without\nsuch an change), and three flags-bit check in the most common case of not\nignoring any whitespaces.\n\n> Since your to counter-proposals give the same results, provide safer and \n> faster processing, eliminate the additional test, as well as being easier \n> to read and comprehend I propose a v3 with just those two patches. I'll \n> be glad to post it, with or without a xdl_recmatch_with_whitespace, if \n> need be. And should I, or do I need to, add something to the commit (ie: \n> ack, tested, ...) ?\n\nI can amend the counterproposal patches with tests from your 6/6 and add\nyour \"Tested-by:\" and commit them myself.\n\n> Thank you again for taking the time to look at this change!\n\nThank _you_ for bringing this issue up in the first place.\n"},{"id":"121579","messageId":"alpine.DEB.2.00.0908231515020.29625@GWPortableVCS","threadId":"20376","inReplyTo":"7vljlauxmk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T20:33:58Z","receivedAt":"2009-08-23T20:33:58Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Junio C Hamano (gitster@pobox.com) wrote on Aug 23, 2009:\n\n> Thell Fowler writes:\n> \n> > Because the flow is much more direct it also makes the test additions to \n> > t4015 obsolete as they essentially tested for line end conditions instead \n> > of whitespace (like they should have).\n> \n> Your patch 6/6 that added the tests were useful to find a bug I originally\n> had, which is the one below that is commented out.\n> \n\nThat's good to hear!\n\n> >> +\t\t/*\n> >> +\t\t * If we do not want -b to imply --ignore-space-at-eol\n> >> +\t\t * then you would need to add this:\n> >> +\t\t *\n> >> +\t\t * if (!(flags & XDF_IGNORE_WHITESPACE_AT_EOL))\n> >> +\t\t *\treturn (s1 <= i1 && s2 <= i2);\n> >> +\t\t *\n> >> +\t\t */\n> >> +\n> >\n> > While it would be nice to have -b and --ignore-space-at-eol be two \n> > different options that could be merged together the documentation states \n> > that -b ignores spaces at eol, and there are scripts that depend on this \n> > behavior.\n> \n> Also that is how \"diff -b\" behaves, and that is why I said your tests\n> found a _bug_ in my original. I'll drop the above large comment and\n> replace it with just a \"/* -b implies --ignore-space-at-eol */\".\n> \n\nIn that case the only other outstanding issue to being able to use \npatch-id to validate a whitespace fixed patch is diff's -B option to catch \nthe situations where the original has multiple blank newlines at the end \nof file.\n\n\n> > Right now the xdl_recmatch() checks three distinct flags before having the \n> > opportunity to do the default behavior of a straight diff. In \n> > xdl_hash_record there is an initial check for whitespace flags.\n> >\n> > ...\n> > \tif (flags & XDF_WHITESPACE_FLAGS)\n> > \t\treturn xdl_hash_record_with_whitespace(data, top, flags);\n> > ...\n> >\n> > Perhaps a similar setup for xdl_rematch() and a \n> > xdl_recmatch_with_whitespace() ?\n> \n> Or we can just move the final else clause up and start the function like\n> this:\n> \n> \tint i1, i2;\n> \n> \tif (!(flags & XDF_WHITESPACE_FLAGS))\n> \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n> \n> \ti1 = i2 = 0;\n> \tif (flags & XDF_IGNORE_WHITESPACE) {\n> \t\t...\n> \n> that would get rid of two unnecessary clearing of variables (i1 and i2,\n> even though I suspect that the compiler _could_ optimize them out without\n> such an change), and three flags-bit check in the most common case of not\n> ignoring any whitespaces.\n> \n\nHA! That's a nifty way to do that with the variables.\n\n> > Since your to counter-proposals give the same results, provide safer and \n> > faster processing, eliminate the additional test, as well as being easier \n> > to read and comprehend I propose a v3 with just those two patches. I'll \n> > be glad to post it, with or without a xdl_recmatch_with_whitespace, if \n> > need be. And should I, or do I need to, add something to the commit (ie: \n> > ack, tested, ...) ?\n> \n> I can amend the counterproposal patches with tests from your 6/6 and add\n> your \"Tested-by:\" and commit them myself.\n> \n\nExcellent.\n\n> > Thank you again for taking the time to look at this change!\n> \n> Thank _you_ for bringing this issue up in the first place.\n\nMy pleasure! It has been quite the learning experience!\n\n-- \nThell\n"},{"id":"121586","messageId":"20090824060708.6117@nanako3.lavabit.com","threadId":"20376","inReplyTo":"7v1vn2yklo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-08-23T21:07:08Z","receivedAt":"2009-08-23T21:07:08Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano \n\n> Having said that, I could use something like this.\n>\n> -- >8 -- cut here -- >8 -- \n> Subject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n>\n> This teaches mailinfo the scissors -- >8 -- mark; the command ignores\n> everything before it in the message body.\n>\n> Signed-off-by: Junio C Hamano \n\nThere are left handed people whose scissors run in the wrong direction.\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex b0906ef..38c01e4 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -725,7 +725,8 @@ static int scissors(const struct strbuf *line)\n \t\t\tscissors_dashes_seen |= 02;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (i + 1 < len && !memcmp(buf + i, \">8\", 2)) {\n+\t\tif (i + 1 < len &&\n+\t\t !memcmp(buf + i, \">8\", 2) || !memcmp(buf + i, \"8<\", 2)) {\n \t\t\tscissors_dashes_seen |= 01;\n \t\t\ti++;\n \t\t\tcontinue;\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"121587","messageId":"7vzl9qtev0.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908231515020.29625@GWPortableVCS","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T21:11:31Z","receivedAt":"2009-08-23T21:11:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n>> Or we can just move the final else clause up and start the function like\n>> this:\n>> \n>> \tint i1, i2;\n>> \n>> \tif (!(flags & XDF_WHITESPACE_FLAGS))\n>> \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n>> \n>> \ti1 = i2 = 0;\n>> \tif (flags & XDF_IGNORE_WHITESPACE) {\n>> \t\t...\n>> \n>> that would get rid of two unnecessary clearing of variables (i1 and i2,\n>> even though I suspect that the compiler _could_ optimize them out without\n>> such an change), and three flags-bit check in the most common case of not\n>> ignoring any whitespaces.\n>> \n>\n> HA! That's a nifty way to do that with the variables.\n\nMy tentative draft to replace the \"how about this\" patch further reworks\nthe loop structure and currently looks like this.\n\nIt adds net 15 lines but among that 12 lines are comments, which is not so\nbad.\n\n-- >8 --\nSubject: [PATCH] xutils: Fix xdl_recmatch() on incomplete lines\n\nThell Fowler noticed that various \"ignore whitespace\" options to git diff\ndo not work well on an incomplete line.\n\nThe loop control of the function responsible for these bugs was extremely\ndifficult to follow. This patch restructures the loops for three variants\nof \"ignore whitespace\" logic.\n\nThe basic idea of the re-written logic is:\n\n - A loop runs while the characters from both strings we are looking at\n match. We declare unmatch immediately when we find something that does\n not match and return false from the function. We break out of the loop\n if we ran out of either side of the string.\n\n The way we skip spaces inside this loop varies depending on the style\n of ignoring whitespaces.\n\n - After the above loop breaks, we know that the parts of the strings we\n inspected so far match, ignoring the whitespaces. The lines can match\n only if the remainder consists of nothing but whitespaces. This part\n of the logic is shared across all three styles.\n\nThe new code is more obvious and should be much easier to follow.\n\nTested-by: Thell Fowler \nSigned-off-by: Junio C Hamano \n---\n xdiff/xutils.c | 77 +++++++++++++++++++++++++++++++++----------------------\n 1 files changed, 46 insertions(+), 31 deletions(-)\n\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 9411fa9..eb7b597 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -190,48 +190,63 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)\n {\n \tint i1, i2;\n \n+\tif (!(flags & XDF_WHITESPACE_FLAGS))\n+\t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n+\n+\ti1 = 0;\n+\ti2 = 0;\n+\n+\t/*\n+\t * -w matches everything that matches with -b, and -b in turn\n+\t * matches everything that matches with --ignore-space-at-eol.\n+\t *\n+\t * Each flavor of ignoring needs different logic to skip whitespaces\n+\t * while we have both sides to compare.\n+\t */\n \tif (flags & XDF_IGNORE_WHITESPACE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1]))\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n-\t\t\t\t\ti1++;\n-\t\t\tif (isspace(l2[i2]))\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n-\t\t\t\t\ti2++;\n-\t\t\tif (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])\n+\t\tgoto skip_ws;\n+\t\twhile (i1 < s1 && i2 < s2) {\n+\t\t\tif (l1[i1++] != l2[i2++])\n \t\t\t\treturn 0;\n+\t\tskip_ws:\n+\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n+\t\t\t\ti1++;\n+\t\t\twhile (i2 < s2 && isspace(l2[i2]))\n+\t\t\t\ti2++;\n \t\t}\n-\t\treturn (i1 >= s1 && i2 >= s2);\n \t} else if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (isspace(l1[i1])) {\n-\t\t\t\tif (!isspace(l2[i2]))\n-\t\t\t\t\treturn 0;\n-\t\t\t\twhile (isspace(l1[i1]) && i1 < s1)\n-\t\t\t\t\ti1++;\n-\t\t\t\twhile (isspace(l2[i2]) && i2 < s2)\n-\t\t\t\t\ti2++;\n-\t\t\t} else if (l1[i1++] != l2[i2++])\n-\t\t\t\treturn 0;\n-\t\t}\n-\t\treturn (i1 >= s1 && i2 >= s2);\n-\t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n-\t\tfor (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {\n-\t\t\tif (l1[i1] != l2[i2]) {\n+\t\twhile (i1 < s1 && i2 < s2) {\n+\t\t\tif (isspace(l1[i1]) && isspace(l2[i2])) {\n+\t\t\t\t/* Skip matching spaces and try again */\n \t\t\t\twhile (i1 < s1 && isspace(l1[i1]))\n \t\t\t\t\ti1++;\n \t\t\t\twhile (i2 < s2 && isspace(l2[i2]))\n \t\t\t\t\ti2++;\n-\t\t\t\tif (i1 < s1 || i2 < s2)\n-\t\t\t\t\treturn 0;\n-\t\t\t\treturn 1;\n+\t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (l1[i1++] != l2[i2++])\n+\t\t\t\treturn 0;\n+\t\t}\n+\t} else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL) {\n+\t\twhile (i1 < s1 && i2 < s2 && l1[i1++] == l2[i2++])\n+\t\t\t; /* keep going */\n+\t}\n+\n+\t/*\n+\t * After running out of one side, the remaining side must have\n+\t * nothing but whitespace for the lines to match.\n+\t */\n+\tif (i1 < s1) {\n+\t\twhile (i1 < s1 && isspace(l1[i1]))\n \t\t\ti1++;\n+\t\treturn (s1 == i1);\n+\t}\n+\tif (i2 < s2) {\n+\t\twhile (i2 < s2 && isspace(l2[i2]))\n \t\t\ti2++;\n-\t\t}\n-\t\treturn i1 >= s1 && i2 >= s2;\n-\t} else\n-\t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n+\t\treturn (s2 == i2);\n+\t}\n+\treturn 1;\n }\n \n static unsigned long xdl_hash_record_with_whitespace(char const **data,\n-- \n1.6.4.1.255.g5556a\n"},{"id":"121588","messageId":"7vtyzyteq4.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090824060708.6117@nanako3.lavabit.com","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T21:14:27Z","receivedAt":"2009-08-23T21:14:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> There are left handed people whose scissors run in the wrong direction.\n\nHeh.\n\n> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n> index b0906ef..38c01e4 100644\n> --- a/builtin-mailinfo.c\n> +++ b/builtin-mailinfo.c\n> @@ -725,7 +725,8 @@ static int scissors(const struct strbuf *line)\n> \t\t\tscissors_dashes_seen |= 02;\n> \t\t\tcontinue;\n> \t\t}\n> -\t\tif (i + 1 < len && !memcmp(buf + i, \">8\", 2)) {\n> +\t\tif (i + 1 < len &&\n> +\t\t !memcmp(buf + i, \">8\", 2) || !memcmp(buf + i, \"8<\", 2)) {\n> \t\t\tscissors_dashes_seen |= 01;\n\nYou need a pair of parentheses around the memcmp || memcmp.\n\nI'll squash that in.\n"},{"id":"121595","messageId":"alpine.DEB.2.00.0908231705200.29625@GWPortableVCS","threadId":"20376","inReplyTo":"20090824060708.6117@nanako3.lavabit.com","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-23T22:13:38Z","receivedAt":"2009-08-23T22:13:38Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Nanako Shiraishi (nanako3@lavabit.com) wrote on Aug 23, 2009:\n\n> Quoting Junio C Hamano \n> \n> > Having said that, I could use something like this.\n> >\n> > -- >8 -- cut here -- >8 -- \n> > Subject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n> >\n> > This teaches mailinfo the scissors -- >8 -- mark; the command ignores\n> > everything before it in the message body.\n> >\n> > Signed-off-by: Junio C Hamano \n> \n> There are left handed people whose scissors run in the wrong direction.\n> \n\nWoohoo! Glad the left handed people aren't being discriminated against. \n;)\n\nBTW - I'm happily using this and think it should be in git!\n\n-- \nThell\n"},{"id":"121596","messageId":"7v7hwurwmu.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908231705200.29625@GWPortableVCS","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-23T22:30:33Z","receivedAt":"2009-08-23T22:30:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n>> > Subject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n> ...\n> BTW - I'm happily using this and think it should be in git!\n\nThe one I sent out had two bugs. Please discard and replace it with a\nnewer one I'll be pushing out on 'pu' later today.\n"},{"id":"121606","messageId":"alpine.DEB.2.00.0908232044060.29625@GWPortableVCS","threadId":"20376","inReplyTo":"7vzl9qtev0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-24T03:26:56Z","receivedAt":"2009-08-24T03:26:56Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Junio C Hamano (gitster@pobox.com) wrote on Aug 23, 2009:\n\n> Thell Fowler writes:\n> \n> >> Or we can just move the final else clause up and start the function like\n> >> this:\n> >> \n> >> \tint i1, i2;\n> >> \n> >> \tif (!(flags & XDF_WHITESPACE_FLAGS))\n> >> \t\treturn s1 == s2 && !memcmp(l1, l2, s1);\n> >> \n> >> \ti1 = i2 = 0;\n> >> \tif (flags & XDF_IGNORE_WHITESPACE) {\n> >> \t\t...\n> >> \n> >> that would get rid of two unnecessary clearing of variables (i1 and i2,\n> >> even though I suspect that the compiler _could_ optimize them out without\n> >> such an change), and three flags-bit check in the most common case of not\n> >> ignoring any whitespaces.\n> >> \n> >\n> > HA! That's a nifty way to do that with the variables.\n> \n> My tentative draft to replace the \"how about this\" patch further reworks\n> the loop structure and currently looks like this.\n> \n> It adds net 15 lines but among that 12 lines are comments, which is not so\n> bad.\n> \n\nIt passed every test I threw at it, although it seemed to be a tad bit \nslower than the previous revision on my sample data so I ran the following \ncommand several times for both the previous and current version:\n\ntime for i in {1..10}; do ./t4015-diff-whitespace.sh>/dev/null && \n./t4015-diff-trailing-whitespace.sh >/dev/null; done\n\n\nAnd these results are fairly average on what I saw:\n\nPrevious version:\nreal\t2m32.669s\nuser\t0m44.051s\nsys\t1m34.702s\n\n\nCurrent version:\nreal\t2m56.818s\nuser\t0m47.671s\nsys\t1m46.723s\n\n--\nThell\n"},{"id":"121609","messageId":"20090824041608.GC3526@vidovic","threadId":"20376","inReplyTo":"7v7hwurwmu.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-24T04:16:08Z","receivedAt":"2009-08-24T04:16:08Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"Subject: [PATCH] Wong title\nFrom: is this one really the author? \n\nThe 23/08/09, Junio C Hamano wrote:\n> \n> >> > Subject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n> \n> The one I sent out had two bugs. Please discard and replace it with a\n> newer one I'll be pushing out on 'pu' later today.\n\n( Tested against current 925bd84 in pu. )\n\nIf we have a mail with this form\n\n
\n Subject: [PATCH] BLAH ONE\n
\n\n Subject: [PATCH] BLAH TWO\n <...>\n -- >8 --\n Subject: [PATCH] BLAH THREE\n\n\nthe commit message looks like\n\n BLAH TWO\n \n Subject: [PATCH] BLAH THREE\n\n\nI'd expect that we take the \"Subject: \" line after the mark and fallback\nto the header if missing.\n\nSame applies to the \"From: \" lines.\n\nThis mail should be usable to your own tests.\n\n-- >8 -- Please squash this to 925bd84 -- >8 --\n\n Signed-off-by: Nicolas Sebrecht \n---\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex fcacc94..0c9a791 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -138,6 +138,9 @@ The commit message is formed by the title taken from\nthe\n where the patch begins. Excess whitespace at the end of each\n line is automatically stripped.\n \n+If a line starts with a \"-- >8 --\" mark in the body of the message,\n+everything before (and the line itself) will be ignored.\n+\n The patch is expected to be inline, directly following the\n message. Any line that is of the form:\n\n-- \nNicolas Sebrecht\n"},{"id":"121611","messageId":"7vk50tq0g5.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090824041608.GC3526@vidovic","subject":"Re: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T04:51:06Z","receivedAt":"2009-08-24T04:51:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Sebrecht writes:\n\n> I'd expect that we take the \"Subject: \" line after the mark and fallback\n> to the header if missing.\n\nPatches welcome.\n"},{"id":"121612","messageId":"20090824141623.6117@nanako3.lavabit.com","threadId":"20376","inReplyTo":"20090824041608.GC3526@vidovic","subject":"Re: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-08-24T05:16:23Z","receivedAt":"2009-08-24T05:16:23Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Nicolas Sebrecht writes:\n\n> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\n> index fcacc94..0c9a791 100644\n> --- a/Documentation/git-am.txt\n> +++ b/Documentation/git-am.txt\n> @@ -138,6 +138,9 @@ The commit message is formed by the title taken from\n> the\n> where the patch begins. Excess whitespace at the end of each\n> line is automatically stripped.\n> \n> +If a line starts with a \"-- >8 --\" mark in the body of the message,\n> +everything before (and the line itself) will be ignored.\n\nLooking at the way other people use the mark in their messages, I think this explanation isn't correct.\n\nA scissors mark doesn't have to be at the beginning. The line has to contain the mark, and it has to consist of only the mark, '-' minus, the phrase \"cut here\", and whitespaces.\n\nI am not familiar enough with the code to comment on the bug you are reporting.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"121613","messageId":"7vmy5pojsg.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"7vk50tq0g5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T05:36:15Z","receivedAt":"2009-08-24T05:36:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Try this patch, perhaps? I forgot to reset the mysteriously named s_hdr\nbuffer.\n\nDoes anybody remember what these s_hdr (vs p_hdr) buffers stand for, by\nthe way?\n\n -- >8 -- cut here -- >8 --\nSubject: [PATCH] squashme to 925bd84 (Teach mailinfo to ignore everything before -- >8 -- mark, 2009-08-23)\n\n builtin-mailinfo.c | 6 ++++++\n t/t5100/sample.mbox | 4 ++++\n 2 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 26548f0..8a3a184 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -758,9 +758,15 @@ static int handle_commit_msg(struct strbuf *line)\n \t}\n \n \tif (scissors(line)) {\n+\t\tint i;\n \t\trewind(cmitmsg);\n \t\tftruncate(fileno(cmitmsg), 0);\n \t\tstill_looking = 1;\n+\t\tfor (i = 0; header[i]; i++) {\n+\t\t\tif (s_hdr_data[i])\n+\t\t\t\tstrbuf_release(s_hdr_data[i]);\n+\t\t\ts_hdr_data[i] = NULL;\n+\t\t}\n \t\treturn 0;\n \t}\n \ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex 95b6842..2c3da52 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -566,10 +566,14 @@ From: Junio Hamano \n Date: Thu, 20 Aug 2009 17:18:22 -0700\n Subject: Why doesn't git-am does not like >8 scissors mark?\n \n+Subject: [PATCH] BLAH ONE\n+\n In real life, we will see a discussion that inspired this patch\n discussing related and unrelated things around >8 scissors mark\n in this part of the message.\n \n+Subject: [PATCH] BLAH TWO\n+\n And the we will see the scissors.\n \n -- >8 -- cut here -- 8< --\n"},{"id":"121614","messageId":"7viqgdoikz.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908232044060.29625@GWPortableVCS","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T06:02:20Z","receivedAt":"2009-08-24T06:02:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler writes:\n\n> It passed every test I threw at it, although it seemed to be a tad bit \n> slower than the previous revision on my sample data so I ran the following \n> command several times for both the previous and current version:\n>\n> time for i in {1..10}; do ./t4015-diff-whitespace.sh>/dev/null && \n> ./t4015-diff-trailing-whitespace.sh >/dev/null; done\n>\n> And these results are fairly average on what I saw:\n>\n> Previous version:\n> real\t2m32.669s\n> user\t0m44.051s\n> sys\t1m34.702s\n>\n>\n> Current version:\n> real\t2m56.818s\n> user\t0m47.671s\n> sys\t1m46.723s\n\nDo you mean by \"previous version\" the one that was broken, or the one I\nsent as a \"how about\" patch?\n\nHere are the numbers I am getting:\n\n$ /usr/bin/time sh -c 'for i in 1 2 3 4 5 6 7 8 9 0; do ./t4015-diff-whitespace.sh; done' >/dev/null\n\n----------------\n\n1.99user 3.65system 0:05.10elapsed 110%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288675minor)pagefaults 0swaps\n\n1.86user 3.66system 0:05.04elapsed 109%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288618minor)pagefaults 0swaps\n\n1.76user 3.87system 0:05.02elapsed 112%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288973minor)pagefaults 0swaps\n\n----------------\n\n1.81user 3.86system 0:05.08elapsed 111%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288836minor)pagefaults 0swaps\n\n1.76user 3.87system 0:04.95elapsed 113%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288880minor)pagefaults 0swaps\n\n1.81user 3.88system 0:05.04elapsed 112%CPU (0avgtext+0avgdata 0maxresident)k\n0inputs+12560outputs (0major+1288530minor)pagefaults 0swaps\n\n----------------\n\nOne set is with patch and one set is the patch reverted. I cannot quite\nremember which one is which ;-) but the difference is within the noise for me.\n\nI have to revisit this sometime after getting a long rest.\n"},{"id":"121615","messageId":"20090824062141.GD3526@vidovic","threadId":"20376","inReplyTo":"7vmy5pojsg.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-24T06:21:41Z","receivedAt":"2009-08-24T06:21:41Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 23/08/09, Junio C Hamano wrote:\n\n> Try this patch, perhaps? I forgot to reset the mysteriously named s_hdr\n> buffer.\n\nNice. Please add\n\n\tTested-by: Nicolas Sebrecht \n\n> Does anybody remember what these s_hdr (vs p_hdr) buffers stand for, by\n> the way?\n\nHas been added by 87ab799234639c .\n\n-- \nNicolas Sebrecht\n"},{"id":"121617","messageId":"7v7hwtofys.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090824062141.GD3526@vidovic","subject":"Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T06:58:51Z","receivedAt":"2009-08-24T06:58:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Sebrecht writes:\n\n>> Does anybody remember what these s_hdr (vs p_hdr) buffers stand for, by\n>> the way?\n>\n> Has been added by 87ab799234639c .\n\nThat much I know ;-), thanks anyway.\n\nThe commit does not _explain_ what they are for, what they mean, and what\nthese mysteriously named variables do.\n"},{"id":"121619","messageId":"20090824071711.GE3526@vidovic","threadId":"20376","inReplyTo":"20090824141623.6117@nanako3.lavabit.com","subject":"[PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-24T07:17:11Z","receivedAt":"2009-08-24T07:17:11Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"[ Please, please, please, wrap your lines. ]\n\nThe 24/08/09, Nanako Shiraishi wrote:\n\n> Looking at the way other people use the mark in their messages, I think this explanation isn't correct.\n\nI'd say that should not document what people do but what the program\ndoes.\n\n> A scissors mark doesn't have to be at the beginning. The line has to contain the mark, and it has to consist of only the mark, '-' minus, the phrase \"cut here\", and whitespaces.\n\n...and (\">8\" or \"<8\"), you're right. But isn't the following mark a bit\ntoo much permissive?\n\n->8\nSubject: [PATCH] squashable to 925bd84 (Teach mailinfo to ignore everything before -- >8 -- mark, 2009-08-23)\n\nSigned-off-by: Nicolas Sebrecht \n---\n\nThis patch supersedes my previous round.\n\n Documentation/git-am.txt | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex fcacc94..1d10371 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -138,6 +138,10 @@ The commit message is formed by the title taken\nfrom the\n where the patch begins. Excess whitespace at the end of each\n line is automatically stripped.\n \n+If a line starts with a \"-- >8 --\" mark in the body of the message,\n+everything before (and the line itself) will be ignored.\n+Whitespaces and strings \"cut here\" are tolerated.\n+\n The patch is expected to be inline, directly following the\n message. Any line that is of the form:\n \n-- \nNicolas Sebrecht\n"},{"id":"121620","messageId":"20090824072458.GF3526@vidovic","threadId":"20376","inReplyTo":"20090824071711.GE3526@vidovic","subject":"[PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-24T07:24:58Z","receivedAt":"2009-08-24T07:24:58Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"( Paste error, sorry. )\n\nThe 24/08/09, Nicolas Sebrecht wrote:\n\n> [ Please, please, please, wrap your lines. ]\n> \n> The 24/08/09, Nanako Shiraishi wrote:\n> \n> > Looking at the way other people use the mark in their messages, I think this explanation isn't correct.\n> \n> I'd say that should not document what people do but what the program\n> does.\n> \n> > A scissors mark doesn't have to be at the beginning. The line has to contain the mark, and it has to consist of only the mark, '-' minus, the phrase \"cut here\", and whitespaces.\n> \n> ...and (\">8\" or \"<8\"), you're right. But isn't the following mark a bit\n> too much permissive?\n\n->8\nSubject: [PATCH] squashme to 925bd84 (Teach mailinfo to ignore everything before -- >8 -- mark, 2009-08-23)\n\n---\n Documentation/git-am.txt | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex fcacc94..5294d47 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -138,6 +138,12 @@ The commit message is formed by the title taken\nfrom the\n where the patch begins. Excess whitespace at the end of each\n line is automatically stripped.\n \n+If a line contains a mark in the body of the message, everything\n+before (and the line itself) will be ignored. A mark has typically\n+the form \"-- >8 -- cut here -- >8 --\". Strictly speacking, it must\n+have one dash at least and a \">8\" (or \"<8\"). Spaces and strings\n+\"cut here\" are permited.\n+\n The patch is expected to be inline, directly following the\n message. Any line that is of the form:\n \n-- \nNicolas Sebrecht\n"},{"id":"121621","messageId":"20090824073147.GG3526@vidovic","threadId":"20376","inReplyTo":"7v7hwtofys.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-24T07:31:47Z","receivedAt":"2009-08-24T07:31:47Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"( cc'ing Don Zickus )\n\nThe 23/08/09, Junio C Hamano wrote:\n> Nicolas Sebrecht writes:\n> \n> >> Does anybody remember what these s_hdr (vs p_hdr) buffers stand for, by\n> >> the way?\n> >\n> > Has been added by 87ab799234639c .\n> \n> That much I know ;-), thanks anyway.\n> \n> The commit does not _explain_ what they are for, what they mean, and what\n> these mysteriously named variables do.\n\n-- \nNicolas Sebrecht\n"},{"id":"299117","messageId":"20090824170906.6117@nanako3.lavabit.com","threadId":"20376","inReplyTo":"20090824141623.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-08-24T08:09:06Z","receivedAt":"2009-08-24T08:09:06Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting myself...\n\n> A scissors mark doesn't have to be at the beginning. The line has to\n> contain the mark, and it has to consist of only the mark, '-' minus, the\n> phrase \"cut here\", and whitespaces.\n\nJunio, perhaps you want to squash some documentation, too.\n\n-- 8< -- cut here -- 8< -- cut here -- 8< --\nSubject: [PATCH] Documentation: describe the scissors mark support of \"git am\"\n\nDescribe what a scissors mark looks like, and explain in what situation\nit is often used.\n\nSigned-off-by: Nanako Shiraishi \n---\n Documentation/git-am.txt | 16 ++++++++++++----\n 1 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex fcacc94..fecd5ac 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -128,10 +128,18 @@ the commit, after stripping common prefix \"[PATCH ]\".\n The \"Subject: \" line is supposed to concisely describe what the\n commit is about in one line of text.\n \n-\"From: \" and \"Subject: \" lines starting the body (the rest of the\n-message after the blank line terminating the RFC2822 headers)\n-override the respective commit author name and title values taken\n-from the headers.\n+A line that contains a scissors mark (either \">8\" or \"8<\") and does not\n+have anything other than scissors, dash (-), whitespaces or a phrase \"cut\n+here\" is called a scissors line. If such a line appears in the body of the\n+message before the patch, everything before it (including the scissors\n+line itself) is ignored. This is useful if you want to begin your message\n+in a discussion thread with comments and suggestions on the message you\n+are responding to, and to conclude it with a patch submission, separating\n+the discussion and the beginning of the proposed commit log message with a\n+scissors line.\n+\n+\"From: \" and \"Subject: \" lines starting the body override the respective\n+commit author name and title values taken from the headers.\n \n The commit message is formed by the title taken from the\n \"Subject: \", a blank line and the body of the message up to\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n\n"},{"id":"121629","messageId":"20090824140223.GA22198@redhat.com","threadId":"20376","inReplyTo":"20090824073147.GG3526@vidovic","subject":"Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2009-08-24T14:02:23Z","receivedAt":"2009-08-24T14:02:23Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Mon, Aug 24, 2009 at 09:31:47AM +0200, Nicolas Sebrecht wrote:\n> ( cc'ing Don Zickus )\n> \n> The 23/08/09, Junio C Hamano wrote:\n> > Nicolas Sebrecht writes:\n> > \n> > >> Does anybody remember what these s_hdr (vs p_hdr) buffers stand for, by\n> > >> the way?\n\n>From what I remember, I used p_hdr to designate primary headers, ie the\noriginal mail headers. s_hdr was supposed to represent the secondary\nheaders, ie the embedded mail headers in the body of the email that could\noverride the original primary mail headers.\n\nI hope that clears things up. Let me know if you have more questions and\nI will try my best to remember what I did. :-)\n\nCheers,\nDon\n\n> > >\n> > > Has been added by 87ab799234639c .\n> > \n> > That much I know ;-), thanks anyway.\n> > \n> > The commit does not _explain_ what they are for, what they mean, and what\n> > these mysteriously named variables do.\n> \n> -- \n> Nicolas Sebrecht\n"},{"id":"121634","messageId":"alpine.DEB.2.00.0908240910120.29625@GWPortableVCS","threadId":"20376","inReplyTo":"7viqgdoikz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-24T14:13:51Z","receivedAt":"2009-08-24T14:13:51Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Junio C Hamano (gitster@pobox.com) wrote on Aug 24, 2009:\n\n> Thell Fowler writes:\n> \n> > It passed every test I threw at it, although it seemed to be a tad bit \n> > slower than the previous revision on my sample data so I ran the following \n> > command several times for both the previous and current version:\n> >\n> \n> Do you mean by \"previous version\" the one that was broken, or the one I\n> sent as a \"how about\" patch?\n> \n\nA quick test shows the version merged to pu is the one that had the \nfastest times. I'll be away from a connection most of today, but will \ntest the different versions against the tests and some sample data and \npost back.\n\n--\nThell\n"},{"id":"121677","messageId":"7vhbvw7uig.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090824140223.GA22198@redhat.com","subject":"Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T21:48:55Z","receivedAt":"2009-08-24T21:48:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Don Zickus writes:\n\n> From what I remember, I used p_hdr to designate primary headers, ie the\n> original mail headers. s_hdr was supposed to represent the secondary\n> headers, ie the embedded mail headers in the body of the email that could\n> override the original primary mail headers.\n\nAh, p for primary and s for secondary. Now it makes sense.\n\nThanks.\n"},{"id":"121681","messageId":"7v3a7g501e.fsf@alter.siamese.dyndns.org","threadId":"20376","inReplyTo":"20090824071711.GE3526@vidovic","subject":"Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-24T22:17:49Z","receivedAt":"2009-08-24T22:17:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Sebrecht writes:\n\n> ... But isn't the following mark a bit\n> too much permissive?\n>\n> ->8\n\nYeah, I agree that we should require a bit longer perforation, and perhaps\nwe should tighten the rules a bit, while at the same time not limiting the\nrequest to cut to the exact phrase \"cut here\". As you pointed out, we do\nnot want to be too lenient to allow misidentification, but at the same\ntime it is nicer to be accomodating and treat something like this as a\nscissors line:\n\n - - - >8 - - - remove everything above this line - - - >8 - - -\n\nI think we have bikeshedded long enough, so I won't be touching this code\nany further only to change the definition of what a scissors mark looks\nlike, but here is what I did during lunch break, with another comment\nadded later to hint what s_hdr_data[] stands for after reading response\nfrom Don Zickus.\n\n-- >8 --\nFrom: Junio C Hamano \nSubject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n\nThis teaches mailinfo the scissors -- >8 -- mark; the command ignores\neverything before it in the message body.\n\nFor lefties among us, we also support -- 8< -- ;-)\n\nSigned-off-by: Junio C Hamano \n---\n builtin-mailinfo.c | 71 ++++++++++++++++++++++++++++++++++++++++-\n t/t5100-mailinfo.sh | 2 +-\n t/t5100/info0014 | 5 +++\n t/t5100/msg0014 | 4 ++\n t/t5100/patch0014 | 64 ++++++++++++++++++++++++++++++++++++\n t/t5100/sample.mbox | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 233 insertions(+), 2 deletions(-)\n create mode 100644 t/t5100/info0014\n create mode 100644 t/t5100/msg0014\n create mode 100644 t/t5100/patch0014\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex b0b5d8f..7e09b51 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -712,6 +712,56 @@ static inline int patchbreak(const struct strbuf *line)\n \treturn 0;\n }\n \n+static int is_scissors_line(const struct strbuf *line)\n+{\n+\tsize_t i, len = line->len;\n+\tint scissors = 0, gap = 0;\n+\tint first_nonblank = -1;\n+\tint last_nonblank = 0, visible, perforation, in_perforation = 0;\n+\tconst char *buf = line->buf;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tif (isspace(buf[i])) {\n+\t\t\tif (in_perforation) {\n+\t\t\t\tperforation++;\n+\t\t\t\tgap++;\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\t\tlast_nonblank = i;\n+\t\tif (first_nonblank < 0)\n+\t\t\tfirst_nonblank = i;\n+\t\tif (buf[i] == '-') {\n+\t\t\tin_perforation = 1;\n+\t\t\tperforation++;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (i + 1 < len &&\n+\t\t (!memcmp(buf + i, \">8\", 2) || !memcmp(buf + i, \"8<\", 2))) {\n+\t\t\tin_perforation = 1;\n+\t\t\tperforation += 2;\n+\t\t\tscissors += 2;\n+\t\t\ti++;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tin_perforation = 0;\n+\t}\n+\n+\t/*\n+\t * The mark must be at least 8 bytes long (e.g. \"-- >8 --\").\n+\t * Even though there can be arbitrary cruft on the same line\n+\t * (e.g. \"cut here\"), in order to avoid misidentification, the\n+\t * perforation must occupy more than a third of the visible\n+\t * width of the line, and dashes and scissors must occupy more\n+\t * than half of the perforation.\n+\t */\n+\n+\tvisible = last_nonblank - first_nonblank + 1;\n+\treturn (scissors && 8 <= visible &&\n+\t\tvisible < perforation * 3 &&\n+\t\tgap * 2 < perforation);\n+}\n+\n static int handle_commit_msg(struct strbuf *line)\n {\n \tstatic int still_looking = 1;\n@@ -723,7 +773,8 @@ static int handle_commit_msg(struct strbuf *line)\n \t\tstrbuf_ltrim(line);\n \t\tif (!line->len)\n \t\t\treturn 0;\n-\t\tif ((still_looking = check_header(line, s_hdr_data, 0)) != 0)\n+\t\tstill_looking = check_header(line, s_hdr_data, 0);\n+\t\tif (still_looking)\n \t\t\treturn 0;\n \t}\n \n@@ -731,6 +782,24 @@ static int handle_commit_msg(struct strbuf *line)\n \tif (metainfo_charset)\n \t\tconvert_to_utf8(line, charset.buf);\n \n+\tif (is_scissors_line(line)) {\n+\t\tint i;\n+\t\trewind(cmitmsg);\n+\t\tftruncate(fileno(cmitmsg), 0);\n+\t\tstill_looking = 1;\n+\n+\t\t/*\n+\t\t * We may have already read \"secondary headers\"; purge\n+\t\t * them to give ourselves a clean restart.\n+\t\t */\n+\t\tfor (i = 0; header[i]; i++) {\n+\t\t\tif (s_hdr_data[i])\n+\t\t\t\tstrbuf_release(s_hdr_data[i]);\n+\t\t\ts_hdr_data[i] = NULL;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \tif (patchbreak(line)) {\n \t\tfclose(cmitmsg);\n \t\tcmitmsg = NULL;\ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex e70ea94..e848556 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \\\n \t'git mailsplit -o. \"$TEST_DIRECTORY\"/t5100/sample.mbox >last &&\n \tlast=`cat last` &&\n \techo total is $last &&\n-\ttest `cat last` = 13'\n+\ttest `cat last` = 14'\n \n for mail in `echo 00*`\n do\ndiff --git a/t/t5100/info0014 b/t/t5100/info0014\nnew file mode 100644\nindex 0000000..ab9c8d0\n--- /dev/null\n+++ b/t/t5100/info0014\n@@ -0,0 +1,5 @@\n+Author: Junio C Hamano\n+Email: gitster@pobox.com\n+Subject: Teach mailinfo to ignore everything before -- >8 -- mark\n+Date: Thu, 20 Aug 2009 17:18:22 -0700\n+\ndiff --git a/t/t5100/msg0014 b/t/t5100/msg0014\nnew file mode 100644\nindex 0000000..259c6a4\n--- /dev/null\n+++ b/t/t5100/msg0014\n@@ -0,0 +1,4 @@\n+This teaches mailinfo the scissors -- >8 -- mark; the command ignores\n+everything before it in the message body.\n+\n+Signed-off-by: Junio C Hamano \ndiff --git a/t/t5100/patch0014 b/t/t5100/patch0014\nnew file mode 100644\nindex 0000000..124efd2\n--- /dev/null\n+++ b/t/t5100/patch0014\n@@ -0,0 +1,64 @@\n+---\n+ builtin-mailinfo.c | 37 ++++++++++++++++++++++++++++++++++++-\n+ 1 files changed, 36 insertions(+), 1 deletions(-)\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index b0b5d8f..461c47e 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -712,6 +712,34 @@ static inline int patchbreak(const struct strbuf *line)\n+ \treturn 0;\n+ }\n+ \n++static int scissors(const struct strbuf *line)\n++{\n++\tsize_t i, len = line->len;\n++\tint scissors_dashes_seen = 0;\n++\tconst char *buf = line->buf;\n++\n++\tfor (i = 0; i < len; i++) {\n++\t\tif (isspace(buf[i]))\n++\t\t\tcontinue;\n++\t\tif (buf[i] == '-') {\n++\t\t\tscissors_dashes_seen |= 02;\n++\t\t\tcontinue;\n++\t\t}\n++\t\tif (i + 1 < len && !memcmp(buf + i, \">8\", 2)) {\n++\t\t\tscissors_dashes_seen |= 01;\n++\t\t\ti++;\n++\t\t\tcontinue;\n++\t\t}\n++\t\tif (i + 7 < len && !memcmp(buf + i, \"cut here\", 8)) {\n++\t\t\ti += 7;\n++\t\t\tcontinue;\n++\t\t}\n++\t\t/* everything else --- not scissors */\n++\t\tbreak;\n++\t}\n++\treturn scissors_dashes_seen == 03;\n++}\n++\n+ static int handle_commit_msg(struct strbuf *line)\n+ {\n+ \tstatic int still_looking = 1;\n+@@ -723,10 +751,17 @@ static int handle_commit_msg(struct strbuf *line)\n+ \t\tstrbuf_ltrim(line);\n+ \t\tif (!line->len)\n+ \t\t\treturn 0;\n+-\t\tif ((still_looking = check_header(line, s_hdr_data, 0)) != 0)\n++\t\tstill_looking = check_header(line, s_hdr_data, 0);\n++\t\tif (still_looking)\n+ \t\t\treturn 0;\n+ \t}\n+ \n++\tif (scissors(line)) {\n++\t\tfseek(cmitmsg, 0L, SEEK_SET);\n++\t\tstill_looking = 1;\n++\t\treturn 0;\n++\t}\n++\n+ \t/* normalize the log message to UTF-8. */\n+ \tif (metainfo_charset)\n+ \t\tconvert_to_utf8(line, charset.buf);\n+-- \n+1.6.4.1\ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex c3074ac..13fa4ae 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -561,3 +561,92 @@ From: (A U Thor)\n Date: Fri, 9 Jun 2006 00:44:16 -0700\n Subject: [PATCH] a patch\n \n+From nobody Mon Sep 17 00:00:00 2001\n+From: Junio Hamano \n+Date: Thu, 20 Aug 2009 17:18:22 -0700\n+Subject: Why doesn't git-am does not like >8 scissors mark?\n+\n+Subject: [PATCH] BLAH ONE\n+\n+In real life, we will see a discussion that inspired this patch\n+discussing related and unrelated things around >8 scissors mark\n+in this part of the message.\n+\n+Subject: [PATCH] BLAH TWO\n+\n+And then we will see the scissors.\n+\n+ This line is not a scissors mark -- >8 -- but talks about it.\n+ - - >8 - - please remove everything above this line - - >8 - -\n+\n+Subject: [PATCH] Teach mailinfo to ignore everything before -- >8 -- mark\n+From: Junio C Hamano \n+\n+This teaches mailinfo the scissors -- >8 -- mark; the command ignores\n+everything before it in the message body.\n+\n+Signed-off-by: Junio C Hamano \n+---\n+ builtin-mailinfo.c | 37 ++++++++++++++++++++++++++++++++++++-\n+ 1 files changed, 36 insertions(+), 1 deletions(-)\n+\n+diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n+index b0b5d8f..461c47e 100644\n+--- a/builtin-mailinfo.c\n++++ b/builtin-mailinfo.c\n+@@ -712,6 +712,34 @@ static inline int patchbreak(const struct strbuf *line)\n+ \treturn 0;\n+ }\n+ \n++static int scissors(const struct strbuf *line)\n++{\n++\tsize_t i, len = line->len;\n++\tint scissors_dashes_seen = 0;\n++\tconst char *buf = line->buf;\n++\n++\tfor (i = 0; i < len; i++) {\n++\t\tif (isspace(buf[i]))\n++\t\t\tcontinue;\n++\t\tif (buf[i] == '-') {\n++\t\t\tscissors_dashes_seen |= 02;\n++\t\t\tcontinue;\n++\t\t}\n++\t\tif (i + 1 < len && !memcmp(buf + i, \">8\", 2)) {\n++\t\t\tscissors_dashes_seen |= 01;\n++\t\t\ti++;\n++\t\t\tcontinue;\n++\t\t}\n++\t\tif (i + 7 < len && !memcmp(buf + i, \"cut here\", 8)) {\n++\t\t\ti += 7;\n++\t\t\tcontinue;\n++\t\t}\n++\t\t/* everything else --- not scissors */\n++\t\tbreak;\n++\t}\n++\treturn scissors_dashes_seen == 03;\n++}\n++\n+ static int handle_commit_msg(struct strbuf *line)\n+ {\n+ \tstatic int still_looking = 1;\n+@@ -723,10 +751,17 @@ static int handle_commit_msg(struct strbuf *line)\n+ \t\tstrbuf_ltrim(line);\n+ \t\tif (!line->len)\n+ \t\t\treturn 0;\n+-\t\tif ((still_looking = check_header(line, s_hdr_data, 0)) != 0)\n++\t\tstill_looking = check_header(line, s_hdr_data, 0);\n++\t\tif (still_looking)\n+ \t\t\treturn 0;\n+ \t}\n+ \n++\tif (scissors(line)) {\n++\t\tfseek(cmitmsg, 0L, SEEK_SET);\n++\t\tstill_looking = 1;\n++\t\treturn 0;\n++\t}\n++\n+ \t/* normalize the log message to UTF-8. */\n+ \tif (metainfo_charset)\n+ \t\tconvert_to_utf8(line, charset.buf);\n+-- \n+1.6.4.1\n-- \n1.6.4.1\n"},{"id":"121713","messageId":"alpine.DEB.2.00.0908250018510.9656@GWPortableVCS","threadId":"20376","inReplyTo":"alpine.DEB.2.00.0908240910120.29625@GWPortableVCS","subject":"Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-08-25T05:58:21Z","receivedAt":"2009-08-25T05:58:21Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"Thell Fowler (git@tbfowler.name) wrote on Aug 24, 2009:\n\n> Junio C Hamano (gitster@pobox.com) wrote on Aug 24, 2009:\n> \n> > Thell Fowler writes:\n> > \n> > > It passed every test I threw at it, although it seemed to be a tad bit \n> > > slower than the previous revision on my sample data so I ran the following \n> > > command several times for both the previous and current version:\n> > >\n> > \n> > Do you mean by \"previous version\" the one that was broken, or the one I\n> > sent as a \"how about\" patch?\n> > \n> \n> A quick test shows the version merged to pu is the one that had the \n> fastest times. I'll be away from a connection most of today, but will \n> test the different versions against the tests and some sample data and \n> post back.\n> \n\nMore extensive testing also shows the version currently in pu is the \nfastest on my sample data when applied to master. I'm not sure why pu \nshows slower times than those same commits applied to master, but they \nare close enough together that I'm guessing no-one would really be \nconcerned.\n\nI was sitting in a waiting room and decided to have a little fun figuring \nout how to average the sys times...\n\nfor arg in \"\" -w -b --ignore-space-at-eol;do sum=0 && for i in {1..50}; \\\ndo n=\"$(/usr/bin/time -f \"%S\" -o /dev/stdout sh -c 'git diff $arg dirty_first>/dev/null;')\"; \\\nsum=$sum+$n; done; echo \"scale=2; ($sum)/$i\"|echo \"$(bc) avg for diff $arg\"; done;\n\npu\n.28 avg for diff \n.29 avg for diff -w\n.33 avg for diff -b\n.29 avg for diff --ignore-space-at-eol\n\npu commits applied to master <=== FASTEST\n9c0d402 xutils: Fix xdl_recmatch() on incomplete lines\n21245fd xutils: Fix hashing an incomplete line with whitespaces at the end\n.26 avg for diff \n.25 avg for diff -w\n.29 avg for diff -b\n.31 avg for diff --ignore-space-at-eol \n\n'how about' patch applied to master\n.26 avg for diff \n.32 avg for diff -w\n.29 avg for diff -b\n.32 avg for diff --ignore-space-at-eol\n\ncurrent master (in order to see the difference in the basic git diff \n-ignoring the fact that incomplete lines where broke since it only affects \n2 files in the test data)\n.30 avg for diff \n.30 avg for diff -w\n.29 avg for diff -b\n.29 avg for diff --ignore-space-at-eol\n\n-- Thell\n"},{"id":"121732","messageId":"fc2ecb5cf28cabb7d183e2835ce46aa9afb2a322.1251215299.git.nicolas.s.dev@gmx.fr","threadId":"20376","inReplyTo":"7v3a7g501e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Re: Teach mailinfo to ignore everything before -- >8 -- mark","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-08-25T16:18:30Z","receivedAt":"2009-08-25T16:18:30Z","isPatch":true,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 24/08/09, Junio C Hamano wrote:\n\n> perhaps \n> we should tighten the rules a bit,\n\n<...>\n\n> I think we have bikeshedded long enough, so I won't be touching this code\n> any further only to change the definition of what a scissors mark looks\n> like,\n\nI'm not sure I understand. Are you still open to a patch touching this code\n/too/?\n\nAnyway, here's what I wrote based on your last round in pu. I've change the\nrules to something because I think we'd rather simple ― and \"easy\" to \nexplain to the end-user ― rules over \"obfuscated\" ones.\n\n-- >8 -- squashable to 8683eeb (ogirin/pu) -- >8 --\n\nSubject: Teach mailinfo to ignore everything before a scissors line\n\nThis teaches mailinfo the scissors mark (e.g. \"-- >8 --\");\nthe command ignores everything before it in the message body.\n\nFor lefties among us, we also support -- 8< -- ;-)\n\nWe can skip this check using the \"--ignore-scissors\" option on both\nthe git-mailinfo and the git-am command line. This is necessary\nbecause the stripped message may be either\n\n interesting from the eyes of the maintainer, regardless what the\n author think;\n\nor\n\n the scissors line check is a false positive.\n\n\nBasically, the rules are:\n\n(1) a scissors mark:\n\n - must be 8 characters long;\n - must have a dash;\n - must have either \">8\" or \"<8\";\n - may contain spaces.\n\n(2) a scissors line:\n\n - must have only one scissors mark;\n or\n - must have any comment between two identical scissors marks;\n - always ignore spaces outside the scissors marks.\n\n\nSigned-off-by: Nicolas Sebrecht \n---\n Documentation/git-am.txt | 14 +++++-\n Documentation/git-mailinfo.txt | 7 ++-\n builtin-mailinfo.c | 103 +++++++++++++++++++++++----------------\n git-am.sh | 14 ++++-\n 4 files changed, 90 insertions(+), 48 deletions(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex fcacc94..2773a3e 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [--3way] [--interactive] [--committer-date-is-author-date]\n \t [--ignore-date] [--ignore-space-change | --ignore-whitespace]\n \t [--whitespace=