[PATCH 0/2] worktree list: fix column alignment

[PATCH 0/2] worktree list: fix column alignment

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If a worktree path contains a multibyte character we end up with
excess padding between the columns in the output of "git worktree
list". This series fixes that and quotes the path to avoid control
characters messing up the output as well.

Base-Commit: fd372d9b1a69a01a676398882bbe3840bf51fe72
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-list-spacing%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/fd372d9b1...b42d0f668
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-list-spacing/v1


Phillip Wood (2):
  worktree list: fix column spacing
  worktree list: quote paths

 builtin/worktree.c       | 41 ++++++++++++++++++++++++++++------------
 t/t2402-worktree-list.sh | 37 +++++++++++++++++++++++-------------
 2 files changed, 53 insertions(+), 25 deletions(-)

-- 
2.52.0.345.g9c3c96ee5a7

[PATCH 1/2] worktree list: fix column spacing

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

The output of "git worktree list" displays a table containing the
worktree path, HEAD OID and branch name for each worktree. The code
aligns the columns by measuring the visual width of the worktree path
when it is printed. Unfortunately it fails to use the visual width
when calculating the width of the column so, if any of the paths
contain a multibyte character, we can end up with excess padding
between columns. The simplest fix would be to replace strlen() with
utf8_strwidth() in measure_widths(). However that leaves us measuring
the visual width twice and the byte length once. By caching the visual
width and printing the padding separately to the worktree path, we only
need to calculate the visual width once and do not need the byte length
at all. The visual widths are stored in an arrays of structs rather
than an array of ints as the next commit will add more struct members.

Even if there are no multibyte characters in any of the paths we still
print an extra space between the path and the object id as the field
width is calculated as one plus the length of the path and we print an
explicit space as well. This is fixed by not printing the extra space.

The tests are updated to include multibyte characters in one of the
worktree paths and to check the spacing of the columns.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/worktree.c       | 35 +++++++++++++++++++++++------------
 t/t2402-worktree-list.sh | 22 ++++++++++------------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 812774a5ca9..0643a22ee58 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -979,14 +979,17 @@ static void show_worktree_porcelain(struct worktree *wt, int line_terminator)
 	fputc(line_terminator, stdout);
 }
 
