Re: [PATCH 1/7] strbuf: add "include_delim" parameter to "strbuf_split"

[Christian Couder <chriscool@tuxfamily.org>]

Le vendredi 13 mars 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Yes, my patch does not do that, because I think including the delimiter
> > is a special case of the more general and useful behavior of not
> > including it.
>
> You got it backwards.
>
> With the way the returned string is used by the single caller that your
> patch adds (which splits at ","), I would agree that lack of delimiter
> allows the result to get used directly in the further processing.
>
> But even in that codepath, I have to say that it is just lazy programming
> that the caller does not postprocess the returned value from the splitter
> function.  If it wants not just accept input such as "a,b,c" but also
> wants to tolerate things like "a, b, c", it will have to look at the
> resulting string, and ignoring the delimiter at the end becomes just a
> small part of the general clean-up process [*1*].

I think talking about "lazy programming" in this case is a bit strong, 
because first "git rev-list --bisect-skip" is plumbing and will be used 
mostly by porcelain and second because there are much more common shell 
utilities that don't tolerate things like "a, b, c". Try using "cut" 
with -f'1, 2' (instead of -f1,2) for example.

> Once you start allowing "split at one of these characters" and/or "split
> at delimiter that matches this pattern", you cannot just discard the
> delimiter if you want to support non-simplistic callers, because they
> would want to know what the delimiter was.

But I let non simplistic callers use "1" as the last parameter if they want 
the delimiter. I just give one more way to use strbuf_split. I don't remove 
anything.

> Stripping out the delimiter is the special case for simplistic callers
> (think "gets()" that strips, and "fgets()" that doesn't).  

Aren't gets and fgets an example that having the choice to strip out the 
delimiter or not is good?

> A more general 
> solution should be by default not to strip it, and I do not think your
> new caller, if it were written correctly, needs stripping behaviour
> either. That means there is no need for the "optionally strip" flag to
> the function in order to support the rest of the series [*2*].

I think my patch 8/7, that I just sent, is a good solution and it still uses 
the new behavior of strbuf_split introduced in 1/7.

> Also comparing this with Perl/Python split() forgets that you are working
> in C, where truncating an existing string is quite cheap (just assign
> '\0'). There is a different trade-off to be made in these language
> environments.

Sorry but I think the goal of the strbuf API is to be quite high level, so I 
think comparing this with Perl/Python is ok.

Best regards,
Christian.