Re: [PATCH 3/5] replace strbuf_expand_dict_cb() with strbuf_expand_step()

From: René Scharfe <l.s.r@web.de>
Date: Jun 27, 2023, 16:24 UTC
Message-ID: <68593128-21e8-a728-719c-632fccf740d6@web.de>
In-Reply-To: <20230627083526.GA1273865@coredump.intra.peff.net>

Am 27.06.23 um 10:35 schrieb Jeff King:

Show 11 quoted lines

> On Tue, Jun 27, 2023 at 04:29:02AM -0400, Jeff King wrote:
>
>> Your comment above does make me wonder if strbuf_expand_step() should be
>> quietly handling "%%" itself. But I guess strbuf_expand() doesn't, and
>> your branch.c quote-literal example probably would not want that
>> behavior.
>
> Er, nope, strbuf_expand() does handle "%%" itself. It really feels like
> we'd want strbuf_expand_step() to do so, too, then. Even if we had two
> variants (a "raw" one which doesn't handle "%%" so that branch.c could
> use it, and then another that wrapped it to handle "%%").

strbuf_expand() handles %% and unrecognized placeholders.

We could do that, or move the %% handling to strbuf_expand_literal or something else. E.g. initially I had a strbuf_expand_default that handled %% and unrecognized placeholders.

I think it's a good idea to first get used to the loop paradigm by eschewing the sugary automatic handling and having everything spelled out explicitly. It's just one more branch. Then we can come up with a better factoring after a while.

René