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

[Ramkumar Ramachandra <artagnon@gmail.com>]

Duy Nguyen wrote:
> 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.

Yeah, it would certainly be nice to have all the format-specifiers
that one unified parser acts on, but isn't this just a matter of
refactoring?  Shouldn't we be starting with cheap callbacks, get
things working, and guard against regressions in the refactoring phase
first?  How else do you propose to start out?

> 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.

Ofcourse.  I didn't notice %(objectsize); %(objectsize[:disk]) is a
fine suggestion.

> 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.

Oh, I'm proposing that Peff implements just %H and
%(objectsize[:disk]) for _now_, because that's what he wants.  It
should be a tiny 20-line parser that's easy to swap out.

> --batch-cols=... or --batch-disk-size would be simpler, but
> we might never be able to remove that code.

Agreed.  The approach paints us into a design-corner, and must
therefore be avoided.