From: Junio C Hamano Date: Fri, 11 Dec 2015 22:31:19 GMT Subject: Re: [PATCH/RFC 06/10] strbuf: introduce strbuf_split_str_without_term() Message-ID: In-Reply-To: Karthik Nayak 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?