From 57bb570b809e913c258dbc5fe9f0f3ec95f4a83c Mon Sep 17 00:00:00 2001 From: David Yonge-Mallo Date: Thu, 7 May 2026 19:16:18 +0200 Subject: [PATCH] fix(panel): position cursor on active file when toggling panel open --- lua/diffview/scene/views/diff/file_panel.lua | 1 + .../views/file_history/file_history_panel.lua | 21 +-- .../tests/functional/cdiff_view_spec.lua | 68 ++++++++++ lua/diffview/tests/functional/panel_spec.lua | 128 ++++++++++++++++++ 4 files changed, 210 insertions(+), 8 deletions(-) diff --git a/lua/diffview/scene/views/diff/file_panel.lua b/lua/diffview/scene/views/diff/file_panel.lua index 091240af..35a12f2e 100644 --- a/lua/diffview/scene/views/diff/file_panel.lua +++ b/lua/diffview/scene/views/diff/file_panel.lua @@ -73,6 +73,7 @@ function FilePanel:open() if not (conf.type == "split" and conf.width == "auto") then vim.cmd("wincmd =") end + self:highlight_cur_file() end function FilePanel:setup_buffer() diff --git a/lua/diffview/scene/views/file_history/file_history_panel.lua b/lua/diffview/scene/views/file_history/file_history_panel.lua index 05f9a249..2f8284f8 100644 --- a/lua/diffview/scene/views/file_history/file_history_panel.lua +++ b/lua/diffview/scene/views/file_history/file_history_panel.lua @@ -117,6 +117,9 @@ function FileHistoryPanel:open() if not (conf.type == "split" and conf.width == "auto") then vim.cmd("wincmd =") end + if self.cur_item[2] then + self:highlight_item(self.cur_item[2]) + end end ---@override @@ -645,15 +648,17 @@ function FileHistoryPanel:set_entry_fold(entry, open) self:render() self:redraw() - if entry.folded then - -- Set the cursor at the top of the log entry - self.components.log.entries.comp:some(function(comp, _, _) - if comp.context == entry then - utils.set_cursor(self.winid, comp.lstart + 1) - return true - end - end) + if not (self:is_open() and entry.folded) then + return end + + -- Set the cursor at the top of the log entry. + self.components.log.entries.comp:some(function(comp, _, _) + if comp.context == entry then + utils.set_cursor(self.winid, comp.lstart + 1) + return true + end + end) end end diff --git a/lua/diffview/tests/functional/cdiff_view_spec.lua b/lua/diffview/tests/functional/cdiff_view_spec.lua index cbf56ba4..38dcd4a9 100644 --- a/lua/diffview/tests/functional/cdiff_view_spec.lua +++ b/lua/diffview/tests/functional/cdiff_view_spec.lua @@ -328,6 +328,74 @@ describe("diffview.api.CDiffView", function() end end) ) + + it( + "places the cursor on the active file when the panel is toggled open (#161)", + test_utils.async_test(function() + local repo = make_repo() + local view + + local ok, err = pcall(function() + config.get_config().file_panel.show = false + + view = CDiffView({ + git_root = repo, + left = Rev(RevType.STAGE), + right = Rev(RevType.LOCAL), + files = make_files({ + make_file_data("a.txt"), + make_file_data("b.txt"), + make_file_data("c.txt"), + }), + update_files = function() + return make_files({ + make_file_data("a.txt"), + make_file_data("b.txt"), + make_file_data("c.txt"), + }) + end, + get_file_data = function() + return {} + end, + }) + + view:open() + local loaded = vim.wait(1000, function() + return not view.is_loading + end, 10) + assert.is_true(loaded, "view did not finish loading within 1s") + + -- Pick a non-first file as the active entry; the bug is that the + -- cursor lands on line 1 instead of on this file's row. + local target_file + for _, f in view.files:iter() do + if f.path == "b.txt" then + target_file = f + break + end + end + assert.is_not_nil(target_file) + view.panel:set_cur_file(target_file) + + assert.falsy(view.panel:is_open()) + + -- Toggle the panel open (simulates :DiffviewToggleFiles). + view.panel:toggle(true) + assert.is_true(view.panel:is_open()) + + -- The cursor should land on the active file rather than the top of + -- the buffer. + local item = view.panel:get_item_at_cursor() + assert.equals(target_file, item) + end) + + close_view(view) + cleanup_repo(repo) + if not ok then + error(err) + end + end) + ) end) -- ----------------------------------------------------------------------- diff --git a/lua/diffview/tests/functional/panel_spec.lua b/lua/diffview/tests/functional/panel_spec.lua index 67454680..f4eab5c9 100644 --- a/lua/diffview/tests/functional/panel_spec.lua +++ b/lua/diffview/tests/functional/panel_spec.lua @@ -1054,6 +1054,134 @@ describe("diffview.ui.panel", function() end) end) + describe("FileHistoryPanel show=false lifecycle", function() + local FileHistoryPanel = + require("diffview.scene.views.file_history.file_history_panel").FileHistoryPanel + + -- Build a stub FileHistoryPanel that bypasses real window creation: + -- `is_open`/`buf_loaded` short-circuit `Panel:open`, and `get_config` + -- returns split+auto so the post-open `wincmd =` branch is skipped. + local function make_panel(cur_item, on_highlight) + return setmetatable({ + cur_item = cur_item or {}, + is_open = function() + return true + end, + buf_loaded = function() + return true + end, + get_config = function() + return { type = "split", width = "auto" } + end, + highlight_item = function(_, item) + if on_highlight then + on_highlight(item) + end + end, + }, { __index = FileHistoryPanel }) + end + + it("places the cursor on cur_item[2] when toggled open (#161)", function() + local file = { path = "a.txt" } + local entry = { files = { file } } + local highlighted + + local panel = make_panel({ entry, file }, function(item) + highlighted = item + end) + + panel:open() + eq(file, highlighted) + end) + + it("does not call highlight_item when cur_item is empty", function() + local panel = make_panel({}, function() + error("highlight_item should not be called") + end) + + assert.has_no.errors(function() + panel:open() + end) + end) + + -- Regression: pressing ``/`` while the panel is toggled off + -- routes through `set_file_by_offset` -> `set_entry_fold`, which used to + -- call `utils.set_cursor` against `self.winid` unconditionally. Once the + -- panel was closed the stored winid was stale, so `nvim_win_get_buf` + -- raised "Invalid window id". + it("set_entry_fold does not touch the cursor when the panel is closed", function() + local cursor_calls = 0 + local entry = { + folded = false, + files = {}, + } + + local panel = setmetatable({ + single_file = false, + winid = 999999, + is_open = function() + return false + end, + render = function() end, + redraw = function() end, + components = { + log = { + entries = { + comp = { + some = function(_, _) + cursor_calls = cursor_calls + 1 + end, + }, + }, + }, + }, + }, { __index = FileHistoryPanel }) + + assert.has_no.errors(function() + panel:set_entry_fold(entry, false) + end) + eq(true, entry.folded) + eq(0, cursor_calls) + end) + + -- Companion to the regression above: when the panel *is* open the + -- iteration must still run so the cursor lands on the collapsed entry. + it("set_entry_fold iterates components when the panel is open", function() + local iter_calls = 0 + local entry = { + folded = false, + files = {}, + } + + local panel = setmetatable({ + single_file = false, + winid = 1, + is_open = function() + return true + end, + render = function() end, + redraw = function() end, + components = { + log = { + entries = { + comp = { + some = function(_, _) + iter_calls = iter_calls + 1 + end, + }, + }, + }, + }, + }, { __index = FileHistoryPanel }) + + assert.has_no.errors(function() + panel:set_entry_fold(entry, false) + end) + eq(true, entry.folded) + eq(1, iter_calls) + end) + end) + describe("Panel on_autocmd dispatch", function() local Panel = require("diffview.ui.panel").Panel