git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Govind Salinas" <govind@sophiasuchtig.com>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>, "Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] pretty.c: add %z specifier.
Date: Fri, 21 Mar 2008 00:42:55 -0500	[thread overview]
Message-ID: <5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com> (raw)
In-Reply-To: <7vtzj0slx4.fsf@gitster.siamese.dyndns.org>

On Fri, Mar 21, 2008 at 12:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> 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.

  reply	other threads:[~2008-03-21  5:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-21  0:45 [PATCH] pretty.c: add %z specifier Govind Salinas
2008-03-21  2:13 ` Jeff King
2008-03-21  4:48 ` Junio C Hamano
2008-03-21  4:51   ` Jeff King
2008-03-21  5:09     ` Junio C Hamano
2008-03-21  5:42       ` Govind Salinas [this message]
2008-03-21  5:50         ` David Symonds
2008-03-21  6:19         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d46db230803202242j60b0e9f6q798afd6c5f468207@mail.gmail.com \
    --to=govind@sophiasuchtig.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).