From 3c83b3c11a5a219394b6b6d2cb0bae3cd76bec33 Mon Sep 17 00:00:00 2001 From: David Yonge-Mallo Date: Tue, 27 Oct 2026 16:45:14 +0100 Subject: [PATCH] fix(view): distinguish no-merge-base from probe errors in worktree overview --- doc/diffview.txt | 24 ++++++++- .../worktree_overview_view.lua | 54 ++++++++++--------- .../worktree_overview_view_spec.lua | 19 +++---- lua/diffview/vcs/adapters/git/init.lua | 12 ++++- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/doc/diffview.txt b/doc/diffview.txt index fd984821..e462e534 100644 --- a/doc/diffview.txt +++ b/doc/diffview.txt @@ -646,9 +646,9 @@ Column legend ~ fallbacks; see `base` in |diffview-config-worktree_overview|). `Nf` Files changed in `base...HEAD` - per `git diff --shortstat`. + per `git diff --numstat`. `+N` `-N` Insertion / deletion counts - from the same shortstat. + from the same numstat. `Ns` `Nu` `N?` Dirty trio from `git status --porcelain`: staged (index-vs-HEAD), @@ -665,6 +665,26 @@ Column legend ~ every row stripped so the rows read as a tidy list. + Numeric cells can collapse to one of two dim + single-column markers in place of a digit: + + `×` The probe failed, so the real + count is unknown. Treating it + as zero would misrepresent the + row as clean, so each affected + cell shows this instead. + `·` The question doesn't apply. + Currently only the diff cells + (`Nf`, `+N`, `-N`) use this, + on branches whose history is + unrelated to the resolved + base so `base...HEAD` has no + merge base and the diff is + undefined. Ahead / behind + still render since + `rev-list --left-right` + needs no merge base. + *:DiffviewLog* :DiffviewLog Open the debug log. diff --git a/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua b/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua index 2ca4f5d7..f477d657 100644 --- a/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua +++ b/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua @@ -759,22 +759,22 @@ local function pad(ctx, n) end end ----Marker used when a probe is unavailable (e.g., no base resolved, or a ----sub-probe exited non-zero). Zero would silently misrepresent the row as ----clean, so each affected cell shows a dim dash instead of a digit. -local UNKNOWN = "─" - ----Resolve a counter cell's `(text, display_width, hl)` tuple. When the ----probe is unavailable the cell collapses to a dim `UNKNOWN` marker ----(display width 1); otherwise it uses the formatted value with the ----caller-supplied highlight. ----@param unknown boolean +---Probe failed: real count unknown. Visually loud so a stale probe can't +---pass for a clean row. +local UNKNOWN = "×" + +---The question doesn't apply (e.g., diff cells on a row whose +---`base...HEAD` has no merge base). Quieter than `UNKNOWN` since nothing +---broke. +local NA = "·" + +---@param marker? string # `UNKNOWN`, `NA`, or nil for "render the number". ---@param n integer ---@param hl string ---@return string text, integer width, string hl -local function fmt_cell(unknown, n, hl) - if unknown then - return UNKNOWN, 1, HL.dim +local function fmt_cell(marker, n, hl) + if marker then + return marker, 1, HL.dim end local s = tostring(n) return s, #s, hl @@ -800,39 +800,42 @@ local function push_stats_cells(ctx, e, w) end local s = e.stats ---@cast s GitAdapter.WorktreeStats - -- Branch counters: nil base means the probes were skipped entirely, so - -- ahead/behind would silently read as 0; `errors` lists sub-probes that - -- exited non-zero. Either case renders the affected cells as unknown - -- so a stale probe doesn't masquerade as a clean row. - local branch_unknown = s.base == nil or vim.tbl_contains(s.errors, "rev-list") - local diff_unknown = s.base == nil or vim.tbl_contains(s.errors, "diff") + -- Nil base means probes were skipped; without a marker the row would + -- read as clean. Diff has the extra `no_merge_base` outcome. + local branch_marker = (s.base == nil or vim.tbl_contains(s.errors, "rev-list")) and UNKNOWN or nil + local diff_marker + if s.base == nil or vim.tbl_contains(s.errors, "diff") then + diff_marker = UNKNOWN + elseif s.no_merge_base then + diff_marker = NA + end local status_unknown = vim.tbl_contains(s.errors, "status") local text, cw, hl - text, cw, hl = fmt_cell(branch_unknown, s.ahead, HL.counter) + text, cw, hl = fmt_cell(branch_marker, s.ahead, HL.counter) pad(ctx, w.ahead - cw) push(ctx, "↑", HL.arrow) push(ctx, text, hl) push(ctx, " ") - text, cw, hl = fmt_cell(branch_unknown, s.behind, HL.counter) + text, cw, hl = fmt_cell(branch_marker, s.behind, HL.counter) pad(ctx, w.behind - cw) push(ctx, "↓", HL.arrow) push(ctx, text, hl) push(ctx, " ") - text, cw, hl = fmt_cell(diff_unknown, s.changed_files, HL.counter) + text, cw, hl = fmt_cell(diff_marker, s.changed_files, HL.counter) pad(ctx, w.files - cw) push(ctx, text, hl) push(ctx, "f", HL.files_suffix) push(ctx, " ") - text, cw, hl = fmt_cell(diff_unknown, s.insertions, HL.insertions) + text, cw, hl = fmt_cell(diff_marker, s.insertions, HL.insertions) pad(ctx, w.ins - cw) push(ctx, "+" .. text, hl) push(ctx, " ") - text, cw, hl = fmt_cell(diff_unknown, s.deletions, HL.deletions) + text, cw, hl = fmt_cell(diff_marker, s.deletions, HL.deletions) pad(ctx, w.dels - cw) push(ctx, "-" .. text, hl) push(ctx, " ") @@ -852,7 +855,8 @@ local function push_stats_cells(ctx, e, w) return end local function dirty(count, width, hl_known, suffix) - local t, w1, h = fmt_cell(status_unknown, count, count > 0 and hl_known or HL.dim) + local marker = status_unknown and UNKNOWN or nil + local t, w1, h = fmt_cell(marker, count, count > 0 and hl_known or HL.dim) pad(ctx, width - w1) push(ctx, t, h) push(ctx, suffix, h) diff --git a/lua/diffview/tests/functional/worktree_overview_view_spec.lua b/lua/diffview/tests/functional/worktree_overview_view_spec.lua index 8e28d89b..1a145296 100644 --- a/lua/diffview/tests/functional/worktree_overview_view_spec.lua +++ b/lua/diffview/tests/functional/worktree_overview_view_spec.lua @@ -303,6 +303,7 @@ describe("diffview.scene.views.worktree_overview WorktreeOverviewView", function staged = 0, unstaged = 0, untracked = 0, + no_merge_base = false, errors = { "status" }, }, } @@ -340,18 +341,18 @@ describe("diffview.scene.views.worktree_overview WorktreeOverviewView", function -- and the suffix letters `s`/`u`/`?` live only on this trio -- so -- a hit on each proves the `status_unknown` branch fired. Plain -- find keeps the `?` from being treated as a pattern. - assert.is_truthy(errored_line:find("─s", 1, true)) - assert.is_truthy(errored_line:find("─u", 1, true)) - assert.is_truthy(errored_line:find("─?", 1, true)) + assert.is_truthy(errored_line:find("×s", 1, true)) + assert.is_truthy(errored_line:find("×u", 1, true)) + assert.is_truthy(errored_line:find("×?", 1, true)) -- Fixture-independent check: anything the two rows share (nil -- base, probe errors on branch/diff) adds the same number of - -- dashes to both lines, so the delta isolates the trio's own - -- contribution. The errored row should carry exactly three - -- more dashes than the clean row. - local _, clean_dashes = clean_line:gsub("─", "") - local _, errored_dashes = errored_line:gsub("─", "") - assert.equals(3, errored_dashes - clean_dashes) + -- `UNKNOWN` markers to both lines, so the delta isolates the + -- trio's own contribution. The errored row should carry exactly + -- three more markers than the clean row. + local _, clean_markers = clean_line:gsub("×", "") + local _, errored_markers = errored_line:gsub("×", "") + assert.equals(3, errored_markers - clean_markers) end) test_utils.close_view(view) diff --git a/lua/diffview/vcs/adapters/git/init.lua b/lua/diffview/vcs/adapters/git/init.lua index 25da2549..98cf5e44 100644 --- a/lua/diffview/vcs/adapters/git/init.lua +++ b/lua/diffview/vcs/adapters/git/init.lua @@ -522,7 +522,8 @@ end ---@field staged integer # Index-vs-HEAD changes from `git status --porcelain`. ---@field unstaged integer # Worktree-vs-index changes. ---@field untracked integer ----@field errors string[] # Tags of probes that returned non-zero: `"rev-list"`, `"diff"`, `"status"`. Non-empty means the counts are partial; callers that would act destructively on a seemingly clean worktree must treat that as unknown state. A `nil` base is not an error (branch probes are skipped and `errors` stays empty for them). +---@field no_merge_base boolean # `base...HEAD` has no merge base (unrelated histories). The diff counts are undefined; `errors` stays clear of `"diff"` since the probe answered a well-defined question. Branch counters are unaffected. +---@field errors string[] # Tags of probes that returned non-zero: `"rev-list"`, `"diff"`, `"status"`. Non-empty means the counts are partial; callers that would act destructively on a seemingly clean worktree must treat that as unknown state. A `nil` base is not an error (branch probes are skipped and `errors` stays empty); nor is `no_merge_base`. ---Build the (unstarted) `Job`s that `worktree_stats` would run and a parser ---that reads their outputs into a `WorktreeStats`. Returned jobs are not @@ -575,6 +576,7 @@ function GitAdapter:build_stats_probe_jobs(toplevel, base) staged = 0, unstaged = 0, untracked = 0, + no_merge_base = false, errors = {}, } @@ -607,7 +609,13 @@ function GitAdapter:build_stats_probe_jobs(toplevel, base) end end else - table.insert(stats.errors, "diff") + -- `A...HEAD` without a merge base (unrelated histories) is a + -- well-defined n/a, not a probe error -- keep it off `errors`. + if table.concat(diff_job.stderr, "\n"):find("no merge base", 1, true) then + stats.no_merge_base = true + else + table.insert(stats.errors, "diff") + end end end