-static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
+struct worktree_display {
+	int width;
+};
+
+static void show_worktree(struct worktree *wt, struct worktree_display *display,
+			  int path_maxwidth, int abbrev_len)
 {
 	struct strbuf sb = STRBUF_INIT;
-	int cur_path_len = strlen(wt->path);
-	int path_adj = cur_path_len - utf8_strwidth(wt->path);
 	const char *reason;
 
-	strbuf_addf(&sb, "%-*s ", 1 + path_maxlen + path_adj, wt->path);
+	strbuf_addf(&sb, "%s%*s", wt->path, 1 + path_maxwidth - display->width, "");
 	if (wt->is_bare)
 		strbuf_addstr(&sb, "(bare)");
 	else {
@@ -1020,20 +1023,24 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
 	strbuf_release(&sb);
 }
 
-static void measure_widths(struct worktree **wt, int *abbrev, int *maxlen)
+static void measure_widths(struct worktree **wt, int *abbrev,
+			   struct worktree_display **d, int *maxwidth)
 {
-	int i;
+	int i, display_alloc = 0;
+	struct worktree_display *display = NULL;
 
 	for (i = 0; wt[i]; i++) {
 		int sha1_len;
-		int path_len = strlen(wt[i]->path);
+		ALLOC_GROW(display, i + 1, display_alloc);
+		display[i].width = utf8_strwidth(wt[i]->path);
 
-		if (path_len > *maxlen)
-			*maxlen = path_len;
+		if (display[i].width > *maxwidth)
+			*maxwidth = display[i].width;
 		sha1_len = strlen(repo_find_unique_abbrev(the_repository, &wt[i]->head_oid, *abbrev));
 		if (sha1_len > *abbrev)
 			*abbrev = sha1_len;
 	}
+	*d = display;
 }
 
 static int pathcmp(const void *a_, const void *b_)
@@ -1079,21 +1086,25 @@ static int list(int ac, const char **av, const char *prefix,
 		die(_("the option '%s' requires '%s'"), "-z", "--porcelain");
 	else {
 		struct worktree **worktrees = get_worktrees();
-		int path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;
+		int path_maxwidth = 0, abbrev = DEFAULT_ABBREV, i;
+		struct worktree_display *display = NULL;
 
 		/* sort worktrees by path but keep main worktree at top */
 		pathsort(worktrees + 1);
 
 		if (!porcelain)
-			measure_widths(worktrees, &abbrev, &path_maxlen);
+			measure_widths(worktrees, &abbrev,
+				       &display, &path_maxwidth);
 
 		for (i = 0; worktrees[i]; i++) {
 			if (porcelain)
 				show_worktree_porcelain(worktrees[i],
 							line_terminator);
 			else
-				show_worktree(worktrees[i], path_maxlen, abbrev);
+				show_worktree(worktrees[i],
+					      &display[i], path_maxwidth, abbrev);
 		}
+		free(display);
 		free_worktrees(worktrees);
 	}
 	return 0;
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index 8ef1cad7f29..a494df6d612 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -30,22 +30,20 @@ test_expect_success 'rev-parse --git-path objects linked worktree' '
 '
 
 test_expect_success '"list" all worktrees from main' '
-	echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
-	test_when_finished "rm -rf here out actual expect && git worktree prune" &&
-	git worktree add --detach here main &&
-	echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git worktree list >out &&
-	sed "s/  */ /g" <out >actual &&
+	echo "$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
+	test_when_finished "rm -rf áááá out actual expect && git worktree prune" &&
+	git worktree add --detach áááá main &&
+	echo "$(git -C áááá rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
+	git worktree list >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success '"list" all worktrees from linked' '
-	echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
-	test_when_finished "rm -rf here out actual expect && git worktree prune" &&
-	git worktree add --detach here main &&
-	echo "$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
-	git -C here worktree list >out &&
-	sed "s/  */ /g" <out >actual &&
+	echo "$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
+	test_when_finished "rm -rf áááá out actual expect && git worktree prune" &&
+	git worktree add --detach áááá main &&
+	echo "$(git -C áááá rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >>expect &&
+	git -C áááá worktree list >actual &&
 	test_cmp expect actual
 '
 
-- 
2.52.0.345.g9c3c96ee5a7

[PATCH 2/2] worktree list: quote paths

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

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.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/worktree.c       | 10 ++++++++--
 t/t2402-worktree-list.sh | 15 ++++++++++++++-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 0643a22ee58..303cc3b2d64 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -980,6 +980,7 @@ static void show_worktree_porcelain(struct worktree *wt, int line_terminator)
 }
 
 struct worktree_display {
+	char *path;
 	int width;
 };
 
@@ -989,7 +990,7 @@ static void show_worktree(struct worktree *wt, struct worktree_display *display,
 	struct strbuf sb = STRBUF_INIT;
 	const char *reason;
 
-	strbuf_addf(&sb, "%s%*s", wt->path, 1 + path_maxwidth - display->width, "");
+	strbuf_addf(&sb, "%s%*s", display->path, 1 + path_maxwidth - display->width, "");
 	if (wt->is_bare)
 		strbuf_addstr(&sb, "(bare)");
 	else {
@@ -1028,11 +1029,14 @@ static void measure_widths(struct worktree **wt, int *abbrev,
 {
 	int i, display_alloc = 0;
 	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);
 
 		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);
 		free(display);
 		free_worktrees(worktrees);
 	}
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index a494df6d612..e0c6abd2f58 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -29,7 +29,8 @@ test_expect_success 'rev-parse --git-path objects linked worktree' '
 	test_cmp expect actual
 '
 
-test_expect_success '"list" all worktrees from main' '
+test_expect_success '"list" all worktrees from main core.quotepath=false' '
+	test_config core.quotepath false &&
 	echo "$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
 	test_when_finished "rm -rf áááá out actual expect && git worktree prune" &&
 	git worktree add --detach áááá main &&
@@ -38,7 +39,19 @@ test_expect_success '"list" all worktrees from main' '
 	test_cmp expect actual
 '
 
+test_expect_success '"list" all worktrees from main core.quotepath=true' '
+	test_config core.quotepath true &&
+	echo "$(git rev-parse --show-toplevel)            $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
+	test_when_finished "rm -rf á out actual expect && git worktree prune" &&
+	git worktree add --detach á main &&
+	echo "\"$(git -C á rev-parse --show-toplevel)\" $(git rev-parse --short HEAD) (detached HEAD)" |
+		sed s/á/\\\\303\\\\241/g >>expect &&
+	git worktree list >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success '"list" all worktrees from linked' '
+	test_config core.quotepath false &&
 	echo "$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
 	test_when_finished "rm -rf áááá out actual expect && git worktree prune" &&
 	git worktree add --detach áááá main &&
-- 
2.52.0.345.g9c3c96ee5a7

Re: [PATCH 0/2] worktree list: fix column alignment

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If a worktree path contains a multibyte character we end up with
> excess padding between the columns in the output of "git worktree
> list". This series fixes that and quotes the path to avoid control
> characters messing up the output as well.

Great.  Thanks.


>
> Base-Commit: fd372d9b1a69a01a676398882bbe3840bf51fe72
> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-list-spacing%2Fv1
> View-Changes-At: https://github.com/phillipwood/git/compare/fd372d9b1...b42d0f668
> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-list-spacing/v1
>
>
> Phillip Wood (2):
>   worktree list: fix column spacing
>   worktree list: quote paths
>
>  builtin/worktree.c       | 41 ++++++++++++++++++++++++++++------------
>  t/t2402-worktree-list.sh | 37 +++++++++++++++++++++++-------------
>  2 files changed, 53 insertions(+), 25 deletions(-)

Re: [PATCH 0/2] worktree list: fix column alignment

[Eric Sunshine]

On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> If a worktree path contains a multibyte character we end up with
> excess padding between the columns in the output of "git worktree
> list". This series fixes that and quotes the path to avoid control
> characters messing up the output as well.

Thanks for Cc:'ing me. I've added Michael Rappazzo to the Cc: list, as
well, since he authored bb9c03b82a (worktree: add 'list' command,
2015-10-08) which added the code in question.

Re: [PATCH 1/2] worktree list: fix column spacing

[Eric Sunshine]

On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> The output of "git worktree list" displays a table containing the
> worktree path, HEAD OID and branch name for each worktree. The code
> aligns the columns by measuring the visual width of the worktree path
> when it is printed. Unfortunately it fails to use the visual width
> when calculating the width of the column so, if any of the paths
> contain a multibyte character, we can end up with excess padding
> between columns. The simplest fix would be to replace strlen() with
> utf8_strwidth() in measure_widths(). However that leaves us measuring
> the visual width twice and the byte length once. By caching the visual
> width and printing the padding separately to the worktree path, we only
> need to calculate the visual width once and do not need the byte length
> at all. The visual widths are stored in an arrays of structs rather
> than an array of ints as the next commit will add more struct members.
> [...]
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -1020,20 +1023,24 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)
> +static void measure_widths(struct worktree **wt, int *abbrev,
> +                          struct worktree_display **d, int *maxwidth)
>  {
> -       int i;
> +       int i, display_alloc = 0;
> +       struct worktree_display *display = NULL;
>
>         for (i = 0; wt[i]; i++) {
>                 int sha1_len;
> -               int path_len = strlen(wt[i]->path);
> +               ALLOC_GROW(display, i + 1, display_alloc);
> +               display[i].width = utf8_strwidth(wt[i]->path);
>
> -               if (path_len > *maxlen)
> -                       *maxlen = path_len;
> +               if (display[i].width > *maxwidth)
> +                       *maxwidth = display[i].width;
>                 sha1_len = strlen(repo_find_unique_abbrev(the_repository, &wt[i]->head_oid, *abbrev));
>                 if (sha1_len > *abbrev)
>                         *abbrev = sha1_len;
>         }
> +       *d = display;
>  }

The reason you're using ALLOC_GROW() rather than simply allocating the
entire `display` array at the start is that `wt` is a NULL-terminated
array, thus you don't know its length ahead of time. Makes sense.

> @@ -1079,21 +1086,25 @@ static int list(int ac, const char **av, const char *prefix,
> +               struct worktree_display *display = NULL;
>                 if (!porcelain)
> +                       measure_widths(worktrees, &abbrev,
> +                                      &display, &path_maxwidth);
> [...]
> +               free(display);
>                 free_worktrees(worktrees);

`display` is correctly freed. Good.

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

[Eric Sunshine]

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.

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

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

[Phillip Wood]

Hi Eric

On 19/11/2025 07:09, Eric Sunshine wrote:
> 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.

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