# [PATCH] pretty.c: add %z specifier. 8 messages from 2008-03-21 to 2008-03-21. Participants: Govind Salinas, Jeff King, Junio C Hamano, David Symonds. Thread: https://gitlist.dev/t/12784 ## Govind Salinas, 2008-03-21 00:45 Subject: [PATCH] pretty.c: add %z specifier. Message-ID: <5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com> URL: https://gitlist.dev/e/5d46db230803201745mb736e98w4925e14b5d92d71d%40mail.gmail.com ``` This adds a %z format which prints out a null character. This allows for easier machine parsing of multiline data. It is also necessary to use write to print out the data since printf will terminate at a null. That in turn requires that an fflush be executed before the write to preserve the order the data is printed. Signed-off-by: Govind Salinas --- log-tree.c | 7 +++++-- pretty.c | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/log-tree.c b/log-tree.c index 608f697..e116a1f 100644 --- a/log-tree.c +++ b/log-tree.c @@ -308,8 +308,11 @@ void show_log(struct rev_info *opt, const char *sep) if (opt->show_log_size) printf("log size %i\n", (int)msgbuf.len); - if (msgbuf.len) - printf("%s%s%s", msgbuf.buf, extra, sep); + if (msgbuf.len) { + fflush(stdout); + write(STDOUT_FILENO, msgbuf.buf, msgbuf.len); + printf("%s%s", extra, sep); + } strbuf_release(&msgbuf); } diff --git a/pretty.c b/pretty.c index 703f521..fd155ec 100644 --- a/pretty.c +++ b/pretty.c @@ -478,6 +478,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, case 'n': /* newline */ strbuf_addch(sb, '\n'); return 1; + case 'z': /* null */ + strbuf_addch(sb, '\0'); + return 1; } /* these depend on the commit */ -- 1.5.4.4.552.g9987b ``` ## Jeff King, 2008-03-21 02:13 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <20080321021337.GD1613@coredump.intra.peff.net> URL: https://gitlist.dev/e/20080321021337.GD1613%40coredump.intra.peff.net In-Reply-To: <5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com> ``` On Thu, Mar 20, 2008 at 07:45:26PM -0500, Govind Salinas wrote: > This adds a %z format which prints out a null character. This allows for > easier machine parsing of multiline data. It is also necessary to use write > to print out the data since printf will terminate at a null. That in turn > requires that an fflush be executed before the write to preserve the order > the data is printed. How about using fwrite instead of write? -Peff ``` ## Junio C Hamano, 2008-03-21 04:48 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <7veja4u1gv.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7veja4u1gv.fsf%40gitster.siamese.dyndns.org In-Reply-To: <5d46db230803201745mb736e98w4925e14b5d92d71d@mail.gmail.com> ``` "Govind Salinas" writes: > diff --git a/pretty.c b/pretty.c > index 703f521..fd155ec 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -478,6 +478,9 @@ static size_t format_commit_item(struct strbuf > *sb, const char *placeholder, > case 'n': /* newline */ > strbuf_addch(sb, '\n'); > return 1; > + case 'z': /* null */ > + strbuf_addch(sb, '\0'); > + return 1; > } > > /* these depend on the commit */ I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits for an octet)? ``` ## Jeff King, 2008-03-21 04:51 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <20080321045137.GA5563@coredump.intra.peff.net> URL: https://gitlist.dev/e/20080321045137.GA5563%40coredump.intra.peff.net In-Reply-To: <7veja4u1gv.fsf@gitster.siamese.dyndns.org> ``` On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote: > > + case 'z': /* null */ > > + strbuf_addch(sb, '\0'); > > + return 1; > > } > > > > /* these depend on the commit */ > > I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits > for an octet)? Because %ad is already taken? :) %x* is still available, though, so maybe %x00? -Peff ``` ## Junio C Hamano, 2008-03-21 05:09 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <7vtzj0slx4.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtzj0slx4.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20080321045137.GA5563@coredump.intra.peff.net> ``` Jeff King writes: > On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote: > >> > + case 'z': /* null */ >> > + strbuf_addch(sb, '\0'); >> > + return 1; >> > } >> > >> > /* these depend on the commit */ >> >> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits >> for an octet)? > > Because %ad is already taken? :) > > %x* is still available, though, so maybe %x00? Perhaps, but before I forget. My much bigger niggle about the "--pretty=format:<>" code I have is that the "log" machinery does not change the usual record "delimiter" to record "terminator" when --pretty=format:<> is in effect. The "log" family generally treats LF/NUL as record delimiter, not terminator, and it is by a very good conscious design. When you are looking at the output from "git log -2", you would want to have a delimiting LF between the first commit and the second commit, but you do not want an extra LF after the second commit. However, when "--pretty=format:<>" is in effect, it is inconvenient that the machinery inserts a LF between each record but not at the end. $ git log -2 --pretty=format:%s may look sane when the pager immediately returns the control to you, but it is not really. To view it: $ git log -2 --pretty=format:%s | cat This would show that there is no LF after the final output, which is quite bad. ``` ## Govind Salinas, 2008-03-21 05:42 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com> URL: https://gitlist.dev/e/5d46db230803202242j60b0e9f6q798afd6c5f468207%40mail.gmail.com In-Reply-To: <7vtzj0slx4.fsf@gitster.siamese.dyndns.org> ``` On Fri, Mar 21, 2008 at 12:09 AM, Junio C Hamano wrote: > > Jeff King writes: > > > On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote: > > > >> > + case 'z': /* null */ > >> > + strbuf_addch(sb, '\0'); > >> > + return 1; > >> > } > >> > > >> > /* these depend on the commit */ > >> > >> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits > >> for an octet)? > > > > Because %ad is already taken? :) > > > > %x* is still available, though, so maybe %x00? > > Perhaps, but before I forget. > > My much bigger niggle about the "--pretty=format:<>" code I have is that > the "log" machinery does not change the usual record "delimiter" to record > "terminator" when --pretty=format:<> is in effect. > > The "log" family generally treats LF/NUL as record delimiter, not > terminator, and it is by a very good conscious design. When you are > looking at the output from "git log -2", you would want to have a > delimiting LF between the first commit and the second commit, but you do > not want an extra LF after the second commit. > > However, when "--pretty=format:<>" is in effect, it is inconvenient that > the machinery inserts a LF between each record but not at the end. > > $ git log -2 --pretty=format:%s > > may look sane when the pager immediately returns the control to you, but > it is not really. To view it: > > $ git log -2 --pretty=format:%s | cat > > This would show that there is no LF after the final output, which is quite > bad. > Sorry, I'm a bit confused. Should I alter the patch to use a different code for null, that would be fine by me? The above seems to be an unrelated issue. Thanks, Govind. ``` ## David Symonds, 2008-03-21 05:50 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: URL: https://gitlist.dev/e/ee77f5c20803202250w1b1f4228y3613109762c93454%40mail.gmail.com In-Reply-To: <5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com> ``` On Fri, Mar 21, 2008 at 4:42 PM, Govind Salinas wrote: > > Sorry, I'm a bit confused. Should I alter the patch to use a different code > for null, that would be fine by me? The above seems to be an unrelated issue. I'm pretty sure the suggestion is that you should change the patch to allow for *any* specific byte value, where the null byte is just a special case. %x00 would be used instead of %z, in other words, and %x20 would be a space character, etc. Dave. ``` ## Junio C Hamano, 2008-03-21 06:19 Subject: Re: [PATCH] pretty.c: add %z specifier. Message-ID: <7v8x0csios.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8x0csios.fsf%40gitster.siamese.dyndns.org In-Reply-To: <5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com> ``` "Govind Salinas" writes: > On Fri, Mar 21, 2008 at 12:09 AM, Junio C Hamano wrote: >> >> Jeff King writes: >> >> > On Thu, Mar 20, 2008 at 09:48:16PM -0700, Junio C Hamano wrote: >> > >> >> > + case 'z': /* null */ >> >> > + strbuf_addch(sb, '\0'); >> >> > + return 1; >> >> > } >> >> > >> >> > /* these depend on the commit */ >> >> >> >> I do not like this at all. Why aren't we doing %XX (2 hexadecimal digits >> >> for an octet)? >> > >> > Because %ad is already taken? :) >> > >> > %x* is still available, though, so maybe %x00? >> >> Perhaps, but before I forget. >> ... > > Sorry, I'm a bit confused. Should I alter the patch to use a different code > for null, that would be fine by me? The above seems to be an unrelated issue. Sorry for confusing you. The above is an unrelated issue. But at least to me it is much more important one. I would not be unhappy at all if we did not have either %z nor %x00, but the above bugs me moderately. Also I suspect the proper fix for that issue would involve the part in log-tree you touched. By the way, I think Jeff's suggestion of %x00 makes more sense than %z. pretty.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/pretty.c b/pretty.c index 16bfb86..308bfad 100644 --- a/pretty.c +++ b/pretty.c @@ -457,6 +457,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, const struct commit *commit = c->commit; const char *msg = commit->buffer; struct commit_list *p; + int h1, h2; /* these are independent of the commit */ switch (placeholder[0]) { @@ -478,6 +479,18 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, case 'n': /* newline */ strbuf_addch(sb, '\n'); return 1; + case 'x': + /* %x00 == NUL, %x0a == LF, etc. */ + if (0 <= (h1 = hexval_table[0xff & placeholder[1]]) && + h1 <= 16 && + 0 <= (h2 = hexval_table[0xff & placeholder[2]]) && + h2 <= 16) { + strbuf_addch(sb, (h1<<4)|h2); + return 2; + } else { + return 0; + } + } /* these depend on the commit */ ```