{"thread":{"id":"64506","subject":"[PATCH 0/2] worktree list: fix column alignment","startedAt":"2025-11-18T16:07:51Z","lastAt":"2025-11-21T16:20:45Z","messageCount":8,"participants":["Phillip Wood","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"530908","messageId":"cover.1763482051.git.phillip.wood@dunelm.org.uk","threadId":"64506","inReplyTo":null,"subject":"[PATCH 0/2] worktree list: fix column alignment","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-18T16:07:31Z","receivedAt":"2025-11-18T16:07:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf a worktree path contains a multibyte character we end up with\nexcess padding between the columns in the output of \"git worktree\nlist\". This series fixes that and quotes the path to avoid control\ncharacters messing up the output as well.\n\nBase-Commit: fd372d9b1a69a01a676398882bbe3840bf51fe72\nPublished-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-list-spacing%2Fv1\nView-Changes-At: https://github.com/phillipwood/git/compare/fd372d9b1...b42d0f668\nFetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-list-spacing/v1\n\n\nPhillip Wood (2):\n  worktree list: fix column spacing\n  worktree list: quote paths\n\n builtin/worktree.c       | 41 ++++++++++++++++++++++++++++------------\n t/t2402-worktree-list.sh | 37 +++++++++++++++++++++++-------------\n 2 files changed, 53 insertions(+), 25 deletions(-)\n\n-- \n2.52.0.345.g9c3c96ee5a7\n\n"},{"id":"530909","messageId":"9417c73b3c4b89ed7c4cb823f3f68e994a968021.1763482051.git.phillip.wood@dunelm.org.uk","threadId":"64506","inReplyTo":"cover.1763482051.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 1/2] worktree list: fix column spacing","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-18T16:07:32Z","receivedAt":"2025-11-18T16:07:52Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nThe output of \"git worktree list\" displays a table containing the\nworktree path, HEAD OID and branch name for each worktree. The code\naligns the columns by measuring the visual width of the worktree path\nwhen it is printed. Unfortunately it fails to use the visual width\nwhen calculating the width of the column so, if any of the paths\ncontain a multibyte character, we can end up with excess padding\nbetween columns. The simplest fix would be to replace strlen() with\nutf8_strwidth() in measure_widths(). However that leaves us measuring\nthe visual width twice and the byte length once. By caching the visual\nwidth and printing the padding separately to the worktree path, we only\nneed to calculate the visual width once and do not need the byte length\nat all. The visual widths are stored in an arrays of structs rather\nthan an array of ints as the next commit will add more struct members.\n\nEven if there are no multibyte characters in any of the paths we still\nprint an extra space between the path and the object id as the field\nwidth is calculated as one plus the length of the path and we print an\nexplicit space as well. This is fixed by not printing the extra space.\n\nThe tests are updated to include multibyte characters in one of the\nworktree paths and to check the spacing of the columns.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n builtin/worktree.c       | 35 +++++++++++++++++++++++------------\n t/t2402-worktree-list.sh | 22 ++++++++++------------\n 2 files changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 812774a5ca9..0643a22ee58 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -979,14 +979,17 @@ static void show_worktree_porcelain(struct worktree *wt, int line_terminator)\n \tfputc(line_terminator, stdout);\n }\n \n-static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n+struct worktree_display {\n+\tint width;\n+};\n+\n+static void show_worktree(struct worktree *wt, struct worktree_display *display,\n+\t\t\t  int path_maxwidth, int abbrev_len)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tint cur_path_len = strlen(wt->path);\n-\tint path_adj = cur_path_len - utf8_strwidth(wt->path);\n \tconst char *reason;\n \n-\tstrbuf_addf(&sb, \"%-*s \", 1 + path_maxlen + path_adj, wt->path);\n+\tstrbuf_addf(&sb, \"%s%*s\", wt->path, 1 + path_maxwidth - display->width, \"\");\n \tif (wt->is_bare)\n \t\tstrbuf_addstr(&sb, \"(bare)\");\n \telse {\n@@ -1020,20 +1023,24 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n \tstrbuf_release(&sb);\n }\n \n-static void measure_widths(struct worktree **wt, int *abbrev, int *maxlen)\n+static void measure_widths(struct worktree **wt, int *abbrev,\n+\t\t\t   struct worktree_display **d, int *maxwidth)\n {\n-\tint i;\n+\tint i, display_alloc = 0;\n+\tstruct worktree_display *display = NULL;\n \n \tfor (i = 0; wt[i]; i++) {\n \t\tint sha1_len;\n-\t\tint path_len = strlen(wt[i]->path);\n+\t\tALLOC_GROW(display, i + 1, display_alloc);\n+\t\tdisplay[i].width = utf8_strwidth(wt[i]->path);\n \n-\t\tif (path_len > *maxlen)\n-\t\t\t*maxlen = path_len;\n+\t\tif (display[i].width > *maxwidth)\n+\t\t\t*maxwidth = display[i].width;\n \t\tsha1_len = strlen(repo_find_unique_abbrev(the_repository, &wt[i]->head_oid, *abbrev));\n \t\tif (sha1_len > *abbrev)\n \t\t\t*abbrev = sha1_len;\n \t}\n+\t*d = display;\n }\n \n static int pathcmp(const void *a_, const void *b_)\n@@ -1079,21 +1086,25 @@ static int list(int ac, const char **av, const char *prefix,\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"-z\", \"--porcelain\");\n \telse {\n \t\tstruct worktree **worktrees = get_worktrees();\n-\t\tint path_maxlen = 0, abbrev = DEFAULT_ABBREV, i;\n+\t\tint path_maxwidth = 0, abbrev = DEFAULT_ABBREV, i;\n+\t\tstruct worktree_display *display = NULL;\n \n \t\t/* sort worktrees by path but keep main worktree at top */\n \t\tpathsort(worktrees + 1);\n \n \t\tif (!porcelain)\n-\t\t\tmeasure_widths(worktrees, &abbrev, &path_maxlen);\n+\t\t\tmeasure_widths(worktrees, &abbrev,\n+\t\t\t\t       &display, &path_maxwidth);\n \n \t\tfor (i = 0; worktrees[i]; i++) {\n \t\t\tif (porcelain)\n \t\t\t\tshow_worktree_porcelain(worktrees[i],\n \t\t\t\t\t\t\tline_terminator);\n \t\t\telse\n-\t\t\t\tshow_worktree(worktrees[i], path_maxlen, abbrev);\n+\t\t\t\tshow_worktree(worktrees[i],\n+\t\t\t\t\t      &display[i], path_maxwidth, abbrev);\n \t\t}\n+\t\tfree(display);\n \t\tfree_worktrees(worktrees);\n \t}\n \treturn 0;\ndiff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh\nindex 8ef1cad7f29..a494df6d612 100755\n--- a/t/t2402-worktree-list.sh\n+++ b/t/t2402-worktree-list.sh\n@@ -30,22 +30,20 @@ test_expect_success 'rev-parse --git-path objects linked worktree' '\n '\n \n test_expect_success '\"list\" all worktrees from main' '\n-\techo \"$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n-\ttest_when_finished \"rm -rf here out actual expect && git worktree prune\" &&\n-\tgit worktree add --detach here main &&\n-\techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit worktree list >out &&\n-\tsed \"s/  */ /g\" <out >actual &&\n+\techo \"$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n+\ttest_when_finished \"rm -rf áááá out actual expect && git worktree prune\" &&\n+\tgit worktree add --detach áááá main &&\n+\techo \"$(git -C áááá rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit worktree list >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success '\"list\" all worktrees from linked' '\n-\techo \"$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n-\ttest_when_finished \"rm -rf here out actual expect && git worktree prune\" &&\n-\tgit worktree add --detach here main &&\n-\techo \"$(git -C here rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n-\tgit -C here worktree list >out &&\n-\tsed \"s/  */ /g\" <out >actual &&\n+\techo \"$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n+\ttest_when_finished \"rm -rf áááá out actual expect && git worktree prune\" &&\n+\tgit worktree add --detach áááá main &&\n+\techo \"$(git -C áááá rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)\" >>expect &&\n+\tgit -C áááá worktree list >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.52.0.345.g9c3c96ee5a7\n\n"},{"id":"530910","messageId":"b42d0f668b4a5ba0ec00fed1377cad5488f62197.1763482051.git.phillip.wood@dunelm.org.uk","threadId":"64506","inReplyTo":"cover.1763482051.git.phillip.wood@dunelm.org.uk","subject":"[PATCH 2/2] worktree list: quote paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-18T16:07:33Z","receivedAt":"2025-11-18T16:07:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf a worktree path contains newlines or other control characters\nit messes up the output of \"git worktree list\". Fix this by using\nquote_path() to display the worktree path. The output of \"git worktree\nlist\" is designed for human consumption, scripts should be using the\n\"--porcelain\" option so this change should not break them.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n builtin/worktree.c       | 10 ++++++++--\n t/t2402-worktree-list.sh | 15 ++++++++++++++-\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 0643a22ee58..303cc3b2d64 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -980,6 +980,7 @@ static void show_worktree_porcelain(struct worktree *wt, int line_terminator)\n }\n \n struct worktree_display {\n+\tchar *path;\n \tint width;\n };\n \n@@ -989,7 +990,7 @@ static void show_worktree(struct worktree *wt, struct worktree_display *display,\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *reason;\n \n-\tstrbuf_addf(&sb, \"%s%*s\", wt->path, 1 + path_maxwidth - display->width, \"\");\n+\tstrbuf_addf(&sb, \"%s%*s\", display->path, 1 + path_maxwidth - display->width, \"\");\n \tif (wt->is_bare)\n \t\tstrbuf_addstr(&sb, \"(bare)\");\n \telse {\n@@ -1028,11 +1029,14 @@ static void measure_widths(struct worktree **wt, int *abbrev,\n {\n \tint i, display_alloc = 0;\n \tstruct worktree_display *display = NULL;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \tfor (i = 0; wt[i]; i++) {\n \t\tint sha1_len;\n \t\tALLOC_GROW(display, i + 1, display_alloc);\n-\t\tdisplay[i].width = utf8_strwidth(wt[i]->path);\n+\t\tquote_path(wt[i]->path, NULL, &buf, 0);\n+\t\tdisplay[i].width = utf8_strwidth(buf.buf);\n+\t\tdisplay[i].path = strbuf_detach(&buf, NULL);\n \n \t\tif (display[i].width > *maxwidth)\n \t\t\t*maxwidth = display[i].width;\n@@ -1104,6 +1108,8 @@ static int list(int ac, const char **av, const char *prefix,\n \t\t\t\tshow_worktree(worktrees[i],\n \t\t\t\t\t      &display[i], path_maxwidth, abbrev);\n \t\t}\n+\t\tfor (i = 0; display && worktrees[i]; i++)\n+\t\t\tfree(display[i].path);\n \t\tfree(display);\n \t\tfree_worktrees(worktrees);\n \t}\ndiff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh\nindex a494df6d612..e0c6abd2f58 100755\n--- a/t/t2402-worktree-list.sh\n+++ b/t/t2402-worktree-list.sh\n@@ -29,7 +29,8 @@ test_expect_success 'rev-parse --git-path objects linked worktree' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '\"list\" all worktrees from main' '\n+test_expect_success '\"list\" all worktrees from main core.quotepath=false' '\n+\ttest_config core.quotepath false &&\n \techo \"$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n \ttest_when_finished \"rm -rf áááá out actual expect && git worktree prune\" &&\n \tgit worktree add --detach áááá main &&\n@@ -38,7 +39,19 @@ test_expect_success '\"list\" all worktrees from main' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '\"list\" all worktrees from main core.quotepath=true' '\n+\ttest_config core.quotepath true &&\n+\techo \"$(git rev-parse --show-toplevel)            $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n+\ttest_when_finished \"rm -rf á out actual expect && git worktree prune\" &&\n+\tgit worktree add --detach á main &&\n+\techo \"\\\"$(git -C á rev-parse --show-toplevel)\\\" $(git rev-parse --short HEAD) (detached HEAD)\" |\n+\t\tsed s/á/\\\\\\\\303\\\\\\\\241/g >>expect &&\n+\tgit worktree list >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '\"list\" all worktrees from linked' '\n+\ttest_config core.quotepath false &&\n \techo \"$(git rev-parse --show-toplevel)      $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]\" >expect &&\n \ttest_when_finished \"rm -rf áááá out actual expect && git worktree prune\" &&\n \tgit worktree add --detach áááá main &&\n-- \n2.52.0.345.g9c3c96ee5a7\n\n"},{"id":"530912","messageId":"xmqqzf8je0il.fsf@gitster.g","threadId":"64506","inReplyTo":"cover.1763482051.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 0/2] worktree list: fix column alignment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-18T17:03:30Z","receivedAt":"2025-11-18T17:03:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> If a worktree path contains a multibyte character we end up with\n> excess padding between the columns in the output of \"git worktree\n> list\". This series fixes that and quotes the path to avoid control\n> characters messing up the output as well.\n\nGreat.  Thanks.\n\n\n>\n> Base-Commit: fd372d9b1a69a01a676398882bbe3840bf51fe72\n> Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-list-spacing%2Fv1\n> View-Changes-At: https://github.com/phillipwood/git/compare/fd372d9b1...b42d0f668\n> Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-list-spacing/v1\n>\n>\n> Phillip Wood (2):\n>   worktree list: fix column spacing\n>   worktree list: quote paths\n>\n>  builtin/worktree.c       | 41 ++++++++++++++++++++++++++++------------\n>  t/t2402-worktree-list.sh | 37 +++++++++++++++++++++++-------------\n>  2 files changed, 53 insertions(+), 25 deletions(-)\n"},{"id":"530944","messageId":"CAPig+cQyP=v2MBEUE=fSON-N-vJgxT-bmVV8nWnoz0JYGc89Ww@mail.gmail.com","threadId":"64506","inReplyTo":"cover.1763482051.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 0/2] worktree list: fix column alignment","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-11-19T06:50:05Z","receivedAt":"2025-11-19T06:50:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n> If a worktree path contains a multibyte character we end up with\n> excess padding between the columns in the output of \"git worktree\n> list\". This series fixes that and quotes the path to avoid control\n> characters messing up the output as well.\n\nThanks for Cc:'ing me. I've added Michael Rappazzo to the Cc: list, as\nwell, since he authored bb9c03b82a (worktree: add 'list' command,\n2015-10-08) which added the code in question.\n"},{"id":"530945","messageId":"CAPig+cQaOx7yptQT=eDfVcsv_NbRseR+5Dvpm4E95z2HMpEKag@mail.gmail.com","threadId":"64506","inReplyTo":"9417c73b3c4b89ed7c4cb823f3f68e994a968021.1763482051.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 1/2] worktree list: fix column spacing","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-11-19T06:55:11Z","receivedAt":"2025-11-19T06:55:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n> The output of \"git worktree list\" displays a table containing the\n> worktree path, HEAD OID and branch name for each worktree. The code\n> aligns the columns by measuring the visual width of the worktree path\n> when it is printed. Unfortunately it fails to use the visual width\n> when calculating the width of the column so, if any of the paths\n> contain a multibyte character, we can end up with excess padding\n> between columns. The simplest fix would be to replace strlen() with\n> utf8_strwidth() in measure_widths(). However that leaves us measuring\n> the visual width twice and the byte length once. By caching the visual\n> width and printing the padding separately to the worktree path, we only\n> need to calculate the visual width once and do not need the byte length\n> at all. The visual widths are stored in an arrays of structs rather\n> than an array of ints as the next commit will add more struct members.\n> [...]\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> @@ -1020,20 +1023,24 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len)\n> +static void measure_widths(struct worktree **wt, int *abbrev,\n> +                          struct worktree_display **d, int *maxwidth)\n>  {\n> -       int i;\n> +       int i, display_alloc = 0;\n> +       struct worktree_display *display = NULL;\n>\n>         for (i = 0; wt[i]; i++) {\n>                 int sha1_len;\n> -               int path_len = strlen(wt[i]->path);\n> +               ALLOC_GROW(display, i + 1, display_alloc);\n> +               display[i].width = utf8_strwidth(wt[i]->path);\n>\n> -               if (path_len > *maxlen)\n> -                       *maxlen = path_len;\n> +               if (display[i].width > *maxwidth)\n> +                       *maxwidth = display[i].width;\n>                 sha1_len = strlen(repo_find_unique_abbrev(the_repository, &wt[i]->head_oid, *abbrev));\n>                 if (sha1_len > *abbrev)\n>                         *abbrev = sha1_len;\n>         }\n> +       *d = display;\n>  }\n\nThe reason you're using ALLOC_GROW() rather than simply allocating the\nentire `display` array at the start is that `wt` is a NULL-terminated\narray, thus you don't know its length ahead of time. Makes sense.\n\n> @@ -1079,21 +1086,25 @@ static int list(int ac, const char **av, const char *prefix,\n> +               struct worktree_display *display = NULL;\n>                 if (!porcelain)\n> +                       measure_widths(worktrees, &abbrev,\n> +                                      &display, &path_maxwidth);\n> [...]\n> +               free(display);\n>                 free_worktrees(worktrees);\n\n`display` is correctly freed. Good.\n"},{"id":"530946","messageId":"CAPig+cSptp+a7jnUp3Tg=7D8WYKFNz4xWU2eaH+X5uy2mWjvgg@mail.gmail.com","threadId":"64506","inReplyTo":"b42d0f668b4a5ba0ec00fed1377cad5488f62197.1763482051.git.phillip.wood@dunelm.org.uk","subject":"Re: [PATCH 2/2] worktree list: quote paths","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-11-19T07:09:42Z","receivedAt":"2025-11-19T07:09:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n> If a worktree path contains newlines or other control characters\n> it messes up the output of \"git worktree list\". Fix this by using\n> quote_path() to display the worktree path. The output of \"git worktree\n> list\" is designed for human consumption, scripts should be using the\n> \"--porcelain\" option so this change should not break them.\n\nI believe that it would be more accurate to say \"--porcelain -z\" since\nthat is the safe combination. Without -z, the output of --porcelain\nwill be gobbledygook if names contain newlines or other control\ncharacters, but that's a long-standing problem[*] outside the scope of\nthis series. Anyhow, probably not worth a reroll.\n\n[*]: There has been talk about correcting the oversight that\n--porcelain alone (without -z) fails to call quote_path(), but such a\nfix never materialized due to backward-compatibility concerns. We\nwould probably need to introduce --porcelain=v2 to finally fix the\ncase when -z isn't used with --porcelain.\n\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> @@ -1028,11 +1029,14 @@ static void measure_widths(struct worktree **wt, int *abbrev,\n>         struct worktree_display *display = NULL;\n> +       struct strbuf buf = STRBUF_INIT;\n>\n>         for (i = 0; wt[i]; i++) {\n>                 int sha1_len;\n>                 ALLOC_GROW(display, i + 1, display_alloc);\n> -               display[i].width = utf8_strwidth(wt[i]->path);\n> +               quote_path(wt[i]->path, NULL, &buf, 0);\n> +               display[i].width = utf8_strwidth(buf.buf);\n> +               display[i].path = strbuf_detach(&buf, NULL);\n\nThe strbuf is unconditionally detached on each iteration.\n\n>                 if (display[i].width > *maxwidth)\n>                         *maxwidth = display[i].width;\n> @@ -1104,6 +1108,8 @@ static int list(int ac, const char **av, const char *prefix,\n>                                 show_worktree(worktrees[i],\n>                                               &display[i], path_maxwidth, abbrev);\n>                 }\n> +               for (i = 0; display && worktrees[i]; i++)\n> +                       free(display[i].path);\n\nAnd the detached buffers are correctly freed.\n\n>                 free(display);\n>                 free_worktrees(worktrees);\n\nAlthough not technically required because the strbuf is\nunconditionally detached each time through the loop, I wonder if it\nwould reduce the cognitive load slightly for future readers to also\nstrbuf_release(&buf) here at the end of the function. Probably not\nworth a reroll, though.\n"},{"id":"531148","messageId":"7583e2aa-ccd4-4316-b5ff-bcba0fc84898@gmail.com","threadId":"64506","inReplyTo":"CAPig+cSptp+a7jnUp3Tg=7D8WYKFNz4xWU2eaH+X5uy2mWjvgg@mail.gmail.com","subject":"Re: [PATCH 2/2] worktree list: quote paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-11-21T16:20:39Z","receivedAt":"2025-11-21T16:20:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Eric\n\nOn 19/11/2025 07:09, Eric Sunshine wrote:\n> On Tue, Nov 18, 2025 at 11:07 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>> If a worktree path contains newlines or other control characters\n>> it messes up the output of \"git worktree list\". Fix this by using\n>> quote_path() to display the worktree path. The output of \"git worktree\n>> list\" is designed for human consumption, scripts should be using the\n>> \"--porcelain\" option so this change should not break them.\n> \n> I believe that it would be more accurate to say \"--porcelain -z\" since\n> that is the safe combination. Without -z, the output of --porcelain\n> will be gobbledygook if names contain newlines or other control\n> characters, but that's a long-standing problem[*] outside the scope of\n> this series. Anyhow, probably not worth a reroll.\n\nI agree that scripts should be using \"-z\" as well but I was just trying \nto make the point that the changes here wont affect sensibly written \nscripts.\n\n> [*]: There has been talk about correcting the oversight that\n> --porcelain alone (without -z) fails to call quote_path(), but such a\n> fix never materialized due to backward-compatibility concerns. We\n> would probably need to introduce --porcelain=v2 to finally fix the\n> case when -z isn't used with --porcelain.\n> \n>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>> ---\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> @@ -1028,11 +1029,14 @@ static void measure_widths(struct worktree **wt, int *abbrev,\n>>          struct worktree_display *display = NULL;\n>> +       struct strbuf buf = STRBUF_INIT;\n>>\n>>          for (i = 0; wt[i]; i++) {\n>>                  int sha1_len;\n>>                  ALLOC_GROW(display, i + 1, display_alloc);\n>> -               display[i].width = utf8_strwidth(wt[i]->path);\n>> +               quote_path(wt[i]->path, NULL, &buf, 0);\n>> +               display[i].width = utf8_strwidth(buf.buf);\n>> +               display[i].path = strbuf_detach(&buf, NULL);\n> \n> The strbuf is unconditionally detached on each iteration.\n> \n>>                  if (display[i].width > *maxwidth)\n>>                          *maxwidth = display[i].width;\n>> @@ -1104,6 +1108,8 @@ static int list(int ac, const char **av, const char *prefix,\n>>                                  show_worktree(worktrees[i],\n>>                                                &display[i], path_maxwidth, abbrev);\n>>                  }\n>> +               for (i = 0; display && worktrees[i]; i++)\n>> +                       free(display[i].path);\n> \n> And the detached buffers are correctly freed.\n> \n>>                  free(display);\n>>                  free_worktrees(worktrees);\n> \n> Although not technically required because the strbuf is\n> unconditionally detached each time through the loop, I wonder if it\n> would reduce the cognitive load slightly for future readers to also\n> strbuf_release(&buf) here at the end of the function. Probably not\n> worth a reroll, though.\n\nI think the counterargument is that it adds cognitive load for anyone \nwho wonders why we're calling strbuf_release() after strbuf_detach() so \nI'm inclined to leave it as is.\n\nThanks for the thorough review\n\nPhillip\n\n"}]}