Hi Eric

On 19/11/2025 07:09, Eric Sunshine wrote:

On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood [off-list ref] wrote:

quoted

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.

[*]: 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.

quoted

Signed-off-by: Phillip Wood <redacted>
---

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.

quoted

                 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.

quoted

                 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