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