Re: [PATCH/RFC 06/10] strbuf: introduce strbuf_split_str_without_term()
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Dec 12, 2015, 16:04 UTC
- Message-ID
- <CAOLa=ZQeK4VGf3n7vOtUspaak9gXn+G-DV9rwWDjr_n2_LeU6g@mail.gmail.com>
- In-Reply-To
- <xmqq8u50ehxk.fsf@gitster.mtv.corp.google.com>
On Sat, Dec 12, 2015 at 4:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 33 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> diff --git a/ref-filter.c b/ref-filter.c
>>>> @@ -892,14 +892,11 @@ static void populate_value(struct ref_array_item *ref)
>>>> * TODO: Implement a function similar to strbuf_split_str()
>>>> * which would omit the separator from the end of each value.
>>>> */
>>>> - s = to_free = strbuf_split_str(valp, ',', 0);
>>>> + s = to_free = strbuf_split_str_without_term(valp, ',', 0);
>>>>
>>>> align->position = ALIGN_LEFT;
>>>>
>>>> while (*s) {
>>>> - /* Strip trailing comma */
>>>> - if (s[1])
>>>> - strbuf_setlen(s[0], s[0]->len - 1);
>>>
>>> I'd prefer to see this ref-filter.c change split out as a separate
>>> patch so as not to pollute the otherwise single-purpose change
>>> introduced by this patch (i.e. capability to omit the terminator).
>>>
>>> Also, it might make sense to move this patch to the head of the
>>> series, since it's conceptually distinct from the rest of the patches,
>>> and could conceivably prove useful on its own, regardless of how the
>>> rest of the series fares.
>>>
>>
>> I guess it makes sense to split this into two separate patches. I'll do that and
>> push it to the top of the series.
>
> Sounds good. I also notice that the "TODO: Implement a function
> similar to..." we see in the precontext can now be removed, as that
> is what is done in this patch?Yes I noticed that when going through the changes Eric suggested. I have removed it.
-- Regards, Karthik Nayak