Re: [PATCH v2 4/7] string-list: optionally trim string pieces split by string_list_split*()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 1, 2025, 03:47 UTC
Message-ID: <xmqqcy9fog8x.fsf@gitster.g>
In-Reply-To: <aIwyIJqEIdSdKINz@mbp>

shejialuo <shejialuo@gmail.com> writes:

Show 28 quoted lines

> 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).

Show 8 quoted lines

>> -		count += append_one(list, p, end, in_place);
>> +		count += append_one(list, p, end, in_place, flags);
>>  
>>  		if (!end)
>>  			return count;
>
> Thanks,
> Jialuo