Re: [PATCH] Add an option to git-ls-tree to display also the size of object

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > Add -l/--long/--size option to git-ls-tree command, which displays
> > object size of an entry after object id (left-justified with minimum
> > width of 7 characters).
> 
> Not a NAK at all (but not an ACK either yet), but just asking
> questions on some design considerations.

I guess I should use [PATCH/RFC] for this patch...

> * Do these options do different things?  If not, why have more
>   than one (or two, --long and its shorthand -l)?

The idea was to have output similar (if possible by git-ls-tree
machinery) to 'ls -l' output, hence -l/--long, but actually it is
about --size.

> * Why pad to 7 places?  Do we have a similar padding elsewhere?
>   Will this ever used by non-scripts?  How does this padding
>   affect parsers other than Perl that read this information?

Padding is added here to make output more human-readable. And I guess
padding of 7 places is default for 'ls -l'.

But certainly padding is not needed.

> * Does it make sense to show size information when giving a tree
>   entry?  I realize not having it in the output would make the
>   job of the script reading the output a bit harder, but if this
>   output is meant also for human consumption I think it would
>   not be so interesting and raise a confusion factor.

Giving tree size information is similar to 'ls -l giving size of
directory, and not total size taken by its contents. That would be
better left for git-ls-tree `--du' option :-)

>   Also I suspect that having to show the size of a tree object,
>   expressed in terms of the canonical representation, might
>   force packv4 aware ls-tree to convert its traversal efficient
>   representation to the canonical one only to get its size.

It still will be accessible, but perhaps it would be less efficient
with v4 pack. It is I think acceptable that -l needs more CPU (and I/O) 
time...

-- 
Jakub Narebski
Poland