From: Eric Sunshine Date: Wed, 19 Nov 2025 07:09:42 GMT Subject: Re: [PATCH 2/2] worktree list: quote paths Message-ID: In-Reply-To: On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood 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. [*]: 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 > --- > 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.