Re: [PATCH v4] rev-list: support human-readable output for `--disk-usage`

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> I don't care much between "printf directly vs strbuf" for the non-human
> case, but it was an earlier review suggestion to connect them. I do
> think the result is a little easier to follow, but mostly I want to make
> it clear that the author is getting stuck between warring review
> comments here. ;)

The function does look like it can later be called millions of
times, at which time we may want to measure, but as the only use of
this helper function is to get called just once at the end with a
single number and print it (as opposed to getting called once per
each object), I do not care too much about the normal case being
made unnecessarily more expensive, either.

I still do not see the point of changing it to print to a strbuf and
puts() the result, though.  It does not make the code shorter, more
efficient, or more readable.  Once "if (we are producing humanize
format)" condition is hit, both of its branches can either be (1)
responsible to print the number to the standard output stream, using
whatever implementation, or (2) responsible to print the number to a
strbuf, so that somebody outside the if statement will be
respohnsible for printing that string to the standard output stream.

The patch chooses (2), which is more complex, for no good reason.  A
good thing about (1) is that the non-human codepath can STAY to be
the same as before, which is one fewer chance to introduce
unnecessary bugs.

Anyway...