{"thread":{"id":"42478","subject":"[PATCH 2/2] Fix tests","startedAt":"2016-05-29T15:36:21Z","lastAt":"2016-05-30T00:30:59Z","messageCount":5,"participants":["Dave Nicolson","Thomas Gummerer","René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"287754","messageId":"01020154fd28aaa8-7c49c7ef-eed2-4092-80c5-6f43986f174a-000000@eu-west-1.amazonses.com","threadId":"42478","inReplyTo":"01020154fd28aa12-f536bf3e-58df-4d1f-b903-929b429954d3-000000@eu-west-1.amazonses.com","subject":"[PATCH 2/2] Fix tests","fromName":"Dave Nicolson","fromEmail":"david.nicolson@gmail.com","sentAt":"2016-05-29T15:36:21Z","receivedAt":"2016-05-29T15:36:21Z","isPatch":true,"sender":{"key":"david.nicolson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2276355?v=4"},"body":"---\n t/lib-diff-alternative.sh        | 4 ++--\n t/t4029-diff-trailing-space.sh   | 2 +-\n t/t4034-diff-words.sh            | 2 +-\n t/t4051-diff-function-context.sh | 6 +++---\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/lib-diff-alternative.sh b/t/lib-diff-alternative.sh\nindex 8b4dbf2..4a5d9d0 100644\n--- a/t/lib-diff-alternative.sh\n+++ b/t/lib-diff-alternative.sh\n@@ -66,7 +66,7 @@ index 6faa5a3..e3af329 100644\n +++ b/file2\n @@ -1,26 +1,25 @@\n  #include <stdio.h>\n- \n+\n +int fib(int n)\n +{\n +    if(n > 2)\n@@ -86,7 +86,7 @@ index 6faa5a3..e3af329 100644\n          printf(\"%d\\n\", foo);\n      }\n  }\n- \n+\n -int fact(int n)\n -{\n -    if(n > 1)\ndiff --git a/t/t4029-diff-trailing-space.sh b/t/t4029-diff-trailing-space.sh\nindex 3ccc237..751469c 100755\n--- a/t/t4029-diff-trailing-space.sh\n+++ b/t/t4029-diff-trailing-space.sh\n@@ -27,7 +27,7 @@ test_expect_success \\\n      git config --bool diff.suppressBlankEmpty true &&\n      git diff f > actual &&\n      test_cmp exp actual &&\n-     perl -i.bak -p -e \"s/^\\$/ /\" exp &&\n+     perl -i.bak -p -e \"s/^\\$//\" exp &&\n      git config --bool diff.suppressBlankEmpty false &&\n      git diff f > actual &&\n      test_cmp exp actual &&\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex f2f55fc..7458e61 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -106,7 +106,7 @@ test_expect_success '--word-diff=porcelain' '\n \t\t-h(4)\n \t\t+h(4),hh[44]\n \t\t~\n-\t\t # significant space\n+\t\t# significant space\n \t\t~\n \t\t a = b + c\n \t\t~\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 001d678..f786139 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -57,7 +57,7 @@ diff --git a/hello.c b/hello.c\n  {\n -\t/* Classic. */\n  \tprintf(\"Hello world.\\n\");\n- \n+\n  \t/* Success! */\n  \treturn 0;\n  }\n@@ -73,12 +73,12 @@ diff --git a/hello.c b/hello.c\n --- a/hello.c\n +++ b/hello.c\n @@ -9,9 +9,8 @@ static int a(void)\n- \n+\n  static int hello_world(void)\n  {\n -\t/* Classic. */\n  \tprintf(\"Hello world.\\n\");\n- \n+\n  \t/* Success! */\n  \treturn 0;\n  }\n\n--\nhttps://github.com/git/git/pull/245\n"},{"id":"287755","messageId":"01020154fd28aa12-f536bf3e-58df-4d1f-b903-929b429954d3-000000@eu-west-1.amazonses.com","threadId":"42478","inReplyTo":null,"subject":"[PATCH 1/2] Do not output whitespace on blank lines","fromName":"Dave Nicolson","fromEmail":"david.nicolson@gmail.com","sentAt":"2016-05-29T15:36:21Z","receivedAt":"2016-05-29T15:36:21Z","isPatch":true,"sender":{"key":"david.nicolson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2276355?v=4"},"body":"---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex d3734d3..459b36a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -471,7 +471,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n \t\thas_trailing_carriage_return = (len > 0 && line[len-1] == '\\r');\n \t\tif (has_trailing_carriage_return)\n \t\t\tlen--;\n-\t\tnofirst = 0;\n+\t\tnofirst = len == 0 && (char)first == ' ' ? 1 : 0;\n \t}\n \n \tif (len || !nofirst) {\n\n--\nhttps://github.com/git/git/pull/245\n"},{"id":"287757","messageId":"20160529170637.GC2286@hank","threadId":"42478","inReplyTo":"01020154fd28aa12-f536bf3e-58df-4d1f-b903-929b429954d3-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH 1/2] Do not output whitespace on blank lines","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2016-05-29T17:06:37Z","receivedAt":"2016-05-29T17:06:37Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 05/29, Dave Nicolson wrote:\n> ---\n\nThe commit message should describe why you think this is a good\nchange.  Without the commit message I don't see how this is a\nimprovement.  It is also missing your Sign-off (see\nDocumentation/SubmittingPatches section 5).\n\nThe test changes you sent in Patch 2/2 should be in this commit,\notherwise the git bisect is broken when it hits this patch, which is\nsomething we want to avoid.\n\n>  diff.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/diff.c b/diff.c\n> index d3734d3..459b36a 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -471,7 +471,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n>  \t\thas_trailing_carriage_return = (len > 0 && line[len-1] == '\\r');\n>  \t\tif (has_trailing_carriage_return)\n>  \t\t\tlen--;\n> -\t\tnofirst = 0;\n> +\t\tnofirst = len == 0 && (char)first == ' ' ? 1 : 0;\n>  \t}\n>  \n>  \tif (len || !nofirst) {\n> \n> --\n> https://github.com/git/git/pull/245\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\n-- \nThomas\n"},{"id":"287760","messageId":"574B25F4.8090409@web.de","threadId":"42478","inReplyTo":"01020154fd28aa12-f536bf3e-58df-4d1f-b903-929b429954d3-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH 1/2] Do not output whitespace on blank lines","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-05-29T17:25:08Z","receivedAt":"2016-05-29T17:25:08Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.05.2016 um 17:36 schrieb Dave Nicolson:\n> ---\n\ngit diff marks context lines (in unified diff format) with a preceding \nspace character.  Your intent is to remove that marker for empty context \nlines, right?  Why?  How much smaller do diffs get by that (assuming \noutput size reduction is one of the reasons)?\n\nHow compatible is it with patch, git apply and other programs that \nhandle diffs?  Interestingly in that context, POSIX [*] allows it, \nsaying: \"It is implementation-defined whether an empty unaffected line \nis written as an empty line or a line containing a single <space> \ncharacter.\"\n\nAlso: missing sign-off (see Documentation/SubmittingPatches).\n\n>   diff.c | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/diff.c b/diff.c\n> index d3734d3..459b36a 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -471,7 +471,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n>   \t\thas_trailing_carriage_return = (len > 0 && line[len-1] == '\\r');\n>   \t\tif (has_trailing_carriage_return)\n>   \t\t\tlen--;\n> -\t\tnofirst = 0;\n> +\t\tnofirst = len == 0 && (char)first == ' ' ? 1 : 0;\n\nWhy the cast from int to char?  And you also don't need the \"? 1 : 0\" part.\n\n>   \t}\n>\n>   \tif (len || !nofirst) {\n>\n> --\n> https://github.com/git/git/pull/245\n\nYour second patch adjusts tests.  It would be better to combine the two \nin order to keep tests working with each commit.\n\nRené\n\n\n[*] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html\n"},{"id":"287769","messageId":"xmqqa8j8pet8.fsf@gitster.mtv.corp.google.com","threadId":"42478","inReplyTo":"574B25F4.8090409@web.de","subject":"Re: [PATCH 1/2] Do not output whitespace on blank lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-30T00:30:59Z","receivedAt":"2016-05-30T00:30:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 29.05.2016 um 17:36 schrieb Dave Nicolson:\n>> ---\n>\n> git diff marks context lines (in unified diff format) with a preceding\n> space character.  Your intent is to remove that marker for empty\n> context lines, right?  Why?  How much smaller do diffs get by that\n> (assuming output size reduction is one of the reasons)?\n>\n> How compatible is it with patch, git apply and other programs that\n> handle diffs?  Interestingly in that context, POSIX [*] allows it,\n> saying: \"It is implementation-defined whether an empty unaffected line\n> is written as an empty line or a line containing a single <space>\n> character.\"\n\nIndeed POSIX allows it, and I am aware of a push to GNU diff some\ntime ago that making this possible (I do not offhand know if it was\na move to make it default, though), which is why \"git apply\" does\ntolerate an empty line in the common context that is not a single SP\non a line since b507b465 (git-apply: prepare for upcoming GNU diff\n-u format change., 2006-10-19).\n\nBut that does not mean we should flip the default without any\njustification like this patch does.\n\nFWIW, GNU diffutils 3.3 seems _not_ to default to this behaviour,\nand require you to say --suppress-blank-empty when asking for this\nform of output.  The rationale for this option in their\ndocumentation is:\n\n    Some text editors and email agents routinely delete trailing\n    blanks, so it can be a problem to deal with diff output files\n    that contain them. You can avoid this problem with the\n    --suppress-blank-empty option. It causes diff to omit trailing\n    blanks at the end of output lines in normal, context, and\n    unified format, unless the trailing blanks were already present\n    in the input.\n\nIf you think about this, the above does not make _any_ sense as a\njustification for that feature.  \"Some editors tend to remove SPs\nthat you do want if they appear at the end of line, which is a\nproblem.  To _AVOID_ this problem, we give an option not to produce\nthe SP that may be eaten by such editors in the first place\".\n\nSeriously?  The reason why you do want them in the first place is\nbecause eating them silently would cause trouble on the receiving\nend, so it is very understandable to prepare the side that _uses_\ndiff output to accept such an output that is missing the SPs at the\nend of the line.  That justification does not make sense for the\nside that _generates_ output at all.\n\nIn any case, I am not strongly opposed to add --suppress-blank-empty\noption to \"git diff\" family, even though I do not see much point in\nit.  I'll not accept a change to make that the default, until \"diff\"\nimplementations everybody else uses uses such default and POSIX\nstarts saying \"a single SP on a line to represent an empty shared\ncontext is deprecated\".\n"}]}