git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 7, 2019, 20:37 UTC
Message-ID
<xmqqef1whfuj.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<827ed342-1079-8145-ed78-1b955264226e@jeffhostetler.com>
Jeff Hostetler <git@jeffhostetler.com> writes:
Show 26 quoted lines
> On 8/1/2019 5:34 PM, Junio C Hamano wrote:
>> "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 do not think this topic/discussion is a place to do so, either. And compensating on the caller's side, like your patch did, is perfectly fine.

I was more disturbed by potential double quoting in some codepaths when I did a cursory audit of users of sq_quote_argv_pretty(), and would think that would be of more importance, though.

Previous: Jeff HostetlerNext: Josh Steadmon
Message 7 of 33 in “trace2: clean up formatting in perf target format”
  1. 0/3 trace2: clean up formatting in perf target formatJeff Hostetler via GitGitGadget, Jul 31, 2019
  2. 1/3 trace2: cleanup column alignment in perf target formatJeff Hostetler via GitGitGadget, Jul 31, 2019
  3. 3/3 trace2: trim whitespace in region messages in perf target formatJeff Hostetler via GitGitGadget, Jul 31, 2019
  4. 2/3 trace2: trim whitespace in start message in perf target formatJeff Hostetler via GitGitGadget, Jul 31, 2019
  5. Junio C HamanoAug 1, 2019
  6. Jeff HostetlerAug 7, 2019
  7. Junio C HamanoAug 7, 2019
  8. Josh SteadmonAug 1, 2019
  9. 0/7 trace2: clean up formatting in perf target formatJeff Hostetler via GitGitGadget, Aug 8, 2019
  10. 1/7 trace2: cleanup column alignment in perf target formatJeff Hostetler via GitGitGadget, Aug 8, 2019
  11. 3/7 trace2: remove dead code in maybe_add_string_va()Jeff Hostetler via GitGitGadget, Aug 8, 2019
  12. 6/7 trace2: cleanup whitespace in normal formatJeff Hostetler via GitGitGadget, Aug 8, 2019
  13. 5/7 quote: add sq_quote_argv_pretty_ltrimJeff Hostetler via GitGitGadget, Aug 8, 2019
  14. Junio C HamanoAug 8, 2019
  15. Jeff HostetlerAug 8, 2019
  16. Junio C HamanoAug 8, 2019
  17. René ScharfeAug 8, 2019
  18. Jeff HostetlerAug 9, 2019
  19. René ScharfeAug 9, 2019
  20. 7/7 trace2: cleanup whitespace in perf formatJeff Hostetler via GitGitGadget, Aug 8, 2019
  21. 4/7 trace2: trim trailing whitespace in normal format error messageJeff Hostetler via GitGitGadget, Aug 8, 2019
  22. 2/7 trace2: trim whitespace in region messages in perf target formatJeff Hostetler via GitGitGadget, Aug 8, 2019
  23. 0/7 trace2: clean up formatting in perf target formatJeff Hostetler via GitGitGadget, Aug 9, 2019
  24. 1/7 trace2: cleanup column alignment in perf target formatJeff Hostetler via GitGitGadget, Aug 9, 2019
  25. 2/7 trace2: trim whitespace in region messages in perf target formatJeff Hostetler via GitGitGadget, Aug 9, 2019
  26. 4/7 trace2: trim trailing whitespace in normal format error messageJeff Hostetler via GitGitGadget, Aug 9, 2019
  27. 6/7 trace2: cleanup whitespace in normal formatJeff Hostetler via GitGitGadget, Aug 9, 2019
  28. 7/7 trace2: cleanup whitespace in perf formatJeff Hostetler via GitGitGadget, Aug 9, 2019
  29. 3/7 trace2: remove dead code in maybe_add_string_va()Jeff Hostetler via GitGitGadget, Aug 9, 2019
  30. 5/7 quote: add sq_append_quote_argv_pretty()Jeff Hostetler via GitGitGadget, Aug 9, 2019
  31. Junio C HamanoAug 9, 2019
  32. Jeff HostetlerAug 9, 2019
  33. Junio C HamanoAug 9, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.