Jakub Narebski [off-list ref] 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.
* Do these options do different things? If not, why have more
than one (or two, --long and its shorthand -l)?
* 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?
* 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.
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.
Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
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
Junio C Hamano [off-list ref] wrote:
Jakub Narebski [off-list ref] writes:
quoted
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).
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.
Yes, you are right Junio. In pack v4 we don't know the size of
the canonical representation. We compute it on the fly when its
needed by summing up the lengths of the names of each element in
the tree, so it requires us to expand the delta chain and is thus
O(delta_depth * entry_count) or something like that.
I didn't see this as a huge problem, as the only in-tree caller at
the time that needed the size and did not also want the canonical
representation was the -s flag to cat-file.
So I'm kind of against adding something that would want to print
that canonical representation for every subtree in a parent tree,
as it would make either pack v4 less efficient for that operation
or force it to store the canonical size, for no other good reason.
--
Shawn.