Re: [PATCH 0/18] snprintf cleanups

Jeff King [off-list ref] writes:

On Wed, Mar 29, 2017 at 09:05:33AM -0700, Junio C Hamano wrote:

quoted

quoted

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.

Yeah, the no-spaces thing should almost certainly wait.

There is still the minor question of whether skipping the strbuf
entirely is nicer, even if you still have to feed it strings without
spaces (i.e., what I posted in my initial reply).

I'm OK either with the series I posted, or wrapping up the alternative
in a commit message.

I do find the updated one easier to follow (if anything it is more
compact); I do not think it is worth a reroll, but it is easy enough
to replace the patch part of the original with the updated patch and
tweak "it's easy to fix by moving to a strbuf" in its log message to
something like "But it's easy to eliminate the allocation with a few
helper functions, and it makes the result easier to follow", so I am
tempted to go that route myself...