Re: [PATCH 0/6] strbuf cleanups

From: Jeff King <peff@peff.net>
Date: May 7, 2023, 13:14 UTC
Message-ID: <20230507131427.GA1852750@coredump.intra.peff.net>
In-Reply-To: <CABPp-BGc6ZK919rEH7-fOVg-nYs2wtvFP-XeQ7V9dEBEJvBYsQ@mail.gmail.com>

On Sat, May 06, 2023 at 05:14:55PM -0700, Elijah Newren wrote:

Show 14 quoted lines

> On Wed, May 3, 2023 at 11:00 AM Calvin Wan <calvinwan@google.com> wrote:
> >
> > While moving strbuf_add_separated_string_list() to a separate file
> > would mean that strbuf would no longer have a dependency on
> > string-list, I don't think that dependency is problematic to begin
> > with. Widening the boundary for strbuf as a string manipulation
> > library to a string and string list manipulation library seems
> > reasonable to me.
> 
> Oh, the high level idea behind string-list might make sense at this
> level, but I was assuming Peff would show up at some point and
> highlight the evils of the current string-list API[1][2][3] and how we
> should avoid using, depending on, or implementing something that acts
> like it.  :-)

You rang? :)

Yes, IMHO this strbuf_add_separated_string_list() is another example of why string-list sucks: it doesn't degrade to a natural array type (because the "util" magic requires a struct). If it were a strvec or similar, we could just pass "const char **str, size_t len", which would make the helper function simpler and more generally useful.

I know there may be other reasons to use a string-list in the caller here, though (looks like it uses "nodup"). So as usual, the situation is not so simple as "we should just switch types".

-Peff