Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2019, 20:01 UTC
Message-ID: <xmqqzhkje8ao.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <7dbee619-2495-6426-b02b-49fd59e4f028@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 6 quoted lines

>> That is, "if we are appending to an existing string, have SP to
>> separate the first element from that existing string; treat the
>> remaining elements the same way (if the buffer is empty, there is no
>> point adding SP at the beginning)".
>
> I don't think that would do what we want.

I know that there are current callers of quote_argv_pretty that either (1) expects that it will always get the leading SP for free, or (2) has to work the unwanted SP around (basically, the places you changed from quote_argv_pretty to quote_argv_pretty_ltrim in your series). But I wanted to see if we can come up with a single helper whose behaviour is easy to explain and understand that both existing and new callers can adopt---and if the resulting codebase becomes easy to understand and maintain overall. And if that would give us the ideal longer term direction.

Show 8 quoted lines

>> I may have found a long-standing bug in sq_quote_buf_pretty(), by
>> the way.  What does it produce when *src is an empty string of
>> length 0?  It does not add anything to dst, but shouldn't we be
>> adding two single-quotes (i.e. an empty string inside sq pair)?
>
> I would think so.  I did a quick grep and most of the calls looked
> guarded, so I don't think this is urgent.  I'll address this in a
> separate commit shortly.

Thanks.