Re: [PATCH 3/4] cat-file: add --batch-disk-sizes option

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 8, 2013, 13:13 UTC
Message-ID: <CACsJy8Dffc2WgtDyUS2g2gmDWG_rTxs389fHcj0ztm6pdJddjQ@mail.gmail.com>
In-Reply-To: <CALkWK0ktNK49zBM4tD8fpNN3VMan7DegfWRtDcOEgTyEbSK9Uw@mail.gmail.com>

On Mon, Jul 8, 2013 at 7:00 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 18 quoted lines

>> This is what I wanted to do with the in for-each-ref's pretty
>> formatting [1]. I used to hack cat-file --batch to extract info I
>> needed for experimenting with various pack index extensions. If you
>> are not in hurry, maybe we can introduce something similar to your
>> syntax, but applicable for all for-each-ref, branch and log family.
>
> I'm still quite confused about this "grand plan".  We have short
> commit-specific format specifiers that don't work with refs, among
> several other quirks in [1].  I personally think we should absolutely
> stay away from short format-specifiers (like %H, %f, %e; we'll soon
> run out of letters, and nobody can tell what they are without the
> documentation anyway) for the new options, and just start adding new
> long-form ones as and when they are necessary.  I think refname:short,
> upstream:track, upstream:trackshort are very sensible choices, and
> that we should continue along that line.  I'm fine with
> format-specifiers having meanings only in certain contexts as long as
> we document it properly (how can we possibly get %(refname) to mean
> something sensible in cat-file?).

The short/long naming is the least I worry about. We could add long names to pretty specifiers. The thing about the last attempt is, you add some extra things on top elsewhere, but format_commit_item code may need to be aware of those changes, which are not obvious when sombody just focuses on format_commit_item. Having all specifiers in one place would be better (hence no hooks, no callbacks) because we get a full picture. And yes we need to deal with specifers that make no sense in certain context.

All that makes changes bigger, but when format_commit_item (now just "format_item") knows how to deal with all kinds of objects and refs, it becomes a small declaration language to extract things out of git. You can use it for pretty printing or mass extraction in the case of "cat-file --batch". "cat-file --batch" is just some bonus on top, mostly to exercise that we do it right.

> As far as this series is concerned, I think Peff can implement %H and
> %(object:[disk-]size) locally without worrying about code-sharing or
> waiting for us.  Then, after the for-each-ref-pretty thing matures, we
> can just replace the code underneath.

There's also syntax sharing. I don't think each command should have its own syntax. f-e-r already has %(objectsize). If we plan to have a common syntax, perhaps %(disk-size) should be %(objectsize:disk) or something. Adding formatting to cat-file --batch from scratch could be another big chunk of code (that also comes with bugs, usually) and may or may not be compatible with the common syntax because of some oversight. --batch-cols=... or --batch-disk-size would be simpler, but we might never be able to remove that code. -- Duy