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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 19, 2025, 07:09 UTC
Message-ID: <CAPig+cSptp+a7jnUp3Tg=7D8WYKFNz4xWU2eaH+X5uy2mWjvgg@mail.gmail.com>
In-Reply-To: <b42d0f668b4a5ba0ec00fed1377cad5488f62197.1763482051.git.phillip.wood@dunelm.org.uk>

On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 5 quoted lines

> 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.

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

Show 14 quoted lines

> 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.

Show 8 quoted lines

>                 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.