From 7e269f2b94125c17c430dade9cdeebdb46b50daf Mon Sep 17 00:00:00 2001 From: David Yonge-Mallo Date: Mon, 12 Oct 2026 05:43:14 +0200 Subject: [PATCH] fix(view): don't hide work when the overview base is HEAD or unresolved --- .../worktree_overview_view.lua | 43 +++++- .../worktree_overview_view_spec.lua | 125 ++++++++++++++++++ 2 files changed, 165 insertions(+), 3 deletions(-) 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 c10dd13d..ece760b6 100644 --- a/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua +++ b/lua/diffview/scene/views/worktree_overview/worktree_overview_view.lua @@ -879,9 +879,46 @@ function WorktreeOverviewView:enter_selected() -- colocated repo (which `preferred_adapter = "jj"` would otherwise do). local args = { "--adapter=git", "-C=" .. entry.path } if base then - -- `A...` is git shorthand for `A...HEAD`: commits reachable from HEAD - -- but not from the merge-base with `A`. Matches the usual review scope. - table.insert(args, base .. "...") + -- If `HEAD` is reachable from `base` (equal, or behind it with no unique + -- commits), the `base...HEAD` range collapses to empty AND the DiffView + -- hides any staged/unstaged working-tree changes behind the empty + -- revision view. Drop the rev arg so the DiffView falls through to + -- working-tree-vs-HEAD, which still surfaces uncommitted work. The + -- sha-equality form of this check caught `HEAD == base` but missed the + -- ancestor case; `merge-base --is-ancestor` catches both. + local _, ancestor_code = target:exec_sync({ "merge-base", "--is-ancestor", "HEAD", base }, { + cwd = entry.path, + log_opt = { label = "WorktreeOverviewView:enter_selected", debug_level = 1 }, + }) + -- Exit 0: HEAD is an ancestor of (or equal to) base; drop the rev arg. + -- Exit 1: HEAD has unique commits; append the usual review scope. + -- Any other exit (typically 128): git could not resolve `base` or `HEAD`; + -- warn and drop the rev arg so DiffView falls through to working-tree + -- rather than failing downstream on an unresolvable `base...` range. + if ancestor_code == 1 then + -- `A...` is git shorthand for `A...HEAD`: commits reachable from HEAD + -- but not from the merge-base with `A`. Matches the usual review scope. + table.insert(args, base .. "...") + elseif ancestor_code ~= 0 then + utils.warn( + "Could not compare `" + .. base + .. "` to HEAD in " + .. entry.path + .. "; showing working-tree changes only. Check `worktree_overview.base`." + ) + end + else + -- No resolvable base means a branch whose unique work is all committed + -- (clean worktree, no `origin/HEAD` / `main` / `master`) opens into an + -- empty-looking working-tree view. Warn so the user can set + -- `worktree_overview.base` to override instead of staring at an empty + -- panel wondering where their commits went. + utils.warn( + "No comparison base resolved for " + .. entry.path + .. "; showing working-tree changes only. Set `worktree_overview.base` to override." + ) end -- Emit before dispatching so a subscriber can prepare per-worktree state diff --git a/lua/diffview/tests/functional/worktree_overview_view_spec.lua b/lua/diffview/tests/functional/worktree_overview_view_spec.lua index 7b58e920..fe88d298 100644 --- a/lua/diffview/tests/functional/worktree_overview_view_spec.lua +++ b/lua/diffview/tests/functional/worktree_overview_view_spec.lua @@ -1,4 +1,5 @@ local async = require("diffview.async") +local DiffView = require("diffview.scene.views.diff.diff_view").DiffView local GitAdapter = require("diffview.vcs.adapters.git").GitAdapter local WorktreeOverviewView = require("diffview.scene.views.worktree_overview.worktree_overview_view").WorktreeOverviewView @@ -427,4 +428,128 @@ describe("diffview.scene.views.worktree_overview WorktreeOverviewView", function end end) ) + + it( + "enter_selected drops the base rev when the worktree HEAD IS the base", + test_utils.async_test(function() + -- Selecting the main worktree (whose HEAD is the resolved base) + -- used to produce `base...HEAD` -> empty range AND hid staged / + -- unstaged work behind the empty revision view. The fix drops the + -- rev arg in that case so the DiffView falls through to a working- + -- tree-vs-HEAD view, which still surfaces an uncommitted change. + local repo = test_utils.make_repo() + + local overview + local opened + local ok, err = pcall(function() + -- Stage an uncommitted change so the fallback has something to + -- display; a passing test still asserts on the view's shape, but + -- a dirty worktree makes the regression concrete. + local f = assert(io.open(repo .. "/dirty.txt", "w")) + f:write("dirty\n") + f:close() + run({ "git", "add", "dirty.txt" }, repo) + + overview = WorktreeOverviewView({ adapter = make_adapter(repo) }) + overview:open() + + -- The repo has exactly one worktree row; select it. + assert.equals(1, #overview.entries) + vim.api.nvim_win_set_cursor(overview.winid, { 1, 0 }) + overview:enter_selected() + + opened = require("diffview.lib").get_current_view() + assert.is_not_nil(opened) + -- The overview and DiffView share `adapter.ctx.toplevel` and both + -- have a nil `rev_arg`, so pin that a *different* view opened and + -- that it's actually a DiffView before trusting the rev-arg shape. + assert.not_equals(overview, opened) + assert.is_true(opened:instanceof(DiffView)) + assert.equals(repo, opened.adapter.ctx.toplevel) + -- `rev_arg` on a working-tree-vs-HEAD DiffView is nil / empty; + -- a `base...` form would set it to that string. + local rev_arg = opened.rev_arg + assert.is_true(rev_arg == nil or rev_arg == "") + -- The staged `dirty.txt` must surface in the DiffView's staged + -- bucket; this is what the fix exists to make visible. `ready` + -- flips before `update_files` populates the panel, so wait on + -- both. + assert.is_true( + vim.wait(5000, function() + return opened.ready and #opened.files.staged > 0 + end), + "DiffView never surfaced the staged fixture" + ) + local found + for _, f in ipairs(opened.files.staged) do + if f.path == "dirty.txt" then + found = f + break + end + end + assert.is_not_nil(found) + end) + + test_utils.close_view(opened) + test_utils.close_view(overview) + test_utils.cleanup_repo(repo) + async.await(async.scheduler()) + + if not ok then + error(err) + end + end) + ) + + it( + "enter_selected warns and still opens when no base can be resolved", + test_utils.async_test(function() + -- With `make_repo` renamed off `main`/`master` and no remotes, the + -- resolver returns nil. The DiffView used to open silently with the + -- working-tree-vs-HEAD fallback, which hides committed changes on + -- the branch; we now emit a visible warning so the user knows what + -- they are (not) looking at. + local repo = test_utils.make_repo() + + local overview + local opened + local warned + local original_notify = vim.notify + vim.notify = function(msg, level) + if level == vim.log.levels.WARN then + warned = msg + end + end + + local ok, err = pcall(function() + run({ "git", "branch", "-m", "feature/only" }, repo) + + overview = WorktreeOverviewView({ adapter = make_adapter(repo) }) + overview:open() + + vim.api.nvim_win_set_cursor(overview.winid, { 1, 0 }) + overview:enter_selected() + + assert.is_string(warned) + assert.is_truthy(warned:find("No comparison base", 1, true)) + opened = require("diffview.lib").get_current_view() + assert.is_not_nil(opened) + -- Without this the test could pass with no DiffView opened: the + -- current tabpage is still the overview's, so `get_current_view` + -- would hand back `overview` itself. + assert.not_equals(overview, opened) + assert.is_true(opened:instanceof(DiffView)) + end) + + vim.notify = original_notify + test_utils.close_view(opened) + test_utils.close_view(overview) + test_utils.cleanup_repo(repo) + async.await(async.scheduler()) + + if not ok then + error(err) + end + end) + ) end)