git.vger.kernel.org archive mirror
 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 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).