On Wed, Feb 17, 2016 at 12:52 AM, Jeff King <[email protected]> wrote:
> On Wed, Feb 17, 2016 at 12:30:05AM +0530, Karthik Nayak wrote:
>
>> Use the newly introduced strbuf_split_str_omit_term() rather than
>> using strbuf_split_str() and manually removing the ',' terminator.
>>
>> Helped-by: Eric Sunshine <[email protected]>
>> Signed-off-by: Karthik Nayak <[email protected]>
>> ---
>>  ref-filter.c | 9 +--------
>>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> Did you consider just using string_list_split for this? AFAICT, you
> don't care about the results being strbufs themselves, and it would do
> what you want without having to bother with patch 1. The result would
> look something like the patch below.
>

I haven't, thanks for bringing it up :)

> Sorry to waltz into a review of v5 with a suggestion to throw out all
> the work done in previous iterations. :-/ I just think the strbuf_split
> interface is kind of clunky and I'd be happy if we could slowly get rid
> of it rather than growing it. Maybe that's not realistic, though (some
> of the callsites _do_ want to do things like strbuf_trim() after
> splitting).
>
> -Peff

That's fine, as I see it, it's better to wait a while and get a better version
of something.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to