On Wed, Jan 11 2023, Matthew John Cheetham via GitGitGadget wrote:
> From: Matthew John Cheetham <mjcheetham@xxxxxxxxxxx>
> [...]
> + } else if (buf.len) {
> + const char *prev = values->v[values->nr - 1];
> + struct strbuf append = STRBUF_INIT;
> + strbuf_addstr(&append, prev);
> +
> + /* Join two non-empty values with a single space. */
> + if (append.len)
> + strbuf_addch(&append, ' ');
> +
> + strbuf_addbuf(&append, &buf);
> +
> + strvec_pop(values);
> + strvec_push_nodup(values, strbuf_detach(&append, NULL));
> + }
> +
I've written something like the strvec_push_nodup() patch that preceded
this myself for similar reasons, and as recently noted in [1] I think
such a thing (although I implemented a different interface) might be
useful in general.
But this really doesn't seem like a good justification for adding this
new API. Let's instead do:
} else if (buf.len) {
const char *prev = values->v[values->nr - 1];
/* Join two non-empty values with a single space. */
const char *const sp = *prev ? " " : ""
strvec_pop(values);
strvec_pushf(values, "%s%s%s", prev, sp, buf.buf);
}
There may be cases where a public strvec_push_nodup() simplifies things,
but this doesn't seem like such a case, just use strvec_pushf() directly
instead, and skip the strbuf & strbuf_detach().
I haven't compiled/tested the above, so there may e.g. be a typo in
there. But I think the general concept should work in this case.
1. https://lore.kernel.org/git/RFC-cover-0.5-00000000000-20221215T090226Z-avarab@xxxxxxxxx/
