Petr Baudis wrote:
quoted text
> Frankly, I doubt this has too much value, and it complicates the code _a
> lot_. If you can't fit your stuff into pretty column, it's better to
> just print whatever you have to and disrupt the columns instead of
> _failing_, isn't it?

Generally, the only reason for such a failure would be requesting
conflicting scale and maxlen values or requesting a maxlen which would
be too small to reasonably display any value, hence an empty string and
a number reporting how much more is necessary to get appropriate output.

In other words, a failure reports that you probably requested an
irrational number for maxlen.  It would probably be easier to understand
if it were in terms of the numeric output rather than the entire string.
If I change it that way, then there shouldn't be an "irrational" positive
number to request, eliminating the need for these failures.

quoted text
> Shouldn't pretty much all of this be documented in the code too?

Should I stuff this in comments in the header file, source file, or both?

quoted text
> My point still stands - in case of binary units, we should always
> consistently use the i suffix. So having an example in the commit
> message that advertises "bps" is simply wrong when it should read "iB/s"
> (like it does with the current progress.c code).
> 
> I may sound boring, but it seems to me that you're still ignoring my
> point quitly without proper counter-argumentation and I think it's an
> important want, and since it's so hard to keep things consistent across
> the wide Git codebase, we should do all we can to keep it.

From a consistency standpoint, I can certainly agree.  It's not hard to
implement.  I wanted to avoid pigeon-holeing, but to keep our reporting
consistent, using '*i' for all binaries works for me.

quoted text
> Whitespace damage? Also at a lot of other places in your patch.

No damage.  It was indicated to me that 80-ish was the preferred width, so
I was trying to follow that.  If that's not true in the C sources, I'll
bundle things up a bit more.

quoted text
> Hmmm. We could have
> 
> +        char *hr_prefixes[] = {
> +		"", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi", NULL
> +	};
> +        char *hr_si_prefixes[] = {
> +		"", "k", "M", "G", "T", "P", "E", "Z", "Y", NULL
> +	};
> 
> ;-)

Per previous, sounds good to me.

Overall, I was looking to create a generic function that could be used
across Git without making assumptions of the consumer.  Hence the maxlen,
scale, SI, and space configurability.

Thanks for the input, and I'll work up another draft.

-- 
Marcus Griep
GPG Key ID: 0x5E968152
——
http://www.boohaunt.net
את.ψο´
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html