Re: [PATCH 5/5] strbuf: simplify strbuf_expand_literal_cb()

From: Jeff King <peff@peff.net>
Date: Jun 27, 2023, 08:57 UTC
Message-ID: <20230627085701.GK1226768@coredump.intra.peff.net>
In-Reply-To: <ded69969-158d-b05f-fdd4-91b26e9b502b@web.de>

On Sat, Jun 17, 2023 at 10:44:00PM +0200, René Scharfe wrote:

> Now that strbuf_expand_literal_cb() is no longer used as a callback,
> drop its "_cb" name suffix and unused context parameter.

Makes sense.

Since most callers just call "format += len", it kind of feels like the appropriate interface might be more like:

  strbuf_expand_literal(&sb, &format);

to auto-advance the format. But I guess that gets weird with this caller:

Show 8 quoted lines

> @@ -1395,7 +1395,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  	char **slot;
> 
>  	/* these are independent of the commit */
> -	res = strbuf_expand_literal_cb(sb, placeholder, NULL);
> +	res = strbuf_expand_literal(sb, placeholder);
>  	if (res)
>  		return res;

which is still in the "return the length" mentality (OTOH, if it advanced the local copy of the placeholder pointer nobody would mind).

I dunno. It is a little thing, and I am OK with it either way (I would not even think of changing it if you were not already touching the function).

-Peff