On Fri, Aug 01, 2025 at 04:09:02PM -0700, Junio C Hamano wrote: > shejialuo <shejialuo@xxxxxxxxx> 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,
