git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Git List <git@vger.kernel.org>
Subject: Re: [PATCH 08/15] for-each-ref: get --pretty using format_commit_message
Date: Thu, 6 Jun 2013 07:07:44 +0700	[thread overview]
Message-ID: <CACsJy8D9bLijPX7JnUEkJ4e9OU9tDN_vGa=gkkHSm3Sr34fWQQ@mail.gmail.com> (raw)
In-Reply-To: <7v61xs32n0.fsf@alter.siamese.dyndns.org>

On Thu, Jun 6, 2013 at 12:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Jun 5, 2013 at 4:12 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> +Caveats:
>>>> +
>>>> +1. Many of the placeholders in "PRETTY FORMATS" are designed to work
>>>> +   specifically on commit objects: when non-commit objects are
>>>> +   supplied, those placeholders won't work.
>>>
>>> Should "won't work" be expanded upon? It's not clear if this means
>>> that git will outright crash, or if it will abort with an appropriate
>>> error message, or if the directive will be displayed as-is or removed
>>> from the output.
>>
>> It will be displayed as-is but that's a bit inconsistent: %(unknown)
>> prints error and aborts while %unknown simply produces %unknown. The
>> latter is how "git log --format" does it. But I think we could make
>> for-each-ref --pretty to do the former for %unknown. It'll be
>> consistent with %(unknown) and we do not need to elaborate much (it's
>> pretty obvious when it does not work).
>
> The Caveat Eric is asking about talks about "what happens to a
> %(field) that only makes sense for a commit when showing a ref
> pointing at a non-commit?", but you are answering "what happend to a
> %(invalidfield) that is not defined", aren't you?

Because %(field) is treated like %(invalidfield) in this case. I'm not
saying this is the best thing to do though.

> IIRC, the reason we show literal from "log --format" is to make it
> easier for the person who misspelt %placeholder to spot it in the
> output, and also make it easier for the person who use %placeholder
> meant for newer versions of Git with an older one.  It would be a
> bit unnice to die for the latter, especially if the format string is
> in a script or something.

That makes more sense for for-each-ref than log because for-each-ref
is a plumbing and should support scripting. But old for-each-ref will
happily reject %(newplacholder) right away. Should we take this
opportunity to change this behavior in for-each-ref too?

> To "log --format", all input objects are expected to be commits, so
> it does not have the "what does %(authordate) give when given a blob"
> issue.
>
> But for "for-each-ref --format", it is perfectly normal that you may
> feed a non-commit; it makes the mechanism unusable if you errored
> out %(authordate) when showing a ref that points at a tag, doesn't
> it?  Substituting an inapplicable placeholder with an empty string
> would be an easies way out, unless it learns a flexible/elaborate
> conditional formatting mechanism, I would think.

There is a blurred line here. %H (or %h) should produce an object name
even for tags or blobs, but right now it produces "%H" instead. The
same applies for %ad and friends, but these are clearly for commits
and should probably behave like %(authordate), i.e. producing empty
string.
--
Duy

  reply	other threads:[~2013-06-06  0:08 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-04 12:35 [PATCH 00/15] Towards a more awesome git-branch Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 01/15] for-each-ref, quote: convert *_quote_print -> *_quote_buf Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 02/15] for-each-ref: don't print out elements directly Ramkumar Ramachandra
2013-06-04 23:13   ` Junio C Hamano
2013-06-04 23:44     ` Duy Nguyen
2013-06-04 12:35 ` [PATCH 03/15] tar-tree: remove dependency on sq_quote_print() Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 04/15] quote: remove sq_quote_print() Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 05/15] pretty: extend pretty_print_context with callback Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 06/15] pretty: limit recursion in format_commit_one() Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 07/15] pretty: allow passing NULL commit to format_commit_message() Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 08/15] for-each-ref: get --pretty using format_commit_message Ramkumar Ramachandra
2013-06-04 21:12   ` Eric Sunshine
2013-06-05 13:21     ` Duy Nguyen
2013-06-05 17:09       ` Junio C Hamano
2013-06-06  0:07         ` Duy Nguyen [this message]
2013-06-04 12:35 ` [PATCH 09/15] for-each-ref: teach verify_format() about pretty's syntax Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 10/15] for-each-ref: introduce format specifier %>(*) and %<(*) Ramkumar Ramachandra
2013-06-04 12:54   ` Duy Nguyen
2013-06-05  8:14     ` Ramkumar Ramachandra
2013-06-05 10:15       ` Duy Nguyen
2013-06-04 12:35 ` [PATCH 11/15] for-each-ref: introduce %(HEAD) marker Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 12/15] for-each-ref: introduce %(upstream:track[short]) Ramkumar Ramachandra
2013-06-04 21:14   ` Eric Sunshine
2013-06-04 12:35 ` [PATCH 13/15] for-each-ref: improve responsiveness of %(upstream:track) Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 14/15] pretty: introduce get_pretty_userformat Ramkumar Ramachandra
2013-06-04 12:35 ` [PATCH 15/15] for-each-ref: use get_pretty_userformat in --pretty Ramkumar Ramachandra
2013-06-04 12:57   ` Duy Nguyen
2013-06-04 21:15   ` Eric Sunshine
2013-06-04 12:49 ` [PATCH 00/15] Towards a more awesome git-branch Duy Nguyen
2013-06-05  8:11   ` Ramkumar Ramachandra

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CACsJy8D9bLijPX7JnUEkJ4e9OU9tDN_vGa=gkkHSm3Sr34fWQQ@mail.gmail.com' \
    --to=pclouds@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).