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, 23:09 UTC
Message-ID: <xmqqa54iljw1.fsf@gitster.g>
In-Reply-To: <aIw85gIurfEhvuPC@mbp>

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

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