On Tue, Apr 18, 2023 at 03:18:43PM -0400, Taylor Blau wrote:
> Introduce a variant of the `string_list_split_in_place()` function that
> takes a string of accepted delimiters.
>
> By contrast to its cousin `string_list_split_in_place()` which splits
> the given string at every instance of the single character `delim`, the
> `_multi` variant splits the given string any any character appearing in
> the string `delim`.
>
> Like `strtok()`, the `_multi` variant skips past sequential delimiting
> characters. For example:
>
> string_list_split_in_place(&xs, xstrdup("foo::bar::baz"), ":", -1);
>
> would place in `xs` the elements "foo", "bar", and "baz".
I have mixed feelings on this.
There are multiple orthogonal options here: single/multiple delimiter,
and how to deal with sequential delimiters (you call it "runs" here,
though I think of it inverted as "allow empty fields"). Plus existing
ones like maxsplit.
Your underlying function, string_list_split_in_place_1(), handles these
independently. But it seems like a subtle gotcha that
string_list_split_in_place() and its _multi() variant, which purport to
differ only in one dimension (representation of delimiter list), also
subtly differ in another dimension. Especially because it's a facet that
might not come up in simple tests, I can see somebody incorrectly
applying one or the other.
Obviously one solution is to add the "runs" option to all variants. But
I'd be hesitant to burden existing callers. So I'd propose one of:
1. Make your _1() function public, with a name like _with_options() or
something (though the function name is sadly already quite long).
Leave string_list_split_in_place() as a wrapper that behaves as
now, and have the few new callers use the with_options() variant.
2. Don't bother implementing the "runs" version. The only users would
be test programs, and nobody cares much either way for their cases.
Document in the commit message (and possibly above the function)
that this isn't a strict replacement for strtok(). That would
hopefully be enough for a potential caller to think about the
behavior, and we can add "runs" if and when somebody actually wants
it.
-Peff
