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

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <chriscool@tuxfamily.org> writes:

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

I consider it is an example that an earlier simplistic API (gets()) later
gets corrected when it is redone right (fgets()) with other changes like a
proper bounds check.

>> 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 also do not agree that you have to keep list of skip both in shell and
rev-list when you go the route I suggested.  I think a separate bisect.c
you did is a good first step to make not just the bisect machinery but the
whole bisect command into a built-in, and even if we do not do the full
rewrite in C in one go, moving these "shell script reads from refs/bisect
only to feed the result to rev-list --bisect" pattern to "shell script
updates refs/bisect and let rev-list --bisect read from there" pattern
would be a good initial step.  Oh, and I did not mean it only for "skip",
but also doing this for "good" and "bad" as well.

For example, you read "refs/bisect/skip-*" and keep that in $skip to:

 - feed it to filter_skipped() which you are making built-in with this
   series;

 - feed it to check_good_are_ancestors_of_bad that in turn calls
   check_merge_bases;

and its use is contained in bisect_next() alone.  After this series is
done, we can move the logic in check_good_are... to bisect.c and you do
not have to read refs/bisect/skip-* in the shell anymore.  IOW, we can
migrate away from the "shell reads from refs/bisect/ and feeds that to
rev-list --bisect" pattern incrementally.