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

[shejialuo <shejialuo@gmail.com>]

On Fri, Aug 01, 2025 at 04:09:02PM -0700, Junio C Hamano wrote:
> shejialuo <shejialuo@gmail.com> writes:
> 
> > I agree with you that we would introduce another variable. However, the
> > thing I quite dislike is that we do ltrim inside the current function
> > and we do rtrim inside `append_one`.
> 
> Your "quite dislike" does not matter unless backed by a reason why
> it is not good, and for that, you need to think a bit deeper.  Then
> you will hopefully appreciate why the current arrangement is more
> optimal ;-)
> 
> There is a clear separation of tasks between this caller-callee
> pair.  The caller is responsible for finding where the current token
> ends, and the callee is responsible for massaging the current token
> into the resulting list.
> 
> But ltrim needs to be done in the caller for this to be efficient.
> 
> Imagine the case where you want to allow both non-empty and trim
> behaviour at the same time, and use a whitespace character as
> delimiter.  If your token has leading whitespaces, instead of
> chopping them into zero-length ranges and feeding it to append_one()
> one by one, only to have them discarded (due to non-empty being
> set), ltrimming in the caller before it decides where the next token
> (i.e. "end") starts is far more efficient.  It may be more
> conceptually cleaner, but cleanliness is more subjective ;-)
> 

Yes, I agree. Thanks for the wonderful explanation.

> > My thinking is that we should handle the [p, end) string in the same
> > place.
> 
> Again this sounds no more than a subjective "quite dislike".  Is
> there a reason why anybody would want to insist these two things be
> done at the same location?
> 
> You could satisfy the subjective "same place requirement" by
> inlining the helper function into its sole caller and still keep the
> current arrangement to ltrim before finding "end", of course.  But
> at that point, I would have to say that it is a tail wagging the
> dog.  You are making the code worse by destroying the caller-callee
> division of responsibilities, only to satisfy a subjetive "quite
> dislike" criteria.
> 

I get your point. In the later review, I think I should avoid commenting
things using a subjetive idea. Really thanks for your suggestion.

Thanks,