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

Re: [PATCH 2/2] worktree list: quote paths

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Nov 21, 2025, 16:20 UTC
Message-ID
<7583e2aa-ccd4-4316-b5ff-bcba0fc84898@gmail.com>
In-Reply-To
<CAPig+cSptp+a7jnUp3Tg=7D8WYKFNz4xWU2eaH+X5uy2mWjvgg@mail.gmail.com>
Hi Eric
On 19/11/2025 07:09, Eric Sunshine wrote:
Show 12 quoted lines
> On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> If a worktree path contains newlines or other control characters
>> it messes up the output of "git worktree list". Fix this by using
>> quote_path() to display the worktree path. The output of "git worktree
>> list" is designed for human consumption, scripts should be using the
>> "--porcelain" option so this change should not break them.
> 
> I believe that it would be more accurate to say "--porcelain -z" since
> that is the safe combination. Without -z, the output of --porcelain
> will be gobbledygook if names contain newlines or other control
> characters, but that's a long-standing problem[*] outside the scope of
> this series. Anyhow, probably not worth a reroll.

I agree that scripts should be using "-z" as well but I was just trying to make the point that the changes here wont affect sensibly written scripts.

Show 42 quoted lines
> [*]: There has been talk about correcting the oversight that
> --porcelain alone (without -z) fails to call quote_path(), but such a
> fix never materialized due to backward-compatibility concerns. We
> would probably need to introduce --porcelain=v2 to finally fix the
> case when -z isn't used with --porcelain.
> 
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -1028,11 +1029,14 @@ static void measure_widths(struct worktree **wt, int *abbrev,
>>          struct worktree_display *display = NULL;
>> +       struct strbuf buf = STRBUF_INIT;
>>
>>          for (i = 0; wt[i]; i++) {
>>                  int sha1_len;
>>                  ALLOC_GROW(display, i + 1, display_alloc);
>> -               display[i].width = utf8_strwidth(wt[i]->path);
>> +               quote_path(wt[i]->path, NULL, &buf, 0);
>> +               display[i].width = utf8_strwidth(buf.buf);
>> +               display[i].path = strbuf_detach(&buf, NULL);
> 
> The strbuf is unconditionally detached on each iteration.
> 
>>                  if (display[i].width > *maxwidth)
>>                          *maxwidth = display[i].width;
>> @@ -1104,6 +1108,8 @@ static int list(int ac, const char **av, const char *prefix,
>>                                  show_worktree(worktrees[i],
>>                                                &display[i], path_maxwidth, abbrev);
>>                  }
>> +               for (i = 0; display && worktrees[i]; i++)
>> +                       free(display[i].path);
> 
> And the detached buffers are correctly freed.
> 
>>                  free(display);
>>                  free_worktrees(worktrees);
> 
> Although not technically required because the strbuf is
> unconditionally detached each time through the loop, I wonder if it
> would reduce the cognitive load slightly for future readers to also
> strbuf_release(&buf) here at the end of the function. Probably not
> worth a reroll, though.

I think the counterargument is that it adds cognitive load for anyone who wonders why we're calling strbuf_release() after strbuf_detach() so I'm inclined to leave it as is.

Thanks for the thorough review
Phillip
Previous: Eric SunshineNext: Junio C Hamano
Message 6 of 8 in “worktree list: fix column alignment”
  1. 0/2 worktree list: fix column alignmentPhillip Wood, Nov 18, 2025
  2. 1/2 worktree list: fix column spacingPhillip Wood, Nov 18, 2025
  3. Eric SunshineNov 19, 2025
  4. 2/2 worktree list: quote pathsPhillip Wood, Nov 18, 2025
  5. Eric SunshineNov 19, 2025
  6. Phillip WoodNov 21, 2025
  7. Junio C HamanoNov 18, 2025
  8. Eric SunshineNov 19, 2025

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.