Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Aug 7, 2019, 20:25 UTC
Message-ID: <827ed342-1079-8145-ed78-1b955264226e@jeffhostetler.com>
In-Reply-To: <xmqqef24muxe.fsf@gitster-ct.c.googlers.com>

On 8/1/2019 5:34 PM, Junio C Hamano wrote:

Show 21 quoted lines

> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> Trim leading/trailing whitespace from the command line
>> printed in the "start" message in the perf target format.
>>
>> We use `sq_quote_argv_pretty()` to format the message
>> and it adds a leading space to the output.  Trim that.
> 
> strbuf_trim() not just drops a single leading space, but removes
> consecutive spaces from both ends.  But the first char after the SP
> comes from the first arg, and it can never be a whitespace (as a
> payload that begins with a whitespace will be quoted, so it will be
> a single quote), and the last char in the buffer would also be
> either a closing single quote (if the last argument ends with a
> whitespace) or a non whitespace "safe" character, so it is safe to
> use strbuf_trim() here.
> 
> I wonder if we want to lose the prepending of SP from
> sq_quote_argv_pretty(), though:

I was wondering about that too, but I didn't want to presume to know what all of the callers wanted. And didn't want to slip in such a change last-minute.

I'll re-roll with a new sq_quote_ function that does not include the leading whitespace and convert/normalize all of my trace2/* uses to call it instead. This eliminates the Trace2 code from the larger conversation (and is a little more efficient than the strbuf_trim()).

Jeff