{"thread":{"id":"12784","subject":"[PATCH] pretty.c: add %z specifier.","startedAt":"2008-03-21T00:45:26Z","lastAt":"2008-03-21T06:19:15Z","messageCount":8,"participants":["Govind Salinas","Jeff King","Junio C Hamano","David Symonds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"72545","messageId":"5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com","threadId":"12784","inReplyTo":null,"subject":"[PATCH] pretty.c: add %z specifier.","fromName":"Govind Salinas","fromEmail":"govind@sophiasuchtig.com","sentAt":"2008-03-21T00:45:26Z","receivedAt":"2008-03-21T00:45:26Z","isPatch":true,"sender":{"key":"govind@sophiasuchtig.com","avatar":null},"body":"This adds a %z format which prints out a null character. This allows for\neasier machine parsing of multiline data. It is also necessary to use write\nto print out the data since printf will terminate at a null. That in turn\nrequires that an fflush be executed before the write to preserve the order\nthe data is printed.\n\nSigned-off-by: Govind Salinas \n---\n log-tree.c | 7 +++++--\n pretty.c | 3 +++\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 608f697..e116a1f 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -308,8 +308,11 @@ void show_log(struct rev_info *opt, const char *sep)\n \tif (opt->show_log_size)\n \t\tprintf(\"log size %i\\n\", (int)msgbuf.len);\n\n-\tif (msgbuf.len)\n-\t\tprintf(\"%s%s%s\", msgbuf.buf, extra, sep);\n+\tif (msgbuf.len) {\n+\t\tfflush(stdout);\n+\t\twrite(STDOUT_FILENO, msgbuf.buf, msgbuf.len);\n+\t\tprintf(\"%s%s\", extra, sep);\n+\t}\n \tstrbuf_release(&msgbuf);\n }\n\ndiff --git a/pretty.c b/pretty.c\nindex 703f521..fd155ec 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -478,6 +478,9 @@ static size_t format_commit_item(struct strbuf\n*sb, const char *placeholder,\n \tcase 'n':\t\t/* newline */\n \t\tstrbuf_addch(sb, '\\n');\n \t\treturn 1;\n+\tcase 'z':\t\t/* null */\n+\t\tstrbuf_addch(sb, '\\0');\n+\t\treturn 1;\n \t}\n\n \t/* these depend on the commit */\n-- \n1.5.4.4.552.g9987b\n"},{"id":"72551","messageId":"20080321021337.GD1613@coredump.intra.peff.net","threadId":"12784","inReplyTo":"5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-03-21T02:13:38Z","receivedAt":"2008-03-21T02:13:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 20, 2008 at 07:45:26PM -0500, Govind Salinas wrote:\n\n> This adds a %z format which prints out a null character. This allows for\n> easier machine parsing of multiline data. It is also necessary to use write\n> to print out the data since printf will terminate at a null. That in turn\n> requires that an fflush be executed before the write to preserve the order\n> the data is printed.\n\nHow about using fwrite instead of write?\n\n-Peff\n"},{"id":"72564","messageId":"7veja4u1gv.fsf@gitster.siamese.dyndns.org","threadId":"12784","inReplyTo":"5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-21T04:48:16Z","receivedAt":"2008-03-21T04:48:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Govind Salinas\" writes:\n\n> diff --git a/pretty.c b/pretty.c\n> index 703f521..fd155ec 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -478,6 +478,9 @@ static size_t format_commit_item(struct strbuf\n> *sb, const char *placeholder,\n> \tcase 'n':\t\t/* newline */\n> \t\tstrbuf_addch(sb, '\\n');\n> \t\treturn 1;\n> +\tcase 'z':\t\t/* null */\n> +\t\tstrbuf_addch(sb, '\\0');\n> +\t\treturn 1;\n> \t}\n>\n> \t/* these depend on the commit */\n\nI do not like this at all. Why aren't we doing %XX (2 hexadecimal digits\nfor an octet)?\n"},{"id":"72566","messageId":"20080321045137.GA5563@coredump.intra.peff.net","threadId":"12784","inReplyTo":"7veja4u1gv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-03-21T04:51:37Z","receivedAt":"2008-03-21T04:51:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote:\n\n> > +\tcase 'z':\t\t/* null */\n> > +\t\tstrbuf_addch(sb, '\\0');\n> > +\t\treturn 1;\n> > \t}\n> >\n> > \t/* these depend on the commit */\n> \n> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits\n> for an octet)?\n\nBecause %ad is already taken? :)\n\n%x* is still available, though, so maybe %x00?\n\n-Peff\n"},{"id":"72569","messageId":"7vtzj0slx4.fsf@gitster.siamese.dyndns.org","threadId":"12784","inReplyTo":"20080321045137.GA5563@coredump.intra.peff.net","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-21T05:09:27Z","receivedAt":"2008-03-21T05:09:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote:\n>\n>> > +\tcase 'z':\t\t/* null */\n>> > +\t\tstrbuf_addch(sb, '\\0');\n>> > +\t\treturn 1;\n>> > \t}\n>> >\n>> > \t/* these depend on the commit */\n>> \n>> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits\n>> for an octet)?\n>\n> Because %ad is already taken? :)\n>\n> %x* is still available, though, so maybe %x00?\n\nPerhaps, but before I forget.\n\nMy much bigger niggle about the \"--pretty=format:<>\" code I have is that\nthe \"log\" machinery does not change the usual record \"delimiter\" to record\n\"terminator\" when --pretty=format:<> is in effect.\n\nThe \"log\" family generally treats LF/NUL as record delimiter, not\nterminator, and it is by a very good conscious design. When you are\nlooking at the output from \"git log -2\", you would want to have a\ndelimiting LF between the first commit and the second commit, but you do\nnot want an extra LF after the second commit.\n\nHowever, when \"--pretty=format:<>\" is in effect, it is inconvenient that\nthe machinery inserts a LF between each record but not at the end.\n\n $ git log -2 --pretty=format:%s\n\nmay look sane when the pager immediately returns the control to you, but\nit is not really. To view it:\n\n $ git log -2 --pretty=format:%s | cat\n\nThis would show that there is no LF after the final output, which is quite\nbad.\n"},{"id":"72571","messageId":"5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com","threadId":"12784","inReplyTo":"7vtzj0slx4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Govind Salinas","fromEmail":"govind@sophiasuchtig.com","sentAt":"2008-03-21T05:42:55Z","receivedAt":"2008-03-21T05:42:55Z","isPatch":true,"sender":{"key":"govind@sophiasuchtig.com","avatar":null},"body":"On Fri, Mar 21, 2008 at 12:09 AM, Junio C Hamano wrote:\n>\n> Jeff King writes:\n>\n> > On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote:\n> >\n> >> > + case 'z': /* null */\n> >> > + strbuf_addch(sb, '\\0');\n> >> > + return 1;\n> >> > }\n> >> >\n> >> > /* these depend on the commit */\n> >>\n> >> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits\n> >> for an octet)?\n> >\n> > Because %ad is already taken? :)\n> >\n> > %x* is still available, though, so maybe %x00?\n>\n> Perhaps, but before I forget.\n>\n> My much bigger niggle about the \"--pretty=format:<>\" code I have is that\n> the \"log\" machinery does not change the usual record \"delimiter\" to record\n> \"terminator\" when --pretty=format:<> is in effect.\n>\n> The \"log\" family generally treats LF/NUL as record delimiter, not\n> terminator, and it is by a very good conscious design. When you are\n> looking at the output from \"git log -2\", you would want to have a\n> delimiting LF between the first commit and the second commit, but you do\n> not want an extra LF after the second commit.\n>\n> However, when \"--pretty=format:<>\" is in effect, it is inconvenient that\n> the machinery inserts a LF between each record but not at the end.\n>\n> $ git log -2 --pretty=format:%s\n>\n> may look sane when the pager immediately returns the control to you, but\n> it is not really. To view it:\n>\n> $ git log -2 --pretty=format:%s | cat\n>\n> This would show that there is no LF after the final output, which is quite\n> bad.\n>\n\nSorry, I'm a bit confused. Should I alter the patch to use a different code\nfor null, that would be fine by me? The above seems to be an unrelated issue.\n\n\nThanks,\nGovind.\n"},{"id":"72572","messageId":"ee77f5c20803202250w1b1f4228y3613109762c93454@mail.gmail.com","threadId":"12784","inReplyTo":"5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2008-03-21T05:50:04Z","receivedAt":"2008-03-21T05:50:04Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On Fri, Mar 21, 2008 at 4:42 PM, Govind Salinas\n wrote:\n>\n> Sorry, I'm a bit confused. Should I alter the patch to use a different code\n> for null, that would be fine by me? The above seems to be an unrelated issue.\n\nI'm pretty sure the suggestion is that you should change the patch to\nallow for *any* specific byte value, where the null byte is just a\nspecial case. %x00 would be used instead of %z, in other words, and\n%x20 would be a space character, etc.\n\n\nDave.\n"},{"id":"72574","messageId":"7v8x0csios.fsf@gitster.siamese.dyndns.org","threadId":"12784","inReplyTo":"5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com","subject":"Re: [PATCH] pretty.c: add %z specifier.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-21T06:19:15Z","receivedAt":"2008-03-21T06:19:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Govind Salinas\" writes:\n\n> On Fri, Mar 21, 2008 at 12:09 AM, Junio C Hamano wrote:\n>>\n>> Jeff King writes:\n>>\n>> > On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote:\n>> >\n>> >> > + case 'z': /* null */\n>> >> > + strbuf_addch(sb, '\\0');\n>> >> > + return 1;\n>> >> > }\n>> >> >\n>> >> > /* these depend on the commit */\n>> >>\n>> >> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits\n>> >> for an octet)?\n>> >\n>> > Because %ad is already taken? :)\n>> >\n>> > %x* is still available, though, so maybe %x00?\n>>\n>> Perhaps, but before I forget.\n>> ...\n>\n> Sorry, I'm a bit confused. Should I alter the patch to use a different code\n> for null, that would be fine by me? The above seems to be an unrelated issue.\n\nSorry for confusing you. The above is an unrelated issue. But at least\nto me it is much more important one. I would not be unhappy at all if we\ndid not have either %z nor %x00, but the above bugs me moderately. Also I\nsuspect the proper fix for that issue would involve the part in log-tree\nyou touched.\n\nBy the way, I think Jeff's suggestion of %x00 makes more sense than %z.\n\n pretty.c | 13 +++++++++++++\n 1 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 16bfb86..308bfad 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -457,6 +457,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,\n \tconst struct commit *commit = c->commit;\n \tconst char *msg = commit->buffer;\n \tstruct commit_list *p;\n+\tint h1, h2;\n \n \t/* these are independent of the commit */\n \tswitch (placeholder[0]) {\n@@ -478,6 +479,18 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,\n \tcase 'n':\t\t/* newline */\n \t\tstrbuf_addch(sb, '\\n');\n \t\treturn 1;\n+\tcase 'x':\n+\t\t/* %x00 == NUL, %x0a == LF, etc. */\n+\t\tif (0 <= (h1 = hexval_table[0xff & placeholder[1]]) &&\n+\t\t h1 <= 16 &&\n+\t\t 0 <= (h2 = hexval_table[0xff & placeholder[2]]) &&\n+\t\t h2 <= 16) {\n+\t\t\tstrbuf_addch(sb, (h1<<4)|h2);\n+\t\t\treturn 2;\n+\t\t} else {\n+\t\t\treturn 0;\n+\t\t}\n+\t\t\n \t}\n \n \t/* these depend on the commit */\n"}]}