{"thread":{"id":"30369","subject":"git log -z still outputting newlines?","startedAt":"2012-04-30T17:58:04Z","lastAt":"2012-05-01T20:03:21Z","messageCount":17,"participants":["Randal L. Schwartz","Andreas Schwab","Jan Krüger","Thomas Rast","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"190324","messageId":"86ty01qez7.fsf@red.stonehenge.com","threadId":"30369","inReplyTo":null,"subject":"git log -z still outputting newlines?","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-04-30T17:58:04Z","receivedAt":"2012-04-30T17:58:04Z","isPatch":false,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":"\n$ git log -z --format='%cE' -5 | od -c\n0000000 g i t s t e r @ p o b o x . c o\n0000020 m \\n g i t s t e r @ p o b o x .\n0000040 c o m \\n g i t s t e r @ p o b o\n0000060 x . c o m \\n g i t s t e r @ p o\n0000100 b o x . c o m \\n g i t s t e r @\n0000120 p o b o x . c o m \\n \n0000132\n\nWhy are all those newlines in there? Bug? Misfeature? Feature? If\nfeature, how do I ensure \\0 in my output? If I add %x00, I get both \\0\n*and* \\n in output. :(\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"},{"id":"190327","messageId":"m2pqaprrup.fsf@igel.home","threadId":"30369","inReplyTo":"86ty01qez7.fsf@red.stonehenge.com","subject":"Re: git log -z still outputting newlines?","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-30T18:34:38Z","receivedAt":"2012-04-30T18:34:38Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"merlyn@stonehenge.com (Randal L. Schwartz) writes:\n\n> $ git log -z --format='%cE' -5 | od -c\n> 0000000 g i t s t e r @ p o b o x . c o\n> 0000020 m \\n g i t s t e r @ p o b o x .\n> 0000040 c o m \\n g i t s t e r @ p o b o\n> 0000060 x . c o m \\n g i t s t e r @ p o\n> 0000100 b o x . c o m \\n g i t s t e r @\n> 0000120 p o b o x . c o m \\n \n> 0000132\n>\n> Why are all those newlines in there? Bug? Misfeature? Feature? If\n> feature, how do I ensure \\0 in my output? If I add %x00, I get both \\0\n> *and* \\n in output. :(\n\n--format=format:%cE respects the -z option.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"190329","messageId":"1335811555-23564-1-git-send-email-jk@jk.gs","threadId":"30369","inReplyTo":"86ty01qez7.fsf@red.stonehenge.com","subject":"[PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2012-04-30T18:45:55Z","receivedAt":"2012-04-30T18:45:55Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"When using a custom format in line termination mode (as opposed to line\nseparation mode), the configured line terminator is not used, so things\nlike \"git log --pretty=tformat:%H -z\" do not work properly.\n\nMake it use the line terminator the user ordered.\n\nSigned-off-by: Jan Krüger \n---\n log-tree.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 34c49e7..44f0268 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n \tif (opt->use_terminator) {\n \t\tif (!opt->missing_newline)\n \t\t\tgraph_show_padding(opt->graph);\n-\t\tputchar('\\n');\n+\t\tputchar(opt->diffopt.line_termination);\n \t}\n \n \tstrbuf_release(&msgbuf);\n-- \n1.7.10\n"},{"id":"190332","messageId":"87zk9toxes.fsf@thomas.inf.ethz.ch","threadId":"30369","inReplyTo":"m2pqaprrup.fsf@igel.home","subject":"Re: git log -z still outputting newlines?","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-04-30T19:02:51Z","receivedAt":"2012-04-30T19:02:51Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Andreas Schwab writes:\n\n> merlyn@stonehenge.com (Randal L. Schwartz) writes:\n>\n>> $ git log -z --format='%cE' -5 | od -c\n>> 0000000 g i t s t e r @ p o b o x . c o\n>> 0000020 m \\n g i t s t e r @ p o b o x .\n>> 0000040 c o m \\n g i t s t e r @ p o b o\n>> 0000060 x . c o m \\n g i t s t e r @ p o\n>> 0000100 b o x . c o m \\n g i t s t e r @\n>> 0000120 p o b o x . c o m \\n \n>> 0000132\n>>\n>> Why are all those newlines in there? Bug? Misfeature? Feature? If\n>> feature, how do I ensure \\0 in my output? If I add %x00, I get both \\0\n>> *and* \\n in output. :(\n>\n> --format=format:%cE respects the -z option.\n\nThe underlying problem is apparently that --format=%cE triggers the\nformat-guessing logic, which assumes you meant --pretty=tformat:%cE\ninstead of --pretty=format:%cE.\n\nIt's probably a bug that --pretty=tformat:%cE does not use \\0 here.\nAfter all the manual states\n\n · tformat:\n\n The tformat: format works exactly like format:, except that it\n provides \"terminator\" semantics instead of \"separator\" semantics.\n\nFixing it may be as easy as the patch below, but I haven't spent much\ntime on it.\n\ndiff --git i/log-tree.c w/log-tree.c\nindex 34c49e7..44f0268 100644\n--- i/log-tree.c\n+++ w/log-tree.c\n@@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n \tif (opt->use_terminator) {\n \t\tif (!opt->missing_newline)\n \t\t\tgraph_show_padding(opt->graph);\n-\t\tputchar('\\n');\n+\t\tputchar(opt->diffopt.line_termination);\n \t}\n \n \tstrbuf_release(&msgbuf);\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"190337","messageId":"7vaa1tf1w8.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"1335811555-23564-1-git-send-email-jk@jk.gs","subject":"Re: [PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T19:36:07Z","receivedAt":"2012-04-30T19:36:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Krüger writes:\n\n> When using a custom format in line termination mode (as opposed to line\n> separation mode), the configured line terminator is not used, so things\n> like \"git log --pretty=tformat:%H -z\" do not work properly.\n>\n> Make it use the line terminator the user ordered.\n>\n> Signed-off-by: Jan Krüger \n> ---\n> log-tree.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/log-tree.c b/log-tree.c\n> index 34c49e7..44f0268 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n> \tif (opt->use_terminator) {\n> \t\tif (!opt->missing_newline)\n> \t\t\tgraph_show_padding(opt->graph);\n> -\t\tputchar('\\n');\n> +\t\tputchar(opt->diffopt.line_termination);\n> \t}\n> \n> \tstrbuf_release(&msgbuf);\n\nLooks sensible. Perhaps we would want to add a test?\n"},{"id":"190340","messageId":"1335817705-24718-1-git-send-email-jk@jk.gs","threadId":"30369","inReplyTo":"7vaa1tf1w8.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2012-04-30T20:28:25Z","receivedAt":"2012-04-30T20:28:25Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"When using a custom format in line termination mode (as opposed to line\nseparation mode), the configured line terminator is not used, so things\nlike \"git log --pretty=tformat:%H -z\" do not work properly.\n\nMake it use the line terminator the user ordered.\n\nSigned-off-by: Jan Krüger \n---\n Here are two simple tests, for both format: and tformat: with -z.\n\n log-tree.c | 2 +-\n t/t4205-log-pretty-formats.sh | 12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 34c49e7..44f0268 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n \tif (opt->use_terminator) {\n \t\tif (!opt->missing_newline)\n \t\t\tgraph_show_padding(opt->graph);\n-\t\tputchar('\\n');\n+\t\tputchar(opt->diffopt.line_termination);\n \t}\n \n \tstrbuf_release(&msgbuf);\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 2ae9faa..03a73ba 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -71,4 +71,16 @@ test_expect_success 'alias loop' '\n \ttest_must_fail git log --pretty=test-foo\n '\n \n+printf \"add bar\\0initial\" > expected\n+test_expect_success 'NUL separation' '\n+\tgit log -z --pretty=\"format:%s\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+printf \"add bar\\0initial\\0\" > expected\n+test_expect_success 'NUL termination' '\n+\tgit log -z --pretty=\"tformat:%s\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.7.10.406.g0017\n"},{"id":"190362","messageId":"7vd36oesjr.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"1335817705-24718-1-git-send-email-jk@jk.gs","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-30T22:58:00Z","receivedAt":"2012-04-30T22:58:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Krüger writes:\n\n> When using a custom format in line termination mode (as opposed to line\n> separation mode), the configured line terminator is not used, so things\n> like \"git log --pretty=tformat:%H -z\" do not work properly.\n>\n> Make it use the line terminator the user ordered.\n>\n> Signed-off-by: Jan Krüger \n> ---\n> Here are two simple tests, for both format: and tformat: with -z.\n\nThanks for being thorough. Very much appreciated.\n\nHaving said that, are we sure that\n\n\tprintf \"add bar\\0initial\"\n\nworks per specification, or merely works by accident in some\nimplementation?\n\nIn C, we have to write this as printf(\"add bar%cinitial\", 0), and the\nabove makes my stomach feel a bit queasy.\n\nAdmittedly we have \"printf \"\\0\\0\" in t6024 and we haven't seen anybody\ncomplain for the past 6 years, so perhaps I shouldn't be worried too much\nabout this.\n\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index 2ae9faa..03a73ba 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -71,4 +71,16 @@ test_expect_success 'alias loop' '\n> \ttest_must_fail git log --pretty=test-foo\n> '\n> \n> +printf \"add bar\\0initial\" > expected\n> +test_expect_success 'NUL separation' '\n> +\tgit log -z --pretty=\"format:%s\" >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +printf \"add bar\\0initial\\0\" > expected\n> +test_expect_success 'NUL termination' '\n> +\tgit log -z --pretty=\"tformat:%s\" >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_done\n"},{"id":"190372","messageId":"4F9F201C.40509@jk.gs","threadId":"30369","inReplyTo":"7vd36oesjr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2012-04-30T23:28:28Z","receivedAt":"2012-04-30T23:28:28Z","isPatch":true,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"On 05/01/2012 12:58 AM, Junio C Hamano wrote:\n> Having said that, are we sure that\n> \n> \tprintf \"add bar\\0initial\"\n> \n> works per specification, or merely works by accident in some\n> implementation?\n\nI don't know for certain. In these cases I go by what dash (Debian's sh\nimplementation) does. To quote its manpage:\n\n\"Only features designated by POSIX, plus a few Berkeley extensions, are\nbeing incorporated into this shell.\"\n\nIt does support this syntax for printf. That's enough for me personally.\nIs it enough for everyone? I don't know.\n\n-Jan\n\n"},{"id":"190375","messageId":"m2ehr4sq1q.fsf@igel.home","threadId":"30369","inReplyTo":"7vd36oesjr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-05-01T00:28:17Z","receivedAt":"2012-05-01T00:28:17Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano writes:\n\n> Having said that, are we sure that\n>\n> \tprintf \"add bar\\0initial\"\n>\n> works per specification, or merely works by accident in some\n> implementation?\n\nSince the backslash is not followed by $ ` \" \\ it is not\nspecial to the shell.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"190376","messageId":"7v62cgenn4.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"m2ehr4sq1q.fsf@igel.home","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T00:43:59Z","receivedAt":"2012-05-01T00:43:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> Junio C Hamano writes:\n>\n>> Having said that, are we sure that\n>>\n>> \tprintf \"add bar\\0initial\"\n>>\n>> works per specification, or merely works by accident in some\n>> implementation?\n>\n> Since the backslash is not followed by $ ` \" \\ it is not\n> special to the shell.\n\nYeah, but I wasn't worried about what shell does in the first place. I was\nworried about what printf(1) does.\n"},{"id":"190385","messageId":"86ehr4r4u0.fsf@red.stonehenge.com","threadId":"30369","inReplyTo":"7vd36oesjr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2012-05-01T02:51:51Z","receivedAt":"2012-05-01T02:51:51Z","isPatch":true,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Junio\" == Junio C Hamano writes:\n\nJunio> Having said that, are we sure that\n\nJunio> \tprintf \"add bar\\0initial\"\n\n\nJunio> works per specification, or merely works by accident in some\nJunio> implementation?\n\n>From the POSIX spec\n(http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html):\n\n In addition to the escape sequences shown in XBD File Format Notation (\n '\\\\' , '\\a' , '\\b' , '\\f' , '\\n' , '\\r' , '\\t' , '\\v' ), \"\\ddd\" , where\n ddd is a one, two, or three-digit octal number, shall be written as a\n byte with the numeric value specified by the octal number.\n\nLooks pretty intentional to me. \\0 is a nul.\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nSmalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.\nSee http://methodsandmessages.posterous.com/ for Smalltalk discussion\n"},{"id":"190391","messageId":"7vhaw0cyql.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"86ehr4r4u0.fsf@red.stonehenge.com","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T04:27:14Z","receivedAt":"2012-05-01T04:27:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"merlyn@stonehenge.com (Randal L. Schwartz) writes:\n\n>>>>>> \"Junio\" == Junio C Hamano writes:\n>\n> Junio> Having said that, are we sure that\n>\n> Junio> \tprintf \"add bar\\0initial\"\n>\n>\n> Junio> works per specification, or merely works by accident in some\n> Junio> implementation?\n>\n> From the POSIX spec\n> (http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html):\n>\n> In addition to the escape sequences shown in XBD File Format Notation (\n> '\\\\' , '\\a' , '\\b' , '\\f' , '\\n' , '\\r' , '\\t' , '\\v' ), \"\\ddd\" , where\n> ddd is a one, two, or three-digit octal number, shall be written as a\n> byte with the numeric value specified by the octal number.\n>\n> Looks pretty intentional to me. \\0 is a nul.\n\nI was staring at the same passage, but somehow it didn't \"click\" for me\nthat the above is _not_ saying that the NUL would terminate the string.\n\nSo I was worried too much and needlessly. Thanks.\n"},{"id":"190402","messageId":"m2mx5snytp.fsf@linux-m68k.org","threadId":"30369","inReplyTo":"7v62cgenn4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] log-tree: use custom line terminator in line termination mode","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-05-01T07:29:54Z","receivedAt":"2012-05-01T07:29:54Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano writes:\n\n> Andreas Schwab writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Having said that, are we sure that\n>>>\n>>> \tprintf \"add bar\\0initial\"\n>>>\n>>> works per specification, or merely works by accident in some\n>>> implementation?\n>>\n>> Since the backslash is not followed by $ ` \" \\ it is not\n>> special to the shell.\n>\n> Yeah, but I wasn't worried about what shell does in the first place. I was\n> worried about what printf(1) does.\n\nOh, I was confused becaused you mentioned the C case where the \\NNN\ninterpretation is done by the compiler, not printf(3).\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"190410","messageId":"20120501085649.GD4998@sigill.intra.peff.net","threadId":"30369","inReplyTo":"7vaa1tf1w8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-01T08:56:49Z","receivedAt":"2012-05-01T08:56:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 30, 2012 at 12:36:07PM -0700, Junio C Hamano wrote:\n\n> Jan Krüger writes:\n> \n> > When using a custom format in line termination mode (as opposed to line\n> > separation mode), the configured line terminator is not used, so things\n> > like \"git log --pretty=tformat:%H -z\" do not work properly.\n> >\n> > Make it use the line terminator the user ordered.\n> >\n> > Signed-off-by: Jan Krüger \n> > ---\n> > log-tree.c | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/log-tree.c b/log-tree.c\n> > index 34c49e7..44f0268 100644\n> > --- a/log-tree.c\n> > +++ b/log-tree.c\n> > @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n> > \tif (opt->use_terminator) {\n> > \t\tif (!opt->missing_newline)\n> > \t\t\tgraph_show_padding(opt->graph);\n> > -\t\tputchar('\\n');\n> > +\t\tputchar(opt->diffopt.line_termination);\n> > \t}\n> > \n> > \tstrbuf_release(&msgbuf);\n> \n> Looks sensible. Perhaps we would want to add a test?\n\nHmm. This came up before, and the issue is (or can be) slightly more\ncomplex:\n\n http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568\n\n-Peff\n"},{"id":"190424","messageId":"7v1un3c23p.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"20120501085649.GD4998@sigill.intra.peff.net","subject":"Re: [PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T16:12:10Z","receivedAt":"2012-05-01T16:12:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Apr 30, 2012 at 12:36:07PM -0700, Junio C Hamano wrote:\n>\n>> Jan Krüger writes:\n>> \n>> > When using a custom format in line termination mode (as opposed to line\n>> > separation mode), the configured line terminator is not used, so things\n>> > like \"git log --pretty=tformat:%H -z\" do not work properly.\n>> >\n>> > Make it use the line terminator the user ordered.\n>> >\n>> > Signed-off-by: Jan Krüger \n>> > ---\n>> > log-tree.c | 2 +-\n>> > 1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/log-tree.c b/log-tree.c\n>> > index 34c49e7..44f0268 100644\n>> > --- a/log-tree.c\n>> > +++ b/log-tree.c\n>> > @@ -682,7 +682,7 @@ void show_log(struct rev_info *opt)\n>> > \tif (opt->use_terminator) {\n>> > \t\tif (!opt->missing_newline)\n>> > \t\t\tgraph_show_padding(opt->graph);\n>> > -\t\tputchar('\\n');\n>> > +\t\tputchar(opt->diffopt.line_termination);\n>> > \t}\n>> > \n>> > \tstrbuf_release(&msgbuf);\n>> \n>> Looks sensible. Perhaps we would want to add a test?\n>\n> Hmm. This came up before, and the issue is (or can be) slightly more\n> complex:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568\n\nYeah, the test given completely forgets about \"log -p\" case, as you said\nin the above:\n\n But we can't just modify that to use the specified line terminator,\n because sometimes it is acting as a separator between commit message and\n diff, and sometimes it is acting as the terminator of the whole record.\n\nSo the patch is not quite right for the \"log -p -z\" (or \"log --stat -z\")\ncase.\n\nThe correct output would have NUL after each commit, so \"-z --format=%s\"\nwould have a single-liner subject with the line-terminating LF replaced\nwith NUL, and \"-p/--stat -z --format=%s\" would have a single-liner subject\nwith its line-terminating LF, followed by the diff/diffstat in which the\nterminating LF of the last line is replaced with NUL, but to be consistent\nwith what \"-p/--stat -z --pretty=format:%s\" does, I think it is OK to\nappend NUL to the diff/diffstat part instead of replacing its last LF with\nNUL.\n"},{"id":"190432","messageId":"7vobq7al12.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"7v1un3c23p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T17:06:17Z","receivedAt":"2012-05-01T17:06:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> Hmm. This came up before, and the issue is (or can be) slightly more\n>> complex:\n>>\n>> http://thread.gmane.org/gmane.comp.version-control.git/122478/focus=122568\n>\n> Yeah, the test given completely forgets about \"log -p\" case, as you said\n> in the above:\n>\n> But we can't just modify that to use the specified line terminator,\n> because sometimes it is acting as a separator between commit message and\n> diff, and sometimes it is acting as the terminator of the whole record.\n>\n> So the patch is not quite right for the \"log -p -z\" (or \"log --stat -z\")\n> case.\n>\n> The correct output would have NUL after each commit, so \"-z --format=%s\"\n> would have a single-liner subject with the line-terminating LF replaced\n> with NUL, and \"-p/--stat -z --format=%s\" would have a single-liner subject\n> with its line-terminating LF, followed by the diff/diffstat in which the\n> terminating LF of the last line is replaced with NUL, but to be consistent\n> with what \"-p/--stat -z --pretty=format:%s\" does, I think it is OK to\n> append NUL to the diff/diffstat part instead of replacing its last LF with\n> NUL.\n\nIn other words, this test on top (the last one only demonstrates the\nbreakage).\n\n t/t4205-log-pretty-formats.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex f00e446..4afd778 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -83,4 +83,20 @@ test_expect_success 'NUL termination' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'NUL separation with --stat' '\n+\tstat0_part=$(git diff --stat HEAD^ HEAD) &&\n+\tstat1_part=$(git diff --stat --root HEAD^) &&\n+\tprintf \"add bar\\n$stat0_part\\n\\0initial\\n$stat1_part\\n\" >expected &&\n+\tgit log -z --stat --pretty=\"format:%s\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_failure 'NUL termination with --stat' '\n+\tstat0_part=$(git diff --stat HEAD^ HEAD) &&\n+\tstat1_part=$(git diff --stat --root HEAD^) &&\n+\tprintf \"add bar\\n$stat0_part\\n\\0initial\\n$stat1_part\\n\\0\" >expected &&\n+\tgit log -z --stat --pretty=\"tformat:%s\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n"},{"id":"190474","messageId":"7vmx5r8y9i.fsf@alter.siamese.dyndns.org","threadId":"30369","inReplyTo":"7vobq7al12.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log-tree: use custom line terminator in line termination mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-01T20:03:21Z","receivedAt":"2012-05-01T20:03:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> The correct output would have NUL after each commit, so \"-z --format=%s\"\n>> would have a single-liner subject with the line-terminating LF replaced\n>> with NUL, and \"-p/--stat -z --format=%s\" would have a single-liner subject\n>> with its line-terminating LF, followed by the diff/diffstat in which the\n>> terminating LF of the last line is replaced with NUL, but to be consistent\n>> with what \"-p/--stat -z --pretty=format:%s\" does, I think it is OK to\n>> append NUL to the diff/diffstat part instead of replacing its last LF with\n>> NUL.\n>\n> In other words, this test on top (the last one only demonstrates the\n> breakage).\n\nJust a short hint if anybody wants to take a stab at it while I am deeply\nin today's integration cycle.\n\nThe code for \"separator\" semantics was a well tested code when\n\"terminator\" semantics was bolted on, and operated like this:\n\n for each record\n do\n if we have shown a record already\n then\n show the termination character\n fi\n show the record\n done\n\nThe only difference between the two semantics is if we append the\ntermination character after all the above is done, so the code should be\nstructured that way, but that may not be how we currently do it. So the\nproper way to add the \"terminator\" semantics ought to be:\n\n if we have shown any record in the loop && opt->use_terminator\n then\n show the termination character\n fi\n\nat the end of the above loop. If we see opt->use_terminator anywhere else\nin the existing code, it is an indication of a bug.\n\nThere is one small glitch. If a record is _not_ terminated with a\nnewline, e.g. \"log --pretty=format:%s\" without --stat/-p, \"separator\"\nsemantics will end up giving an incomplete line at the end. Most of the\ntime we will be piping out output to the pager so it may not be a problem\nin the real life, but it would be nicer to also terminate such an output.\n\nSo the resulting logic should look something like:\n\n for each record\n do\n if we have shown a record already\n then\n show the termination character\n fi\n show the record\n remember if the record ended with a LF\n done\n if we have shown any record in the loop &&\n (opt->use_terminator ||\n (opt->diffopt.line_termination == '\\n' &&\n the last record did not end with a LF))\n then\n show the termination character\n fi\n \n"}]}