worktree list: fix column spacing

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 812774a5ca992c5523116811239bb5a2f505f677..0643a22ee58b89c63fdf22cc34823e0851b6e2c2 100644 (file)
--- 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 8ef1cad7f29d7d72d9a83f9a3de753c88304550e..a494df6d6126680af12c10d2d39b809f7d37aaa7 100755 (executable)
--- 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
 '