All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Will Palmer <wmpalmer@gmail.com>,
	git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH/RFC 0/9] add long forms for format placeholders
Date: Tue, 29 Mar 2011 08:46:10 +0200	[thread overview]
Message-ID: <4D918032.3010608@drmicha.warpmail.net> (raw)
In-Reply-To: <7vei5qvkgw.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 29.03.2011 02:28:
> Will Palmer <wmpalmer@gmail.com> writes:
> 
>> I've been kicking around this series for a while now as part of a larger
>> effort of refactoring the pretty formats. A recent discussion on the
>> list has lead me to believe that this smaller subset may be of use
>> sooner, rather than later.
>>
>> This series attempts to add "long forms" for common format placeholders
>> in the "git log" family of commands, making the way for yet more
>> placeholders to be added without needing to worry too much about the
>> increasingly limited set of available one-letter mnemonics. It also
>> moves towards the possibility of eventual unification with the format
>> options in for-each-ref.
> 
> I don't claim that I read 1300+ long [PATCH 5/9] carefully, but I like the
> direction in which this topic is going very much.
> 
> Except that [PATCH 2/9] looked quite out of place---more like "I wanted to
> sneak this feature in" than "this was needed to keep the resulting code
> backward compatible" or anything like that.
> 
> Off the top of my head, I don't think of a reason to say that [PATCH 3/9]
> is going in a wrong direction---is there a reason to make you worried in
> the particular change?

I'm wondering how much of this could and should be shared with
for-each-ref. Notable differences that I'm aware of:

- for-each-ref is about (named) refs which can point to any type of
object; rev-list/log is about commit objects

- for-each-ref deals with "few" objects typically, rev-list/log with many

So, other than %(refname), %(upstream) and %(tagger...), all
for-each-ref placeholders make sense for rev-list/log.

Sharing the parser would serve several purposes:

- reduced code
- increased test coverage (for-each-ref tests would test the parser)
- speed up for for-each-ref (due to your nice separation)
- short placeholders for for-each-ref
- automatic consistency between the two

Michael

  parent reply	other threads:[~2011-03-29  6:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-28 23:17 [PATCH/RFC 0/9] add long forms for format placeholders Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 1/9] mention --date=raw in rev-list and blame help Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 2/9] add support for --date=unix to complement %at Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 3/9] interpret %C(invalid) as we would %%C(invalid) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 4/9] add sanity length check to format_person_part Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 5/9] refactor pretty.c into "parse" and "format" steps Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 6/9] add long-form %(wrap:...) for %w(...) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 7/9] add long form %(color:...) for %C(...) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 8/9] add long forms %(authordate) and %(committerdate) Will Palmer
2011-03-28 23:17 ` [PATCH/RFC 9/9] add long forms for author and committer identity Will Palmer
2011-03-29  0:28 ` [PATCH/RFC 0/9] add long forms for format placeholders Junio C Hamano
2011-03-29  6:44   ` Will Palmer
2011-03-29  6:46   ` Michael J Gruber [this message]
2011-03-29  7:27     ` Will Palmer

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=4D918032.3010608@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=wmpalmer@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.