Re: [PATCH 0/18] snprintf cleanups

Jeff King [off-list ref] writes:

On Tue, Mar 28, 2017 at 03:33:48PM -0700, Junio C Hamano wrote:

quoted

Jeff King [off-list ref] writes:

quoted

It's a lot of patches, but hopefully they're all pretty straightforward
to read.

Yes, quite a lot of changes.  I didn't see anything questionable in
there.

As to the "patch-id" thing, I find the alternate one slightly easier
to read.  Also, exactly because this is not a performance critical
codepath, it may be better if patch_id_add_string() filtered out
whitespaces; that would allow the source to express things in more
natural way, e.g.

		patch_id_addf(&ctx, "new file mode");
		patch_id_addf(&ctx, "%06o", p->two->mode);
		patch_id_addf(&ctx, "--- /dev/null");
		patch_id_addf(&ctx, "+++ b/%.*s", len2, p->two->path);

Or I may be going overboard by bringing "addf" into the mix X-<.

I think there are two things going on in your example.

One is that obviously patch_id_addf() removes the spaces from the
result. But we could do that now by keeping the big strbuf_addf(), and
then just walking the result and feeding non-spaces.

The second is that your addf means we are back to formatting everything
into a buffer again....

You are right to point out that I was blinded by the ugliness of
words stuck together without spaces in between, which was inherited
from the original code, and failed to see the sole point of this
series, which is to remove truncation without adding unnecessary
allocation and freeing.

Thanks for straighten my thinking out.  I think the seeming
ugliness, if it ever becomes a real problem, should be handled
outside this series after the dust settles.