shejialuo <shejialuo@xxxxxxxxx> writes:
> On Thu, Jul 31, 2025 at 03:46:03PM -0700, Junio C Hamano wrote:
>> static int split_string(struct string_list *list, const char *string, const char *delim,
>> - int maxsplit, int in_place)
>> + int maxsplit, int in_place, unsigned flags)
>> {
>> int count = 0;
>> const char *p = string;
>> @@ -320,12 +327,18 @@ static int split_string(struct string_list *list, const char *string, const char
>> for (;;) {
>> char *end;
>>
>> + if (flags & STRING_LIST_SPLIT_TRIM) {
>> + /* ltrim */
>> + while (*p && isspace(*p))
>> + p++;
>> + }
>> +
>> if (0 <= maxsplit && maxsplit <= count)
>> end = NULL;
>> else
>> end = strpbrk(p, delim);
>>
>
> In `append_one`, we would tell whether `end` is NULL. I somehow feel
> strange why we need to do that in `append_one`. Should we just set `end`
> to be `p + strlen(p)` when `end` is NULL. And then we could do rtrim
> inside this function instead of `append_one` to avoid passing "flags" to
> `append_one`.
Sorry, but I do not see why such an alternative design is a better
idea. The helper function's purpose is to stuff the substring at
[p..end), possibly after rtrimming, to the list. You could compute
rtrim in the caller, but that would make the logic here more complex
(at least, you'd need to introduce yet another variable similar to
"end" that points at the real tail of the string, and you cannot
reuse "end" for it, because of the exit condition you see below).
>> - count += append_one(list, p, end, in_place);
>> + count += append_one(list, p, end, in_place, flags);
>>
>> if (!end)
>> return count;
>
> Thanks,
> Jialuo
