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

> shejialuo <shejialuo@gmail.com> 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).
>