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

On Mon, Jul 08, 2013 at 07:07:01PM +0530, Ramkumar Ramachandra wrote:

quoted

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.

quoted

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.

I went with %(objectname), %(objectsize), and %(objectsize:disk). The
former match for-each-ref, and the latter extends it in a natural-ish
way (though it is arguable whether "foo:modifier" should mean "do
something with the foo value" or "this is like foo, but not quite"). In
the long run, I would like to see long names for each atom, with short
aliases (so "%H" and "%h" for "%(objectname) and "%(objectname:short)",
available everywhere).

But I think it is OK to start without the aliases, and then pick them up
as the implementations and interfaces are unified. IOW, it is OK to say
"cat-file has not learned about %H yet", and later add it; we cannot
teach it "%H" now and then change our minds later.

quoted

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

I would say it is worth one of the other routes if it turned out to be
dramatically simpler. But having just done the work to add formats for
cat-file, it is really not too bad. It lacks some of the niceties of the
other formatters (e.g., colors), but again, we can always add them in
later as the implementations unify.

-Peff