Re: [PATCH/RFC 06/10] strbuf: introduce strbuf_split_str_without_term()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Karthik Nayak <karthik.188@xxxxxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]