From 3896a932657d209ddf4a3e72f701197ae0b27361 Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Mon, 24 Aug 2026 15:33:32 -1000 Subject: Fix the submodule rows in the tree listing A submodule row linked to a path with its trailing slash stripped, was labelled by its path rather than its entry name, shared the name's element with its hash, claimed a size of zero and offered a blame button that cannot work on a gitlink. --- assets/cgit.css | 13 +++++++++++++ source/ui-shared.c | 30 +++++++++++++++++------------- source/ui-tree.c | 7 +++++-- tests/t0104-tree.sh | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 15 deletions(-) diff --git a/assets/cgit.css b/assets/cgit.css index 6a87faf..ef6c316 100644 --- a/assets/cgit.css +++ b/assets/cgit.css @@ -676,6 +676,19 @@ div#cgit .ls-mod { font-family: var(--font-mono); } +div#cgit table.list td .ls-mod { + font-style: italic; +} + +div#cgit table.list td a.ls-mod { + color: var(--link); +} + +div#cgit .ls-mod-hash { + color: var(--muted); + font-family: var(--font-mono); +} + div#cgit td.ls-size { text-align: right; font-family: var(--font-mono); diff --git a/source/ui-shared.c b/source/ui-shared.c index 05fa30b..02352bf 100644 --- a/source/ui-shared.c +++ b/source/ui-shared.c @@ -1032,7 +1032,8 @@ void cgit_submodule_link(const char *class, char *path, const char *rev) { struct string_list *list; struct string_list_item *item; - char tail, *dir; + char tail; + const char *name; size_t len; len = 0; @@ -1047,6 +1048,12 @@ void cgit_submodule_link(const char *class, char *path, const char *rev) item = lookup_path(list, path); } } + + // The row shows the entry's own name, like every other row, while + // the lookups above want the full path. + name = strrchr(path, '/'); + name = name ? name + 1 : path; + if (item || ctx.repo->module_link) { html("util, args, 1); } else { - const char *args[2]; - dir = strrchr(path, '/'); - if (dir) - dir++; - else - dir = path; - args[0] = dir; - args[1] = rev; + const char *args[] = { name, rev }; emit_module_link(ctx.repo->module_link, args, 2); } html("'>"); - html_txt(path); + html_txt(name); html(""); } else { html(""); - html_txt(path); + html_txt(name); html(""); } - html_txtf(" @ %.7s", rev); - if (item && tail) + html(" @ "); + html_txtf("%.7s", rev); + html(""); + if (tail == '/') path[len - 1] = tail; } diff --git a/source/ui-tree.c b/source/ui-tree.c index cf3938e..ce57f92 100644 --- a/source/ui-tree.c +++ b/source/ui-tree.c @@ -333,7 +333,10 @@ static void print_ls_row(const struct object_id *oid, const char *pathname, free(buf); strbuf_release(&linkpath); } - htmlf("%lu", size); + if (S_ISGITLINK(mode)) + html("-"); + else + htmlf("%lu", size); html(""); cgit_log_link("log", NULL, "button", ctx.qry.head, @@ -345,7 +348,7 @@ static void print_ls_row(const struct object_id *oid, const char *pathname, if (!S_ISGITLINK(mode)) cgit_plain_link("plain", NULL, "button", ctx.qry.head, walk->rev, fullpath.buf); - if (!S_ISDIR(mode) && ctx.repo->enable_blame) + if (!S_ISDIR(mode) && !S_ISGITLINK(mode) && ctx.repo->enable_blame) cgit_blame_link("blame", NULL, "button", ctx.qry.head, walk->rev, fullpath.buf); html("\n"); diff --git a/tests/t0104-tree.sh b/tests/t0104-tree.sh index f4039b6..a30297f 100755 --- a/tests/t0104-tree.sh +++ b/tests/t0104-tree.sh @@ -36,4 +36,53 @@ test_expect_success 'verify a+b?h=1+2 link' ' grep "/foo%2Bbar/tree/a%2Bb?h=1%2B2" tmp ' +test_expect_success 'set up a repo with a nested submodule' ' + mkrepo repos/submodules 1 && + ( + cd repos/submodules && + sub=$(git rev-parse HEAD) && + git update-index --add --cacheinfo 160000,$sub,vendor/sub && + git commit -m gitlink + ) && + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "enable-blame=1" && + echo "repo.url=submodules" && + echo "repo.path=$PWD/repos/submodules/.git" + } >submodrc +' + +test_expect_success 'generate submodules/tree/vendor' ' + CGIT_CONFIG="$PWD/submodrc" QUERY_STRING="url=submodules/tree/vendor" \ + cgit >tmp +' + +test_expect_success 'a submodule row shows the entry name alone' ' + grep "class=.ls-mod.>sub" tmp && + ! grep ">vendor/sub" tmp +' + +test_expect_success 'the submodule hash is titled with the full id' ' + sub=$(git -C repos/submodules rev-parse HEAD~1) && + grep "class=.ls-mod-hash. title=.$sub.>" tmp +' + +test_expect_success 'a submodule row shows no size' ' + grep "ls-mod" tmp >row && + grep "class=.ls-size.>-" row +' + +test_expect_success 'a submodule row offers log but not blame or plain' ' + grep "ls-mod" tmp >row && + grep ">log" row && + ! grep ">blame" row && + ! grep ">plain" row +' + +test_expect_success 'a file row still offers blame' ' + CGIT_CONFIG="$PWD/submodrc" QUERY_STRING="url=submodules/tree/" cgit >tmp && + grep ">blame" tmp +' + test_done -- cgit v2.8.0