Re: [PATCH/RFC 0/9] add long forms for format placeholders
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Mar 29, 2011, 06:46 UTC
- Message-ID
- <4D918032.3010608@drmicha.warpmail.net>
- In-Reply-To
- <7vei5qvkgw.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 29.03.2011 02:28:
Show 24 quoted lines
> 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