[PATCH 1/2] worktree list: fix column spacing
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(-)
Show changes to 2 files +33 −24
builtin/worktree.c, t/t2402-worktree-list.sh
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
