# [PATCH 0/2] worktree list: fix column alignment 8 messages from 2025-11-18 to 2025-11-21. Participants: Phillip Wood, Junio C Hamano, Eric Sunshine. Thread: https://gitlist.dev/t/64506 ## Phillip Wood, 2025-11-18 16:07 Subject: [PATCH 0/2] worktree list: fix column alignment Message-ID: URL: https://gitlist.dev/e/cover.1763482051.git.phillip.wood%40dunelm.org.uk ``` From: Phillip Wood 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 ``` ## Phillip Wood, 2025-11-18 16:07 Subject: [PATCH 1/2] worktree list: fix column spacing Message-ID: <9417c73b3c4b89ed7c4cb823f3f68e994a968021.1763482051.git.phillip.wood@dunelm.org.uk> URL: https://gitlist.dev/e/9417c73b3c4b89ed7c4cb823f3f68e994a968021.1763482051.git.phillip.wood%40dunelm.org.uk In-Reply-To: ``` From: Phillip Wood 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 --- 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" 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" 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 ``` ## Phillip Wood, 2025-11-18 16:07 Subject: [PATCH 2/2] worktree list: quote paths Message-ID: URL: https://gitlist.dev/e/b42d0f668b4a5ba0ec00fed1377cad5488f62197.1763482051.git.phillip.wood%40dunelm.org.uk In-Reply-To: ``` From: Phillip Wood 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 --- 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 ``` ## Junio C Hamano, 2025-11-18 17:03 Subject: Re: [PATCH 0/2] worktree list: fix column alignment Message-ID: URL: https://gitlist.dev/e/xmqqzf8je0il.fsf%40gitster.g In-Reply-To: ``` Phillip Wood writes: > From: Phillip Wood > > 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(-) ``` ## Eric Sunshine, 2025-11-19 06:50 Subject: Re: [PATCH 0/2] worktree list: fix column alignment Message-ID: URL: https://gitlist.dev/e/CAPig%2BcQyP%3Dv2MBEUE%3DfSON-N-vJgxT-bmVV8nWnoz0JYGc89Ww%40mail.gmail.com In-Reply-To: ``` On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood 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. ``` ## Eric Sunshine, 2025-11-19 06:55 Subject: Re: [PATCH 1/2] worktree list: fix column spacing Message-ID: URL: https://gitlist.dev/e/CAPig%2BcQaOx7yptQT%3DeDfVcsv_NbRseR%2B5Dvpm4E95z2HMpEKag%40mail.gmail.com In-Reply-To: <9417c73b3c4b89ed7c4cb823f3f68e994a968021.1763482051.git.phillip.wood@dunelm.org.uk> ``` On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood 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 > --- > 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. ``` ## Eric Sunshine, 2025-11-19 07:09 Subject: Re: [PATCH 2/2] worktree list: quote paths Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSptp%2Ba7jnUp3Tg%3D7D8WYKFNz4xWU2eaH%2BX5uy2mWjvgg%40mail.gmail.com 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. ``` ## Phillip Wood, 2025-11-21 16:20 Subject: Re: [PATCH 2/2] worktree list: quote paths Message-ID: <7583e2aa-ccd4-4316-b5ff-bcba0fc84898@gmail.com> URL: https://gitlist.dev/e/7583e2aa-ccd4-4316-b5ff-bcba0fc84898%40gmail.com In-Reply-To: ``` Hi Eric On 19/11/2025 07:09, Eric Sunshine wrote: > 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. 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 >> --- >> 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 ```