From b7fc5eeaca6aa18a2f27742f208a6f44df24841d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:22:52 +0200 Subject: [PATCH 1/6] feat(file-history): step to the next change here --- doc/diffview.txt | 35 +- lua/diffview/actions.lua | 4 + lua/diffview/line_map.lua | 77 +++ lua/diffview/scene/file_entry.lua | 8 + .../views/file_history/file_history_view.lua | 157 +++++ .../scene/views/file_history/listeners.lua | 6 + .../scene/views/standard/standard_view.lua | 148 ++--- .../tests/functional/change_here_spec.lua | 621 ++++++++++++++++++ .../tests/functional/cursor_carry_spec.lua | 79 ++- 9 files changed, 1046 insertions(+), 89 deletions(-) create mode 100644 lua/diffview/line_map.lua create mode 100644 lua/diffview/tests/functional/change_here_spec.lua diff --git a/doc/diffview.txt b/doc/diffview.txt index fe86b860..fc4ec7f1 100644 --- a/doc/diffview.txt +++ b/doc/diffview.txt @@ -2065,6 +2065,26 @@ select_last_entry *diffview-actions-select_last_en Select the commit preceding the subject. +select_next_change_here *diffview-actions-select_next_change_here* + Contexts: `file_history_view`, `file_history_panel` + + Open the next older commit that changes the code under the cursor. + Commits that touch the file somewhere else are skipped, and are not + opened on the way: only the revision's content is read. See + |diffview-cursor-carry| for how the line is followed. + + In a history spanning several files, commits that leave the file alone + are skipped too, and a commit that touches it alongside others is read + at that file rather than at whichever one it lists first. + + If no older commit changes the line, the view does not move. + +select_prev_change_here *diffview-actions-select_prev_change_here* + Contexts: `file_history_view`, `file_history_panel` + + As |diffview-actions-select_next_change_here|, stepping toward the + newer commits. + stage_all *diffview-actions-stage_all* Contexts: `diff_view`, `file_panel` @@ -2152,8 +2172,10 @@ Cursor carry ~ When a step lands on another revision of the file you are already reading, the cursor stays on the same line of code rather than the same line number. Diffview diffs the arriving buffer against the one you are leaving, then maps -your cursor line through the result. This covers the entry and commit actions -alike: +your cursor line through the result. + +This applies to every swap between files, however you reached it -- a row in +the file panel, |diffview-actions-goto_file|, or the entry and commit actions: • |diffview-actions-select_next_entry| • |diffview-actions-select_prev_entry| @@ -2164,6 +2186,13 @@ A line inside a hunk the step introduces has no single counterpart. It lands on that hunk's first line, or on the last surviving line before it when the hunk only deletes. +That same distinction answers whether a commit changed the code you are +reading, which is what |diffview-actions-select_next_change_here| walks on. +Those two actions do not diff the endpoints of their skip: they map the cursor +through every revision they pass, and land on the line that walk arrives at. +Over a run of commits that only shift the code, an endpoint diff has no way to +tell a moved block from a deleted one. + *diffview-unused-actions* Unused actions ~ @@ -2176,6 +2205,8 @@ actions: • |diffview-actions-goto_file_edit| • |diffview-actions-goto_file_edit_close| • |diffview-actions-jumpto_conflict| + • |diffview-actions-select_next_change_here| + • |diffview-actions-select_prev_change_here| • |diffview-actions-view_windo| ============================================================================== diff --git a/lua/diffview/actions.lua b/lua/diffview/actions.lua index 6fdbeb02..9641ec90 100644 --- a/lua/diffview/actions.lua +++ b/lua/diffview/actions.lua @@ -65,6 +65,8 @@ local pl = lazy.access(utils, "path") --[[@as PathLib ]] ---@field select_last_entry fun() ---@field select_next_commit fun() ---@field select_prev_commit fun() +---@field select_next_change_here fun() +---@field select_prev_change_here fun() ---@field stage_all fun() ---@field toggle_files fun() ---@field toggle_flatten_dirs fun() @@ -1265,6 +1267,8 @@ local action_names = { "select_last_entry", "select_next_commit", "select_prev_commit", + "select_next_change_here", + "select_prev_change_here", "stage_all", "toggle_files", "toggle_flatten_dirs", diff --git a/lua/diffview/line_map.lua b/lua/diffview/line_map.lua new file mode 100644 index 00000000..19df600d --- /dev/null +++ b/lua/diffview/line_map.lua @@ -0,0 +1,77 @@ +-- Nvim 0.12 added `vim.text.diff`. `vim.diff` still works, but LuaLS marks +-- it deprecated. Alias once, as `inline_diff` does. +---@diagnostic disable-next-line: deprecated +local diff = vim.diff + +---Following a line of code from one revision of a file into another. +--- +---This is pure text work: it takes lines and hunks and gives back a line, +---touching no window, buffer, or view state. It lives on its own because both +---the cursor carry and the change-here walk need it, and neither should have to +---reach through the other's view class to get at it. +local M = {} + +---Lines as `vim.diff` input. Without the trailing newline `vim.diff` reports +---an addition or deletion at EOF as a modification of the adjacent line. +---`inline_diff` terminates its input the same way. +---@param lines string[] +---@return string +local function lines_text(lines) + return table.concat(lines, "\n") .. "\n" +end + +---Map a line number from the `a` side of a diff onto the `b` side. A line +---following a hunk shifts by that hunk's size delta. A line inside a hunk has +---no single counterpart, so it maps to the hunk's start in `b`. +---@param hunks integer[][] # `vim.diff` "indices" hunks: `{ start_a, count_a, start_b, count_b }`, ascending. +---@param lnum integer +---@return integer +---@return boolean # `true` when `lnum` fell inside a hunk, i.e. the diff rewrote the code it pointed at. +function M.map(hunks, lnum) + local delta = 0 + + for _, hunk in ipairs(hunks) do + local start_a, count_a, start_b, count_b = hunk[1], hunk[2], hunk[3], hunk[4] + + if count_a == 0 then + -- Pure insertion, anchored *after* `start_a`. + if lnum <= start_a then + break + end + delta = delta + count_b + else + local last_a = start_a + count_a - 1 + if lnum < start_a then + break + elseif lnum <= last_a then + -- `start_b` is the hunk's first line in `b`. When the hunk only + -- deletes, it is the last surviving line before the hunk. + return math.max(1, start_b), true + end + delta = delta + count_b - count_a + end + end + + return math.max(1, lnum + delta), false +end + +---Diff two revisions of a file and map `lnum` from the first onto the second. +---Takes lines rather than buffers so the history walk can read a revision +---without opening it. +---@param from_lines string[] +---@param to_lines string[] +---@param lnum integer +---@return integer +---@return boolean # `true` when the two revisions differ over `lnum`. +function M.between(from_lines, to_lines, lnum) + local ok, hunks = + pcall(diff, lines_text(from_lines), lines_text(to_lines), { result_type = "indices" }) + + if not (ok and type(hunks) == "table") then + return lnum, false + end + + return M.map(hunks, lnum) +end + +return M diff --git a/lua/diffview/scene/file_entry.lua b/lua/diffview/scene/file_entry.lua index c9e3df6a..4366dcc3 100644 --- a/lua/diffview/scene/file_entry.lua +++ b/lua/diffview/scene/file_entry.lua @@ -98,6 +98,14 @@ function FileEntry:init(opt) self._extra_owned = opt._extra_owned or {} end +---The `File` this entry's main window shows, without opening the entry. Nil +---while the layout has no main window bound. +---@return vcs.File? +function FileEntry:main_file() + local win = self.layout and self.layout:get_main_win() + return win and win.file or nil +end + ---Destroy owned Files. `force` still drops COMMIT/STAGE/CUSTOM buffers ---unconditionally, but LOCAL buffers stay guarded by `is_buf_in_use`: they ---represent the user's real file buffer and may be visible in windows diff --git a/lua/diffview/scene/views/file_history/file_history_view.lua b/lua/diffview/scene/views/file_history/file_history_view.lua index 079b0aac..ab663796 100644 --- a/lua/diffview/scene/views/file_history/file_history_view.lua +++ b/lua/diffview/scene/views/file_history/file_history_view.lua @@ -14,6 +14,7 @@ local File = lazy.access("diffview.vcs.file", "File") ---@type vcs.File|LazyModu local RevType = lazy.access("diffview.vcs.rev", "RevType") ---@type RevType|LazyModule local StandardView = lazy.access("diffview.scene.views.standard.standard_view", "StandardView") ---@type StandardView|LazyModule local config = lazy.require("diffview.config") ---@module "diffview.config" +local line_map = lazy.require("diffview.line_map") ---@module "diffview.line_map" local utils = lazy.require("diffview.utils") ---@module "diffview.utils" local api = vim.api @@ -209,6 +210,162 @@ function FileHistoryView:prev_item() end end +---The file `entry` carries at `path`, under that name or the one it was renamed +---from, or nil when that commit leaves the path alone and so cannot have changed +---the code under the cursor. `pick_entry_target` +---answers with `entry.files[1]`, which is the wrong file as soon as a commit +---touches more than one, so the walk resolves by path instead. +--- +---Two modes keep their own answer. `pin_local` already resolves by path, via an +---overlay when the commit lacks the file. Single-file history follows one +---logical file across renames, so its lone `FileEntry` is the right target +---whatever its path -- the same reasoning `_resolve_pinned_target` applies. +---@param self FileHistoryView +---@param entry LogEntry +---@param path string +---@return FileEntry? +local function pick_change_here_target(self, entry, path) + if self.pin_local then + return self:pick_entry_target(entry) + end + + if entry.single_file then + return entry.files[1] + end + + for _, f in ipairs(entry.files) do + -- `oldpath` is the other half of the answer. A rename splits the history in + -- two: commits older than it list the file under its old name, newer ones + -- under the new one, and only the renaming commit carries both. Matching on + -- `path` alone loses the file at that seam and then skips every commit past + -- it, which reads to the caller as a history where nothing changes the line. + if f.path == path or f.oldpath == path then + return f + end + end +end + +---Walk the history until a commit changes the code under the cursor, then open +---that commit. Reading each revision's content is enough to decide, so the +---commits in between are never opened. +---@param self FileHistoryView +---@param dir integer # `1` walks toward the older commits, `-1` toward the newer. +FileHistoryView.select_change_here = async.void(function(self, dir) + local cur_entry, cur_file = self.panel.cur_item[1], self.panel.cur_item[2] + if not (cur_entry and cur_file) then + return + end + + local idx = utils.vec_indexof(self.panel.entries, cur_entry) + local win = self.cur_layout and self.cur_layout:get_main_win() + if idx == -1 or not (win and win.id and api.nvim_win_is_valid(win.id)) then + return + end + + local lnum = api.nvim_win_get_cursor(win.id)[1] + local lines = api.nvim_buf_get_lines(api.nvim_win_get_buf(win.id), 0, -1, false) + local found + -- The newer commit of any neighbouring pair owns the difference between + -- them, so walking toward the older commits the answer is the entry read + -- before this one: `lines` is its content, and `lnum` the cursor in it. + local prev + -- The panel appends entries as the history loads, so running out of them + -- means the end of the history only once it has stopped. + local still_loading = false + + while true do + idx = idx + dir + local entry = self.panel.entries[idx] + if not entry then + still_loading = self.panel.updating + break + end + + -- A commit that leaves the path alone cannot have changed the code under + -- the cursor, so it is passed over without reading a revision at all. + local candidate = pick_change_here_target(self, entry, cur_file.path) + + if candidate then + local file = candidate:main_file() + + -- A rename read from the new name toward the old one: the candidate + -- still sits at the path under the cursor, and only `oldpath` says the + -- line is about to mean something else. + local renamed = candidate.oldpath ~= nil and candidate.oldpath ~= candidate.path + + -- Nothing to compare against. The cursor's line means something else + -- under another path -- a rename, reached from either side, since a + -- commit that merely skips the path never becomes a candidate -- and a + -- binary or unreadable revision has no lines at all. Open it and let the + -- reader judge. + if not file or file.binary or candidate.path ~= cur_file.path or renamed then + found = candidate + break + end + + local err, next_lines = await(file.adapter:show(file.path, file.rev)) + if err or not next_lines then + found = candidate + break + end + + local touched + local before = lnum + lnum, touched = line_map.between(lines, next_lines, lnum) + lines = next_lines + + -- Walking toward the newer commits the difference belongs to the + -- candidate itself. Walking toward the older ones it belongs to the + -- entry the walk read before it, and on the first step that entry is + -- the one already open: the reader asked for the next commit that + -- changes the line, not the one they are looking at, so the walk + -- carries on. + local target, target_lnum + if touched then + if dir < 0 then + target, target_lnum = candidate, lnum + elseif prev then + target, target_lnum = prev, before + end + end + + if target then + found = target + + -- The walk mapped the cursor through every revision it passed, so it + -- holds a line the carry cannot re-derive: left to itself the carry + -- diffs the starting revision straight against this one, and across a + -- skip of several commits that single diff has no way to tell a moved + -- body from a deleted one -- it reads the cursor's line as deleted and + -- drops the cursor above the hunk. Hand the walked line over. + -- + -- Only this branch has one. The breaks above leave `lnum` pointing + -- into the text of some earlier revision. + self:set_carry_lnum(target, target_lnum) + break + end + + prev = candidate + end + end + + -- `adapter:show` resumes us in its job's `on_exit`, which is a fast event + -- context. Everything below touches the API. + await(async.scheduler()) + + -- Nothing ahead changes this line, so the reader stays where they are rather + -- than being dropped at the far end of the history. + if not found then + utils.info( + still_loading and "The history is still loading. Nothing read so far changes this line." + or "No further commit changes this line." + ) + return + end + + await(self:set_file(found)) +end) + ---@param self FileHistoryView ---@param file FileEntry ---@param focus? boolean diff --git a/lua/diffview/scene/views/file_history/listeners.lua b/lua/diffview/scene/views/file_history/listeners.lua index 28ae8f45..6a917e4b 100644 --- a/lua/diffview/scene/views/file_history/listeners.lua +++ b/lua/diffview/scene/views/file_history/listeners.lua @@ -161,6 +161,12 @@ return function(view) -- See `select_first_entry` for the pin_local rationale. view:set_file(view:pick_entry_target(next_entry) or next_entry.files[1]) end, + select_next_change_here = function() + view:select_change_here(1) + end, + select_prev_change_here = function() + view:select_change_here(-1) + end, ---Navigate to next file within the current commit. next_entry_in_commit = function() local cur_entry = view.panel.cur_item[1] diff --git a/lua/diffview/scene/views/standard/standard_view.lua b/lua/diffview/scene/views/standard/standard_view.lua index 6ac84d8d..8e28f4ab 100644 --- a/lua/diffview/scene/views/standard/standard_view.lua +++ b/lua/diffview/scene/views/standard/standard_view.lua @@ -9,17 +9,13 @@ local Diff4 = lazy.access("diffview.scene.layouts.diff_4", "Diff4") ---@type Dif local Panel = lazy.access("diffview.ui.panel", "Panel") ---@type Panel|LazyModule local View = lazy.access("diffview.scene.view", "View") ---@type View|LazyModule local config = lazy.require("diffview.config") ---@module "diffview.config" +local line_map = lazy.require("diffview.line_map") ---@module "diffview.line_map" local oop = lazy.require("diffview.oop") ---@module "diffview.oop" local utils = lazy.require("diffview.utils") ---@module "diffview.utils" local api = vim.api local await, pawait = async.await, async.pawait --- Nvim 0.12 added `vim.text.diff`. `vim.diff` still works, but LuaLS marks --- it deprecated. Alias once, as `inline_diff` does. ----@diagnostic disable-next-line: deprecated -local diff = vim.diff - local M = {} ---Predicate matching `DiffView.update_files_impl`'s cancellation guard. @@ -42,6 +38,7 @@ end ---@field layouts table ---@field no_panel? boolean # Per-view `--no-panel` override. When set, takes precedence over the panel's `show` config (`nil` means defer to config). ---@field cursor_map table # Repo-relative path → the cursor and viewport last seen for that path. Consumed the next time the path opens. +---@field package _carry_lnum StandardView.CarryLnum? # A line resolved by a caller that knows it better than a diff between the two revisions can. Consumed once, by the next `restore_main_view`. ---@field package _set_file_in_flight Future? # Active `_set_file` worker; queued callers await this so `await(set_file)` returns only after the latest pending file is opened. ---@field package _set_file_pending FileEntry? # Newest file queued while `_set_file_in_flight` is set; the worker picks it up before terminating. local StandardView = oop.create_class("StandardView", View.__get()) @@ -133,6 +130,10 @@ function StandardView:init(opt) self.emitter:on("post_layout", utils.bind(self.post_layout, self)) end +---@class StandardView.CarryLnum +---@field file FileEntry # The entry the line was resolved against. The line means nothing in any other revision, so a restore for a different entry ignores it. +---@field lnum integer # A line in `file`'s main revision. + ---@class StandardView.CarryState ---@field winview table # A `winsaveview()` dict. ---@field bufnr? integer # The buffer `winview` was captured in. Missing on a state restored from a session sidecar. @@ -172,72 +173,51 @@ local function reveal_cursor_line(winid) end) end +---Rewrite `winview` to sit on `lnum`, keeping the cursor at the screen offset +---it had instead of outside the replayed window. +---@param winview table # A `winsaveview()` dict. +---@param lnum integer +---@return table +local function winview_at(winview, lnum) + local out = vim.deepcopy(winview) + out.lnum = lnum + + if type(out.topline) == "number" then + out.topline = math.max(1, out.topline + (lnum - winview.lnum)) + end + + return out +end + ---Translate `state` into the window's current buffer, then apply it. ---@param winid integer ---@param state StandardView.CarryState +---@param lnum integer? # A line in the window's current buffer, already resolved by the caller. Given one, nothing is diffed and the cursor goes there. ---@return boolean # `true` when `winrestview` ran without error. -local function apply_winview(winid, state) - local from_buf = state.bufnr - -- Neovim can hand a wiped buffer's handle to another file. Diffing against - -- that file would place the cursor on an unrelated line. - if - from_buf - and not (api.nvim_buf_is_valid(from_buf) and api.nvim_buf_get_name(from_buf) == state.bufname) - then - from_buf = nil - end +local function apply_winview(winid, state, lnum) + local target + + if lnum then + target = winview_at(state.winview, lnum) + else + local from_buf = state.bufnr + -- Neovim can hand a wiped buffer's handle to another file. Diffing against + -- that file would place the cursor on an unrelated line. + if + from_buf + and not (api.nvim_buf_is_valid(from_buf) and api.nvim_buf_get_name(from_buf) == state.bufname) + then + from_buf = nil + end - local target = - StandardView._translate_winview(state.winview, from_buf, api.nvim_win_get_buf(winid)) + target = StandardView._translate_winview(state.winview, from_buf, api.nvim_win_get_buf(winid)) + end return (pcall(api.nvim_win_call, winid, function() vim.fn.winrestview(target) end)) end ----A buffer's contents as `vim.diff` input. Without the trailing newline ----`vim.diff` reports an addition or deletion at EOF as a modification of the ----adjacent line. `inline_diff` terminates its input the same way. ----@param bufnr integer ----@return string -local function buf_text(bufnr) - return table.concat(api.nvim_buf_get_lines(bufnr, 0, -1, false), "\n") .. "\n" -end - ----Map a line number from the `a` side of a diff onto the `b` side. A line ----following a hunk shifts by that hunk's size delta. A line inside a hunk has ----no single counterpart, so it maps to the hunk's start in `b`. ----@param hunks integer[][] # `vim.diff` "indices" hunks: `{ start_a, count_a, start_b, count_b }`, ascending. ----@param lnum integer ----@return integer -function StandardView._map_lnum(hunks, lnum) - local delta = 0 - - for _, hunk in ipairs(hunks) do - local start_a, count_a, start_b, count_b = hunk[1], hunk[2], hunk[3], hunk[4] - - if count_a == 0 then - -- Pure insertion, anchored *after* `start_a`. - if lnum <= start_a then - break - end - delta = delta + count_b - else - local last_a = start_a + count_a - 1 - if lnum < start_a then - break - elseif lnum <= last_a then - -- `start_b` is the hunk's first line in `b`. When the hunk only - -- deletes, it is the last surviving line before the hunk. - return math.max(1, start_b) - end - delta = delta + count_b - count_a - end - end - - return math.max(1, lnum + delta) -end - ---Rewrite a `winsaveview` dict so its cursor points at the same code in ---`to_buf` as it did in `from_buf`. Returns `winview` unchanged whenever the ---translation can't be computed, which is the untranslated behaviour. @@ -253,28 +233,17 @@ function StandardView._translate_winview(winview, from_buf, to_buf) return winview end - local ok, hunks = pcall(diff, buf_text(from_buf), buf_text(to_buf), { result_type = "indices" }) - - if not (ok and type(hunks) == "table") then - return winview - end - - local lnum = StandardView._map_lnum(hunks, winview.lnum) + local lnum = line_map.between( + api.nvim_buf_get_lines(from_buf, 0, -1, false), + api.nvim_buf_get_lines(to_buf, 0, -1, false), + winview.lnum + ) if lnum == winview.lnum then return winview end - local out = vim.deepcopy(winview) - out.lnum = lnum - - if type(out.topline) == "number" then - -- Keeps the cursor at its old screen offset instead of outside the - -- replayed window. - out.topline = math.max(1, out.topline + (lnum - winview.lnum)) - end - - return out + return winview_at(winview, lnum) end ---Snapshot the main diff window's cursor + viewport into @@ -300,15 +269,33 @@ function StandardView:snapshot_main_view(path, alias) end end +---Hand the next `restore_main_view` a line the caller resolved itself, in +---`file`'s revision. See `StandardView.CarryLnum`. +---@param file FileEntry +---@param lnum integer +function StandardView:set_carry_lnum(file, lnum) + self._carry_lnum = { file = file, lnum = lnum } +end + ---Pop and apply the saved view state for `path`. Diffing the snapshotted ---buffer against the arriving one moves the cursor line with its code, so a ---step lands on the same line of code rather than the same line number. ---A successful apply drops the entry; the next swap away from `path` puts a ---fresh one back. A failed apply (no main window, or `winrestview` errors) ---keeps the entry for a later attempt. +--- +---A caller that resolved the line itself, via `_carry_lnum`, overrides that +---diff. It is keyed on the arriving `FileEntry` rather than on `path`, because +---the line only means anything in the revision it was resolved against, and a +---superseded `set_file` can leave one behind for a later open of the same path. ---@param path string repo-relative file path. ---@return boolean # `true` when a saved state was applied successfully. function StandardView:restore_main_view(path) + -- One-shot, popped before any early return: a line held over past the open it + -- was resolved for is a line resolved against the wrong revision. + local carried = self._carry_lnum + self._carry_lnum = nil + local target = self.cursor_map[path] if target == nil then return false @@ -323,9 +310,14 @@ function StandardView:restore_main_view(path) return false end + local lnum + if carried and carried.file == self.cur_entry then + lnum = carried.lnum + end + -- We place only the main window. The layout's other windows follow it -- through `'cursorbind'`. - local ok = apply_winview(win.id, target) + local ok = apply_winview(win.id, target, lnum) if ok then reveal_cursor_line(win.id) diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua new file mode 100644 index 00000000..dee72ee9 --- /dev/null +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -0,0 +1,621 @@ +local config = require("diffview.config") +local helpers = require("diffview.tests.helpers") +local lib = require("diffview.lib") + +local api = vim.api +local eq = helpers.eq +local commit = helpers.commit +local line_at = helpers.line_at +local body = helpers.body +local write = helpers.write + +-- Only c3 edits the body. The commit after it prepends, which moves the body's +-- line numbers without changing a line of it, and so does the one before. c3b +-- leaves `file.txt` alone, so it shows up only in the unfiltered history: +-- +-- c1 file.txt body 1..20 (20 lines) +-- c2 file.txt head 1..30 + body (50) +-- c3 file.txt body 5 rewritten (50) +-- c3b other.txt new file +-- c4 file.txt mid 1..5 + head + body (55) +local function make_repo() + local repo = helpers.init_repo() + local lines = body("body", 20) + write(repo, "file.txt", lines) + commit(repo, "c1") + + lines = vim.list_extend(body("head", 30), lines) + write(repo, "file.txt", lines) + commit(repo, "c2") + + lines[35] = "body 5 rewritten" + write(repo, "file.txt", lines) + commit(repo, "c3") + + write(repo, "other.txt", body("other", 8)) + commit(repo, "c3b") + + lines = vim.list_extend(body("mid", 5), lines) + write(repo, "file.txt", lines) + commit(repo, "c4") + + return repo +end + +describe("select_change_here", function() + local repo, cwd, view, original_config + + before_each(function() + original_config = vim.deepcopy(config.get_config()) + config.get_config().use_icons = false + repo = make_repo() + cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo)) + end) + + after_each(function() + vim.cmd("cd " .. vim.fn.fnameescape(cwd)) + helpers.close_view(view) + view = nil + helpers.cleanup_repo(repo) + config.setup(original_config) + end) + + local function main_win() + return view.cur_layout:get_main_win().id + end + + ---Open the history on c4 and put the cursor on `text`. + ---@param text string + ---@param paths string[]? # Path filter. Defaults to `file.txt` only. + ---@param n_entries integer? # Entries that filter yields. Defaults to 4. + ---@return integer main_win + local function open_on(text, paths, n_entries) + view = lib.file_history(nil, paths or { "file.txt" }) + assert.is_not_nil(view) + view:open() + + assert.is_true( + vim.wait(10000, function() + return view.ready and #view.panel.entries >= (n_entries or 4) and view.cur_layout ~= nil + end), + "view never became ready" + ) + assert.is_true( + vim.wait(10000, function() + return api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) >= 55 + end), + "the b-side buffer never loaded" + ) + + local main = main_win() + local lines = api.nvim_buf_get_lines(api.nvim_win_get_buf(main), 0, -1, false) + local row = assert(vim.fn.index(lines, text) + 1 > 0 and vim.fn.index(lines, text) + 1) + + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { row, 0 }) + eq(text, line_at(main)) + + return main + end + + ---@param idx integer # Panel entry index the walk must come to rest on. + ---@param lines integer # Line count of that commit's buffer, so the wait also + ---covers the swap the panel move was only the start of. + local function wait_for_entry(idx, lines) + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[idx] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == lines + end), + ("the walk never came to rest on entry %d"):format(idx) + ) + vim.wait(200) + end + + it("passes a commit that only shifts the line and opens the one that rewrote it", function() + open_on("body 5 rewritten") + + view:select_change_here(1) + + -- c4 only prepends the `mid` block, so the line reads the same in c3 as in + -- c4. c3 rewrote it, so that is where the walk stops: c2 merely shifts the + -- line again and is not a commit that changes it. + wait_for_entry(2, 50) + eq("body 5 rewritten", line_at(main_win())) + end) + + it("stays put when nothing older changes the line", function() + local main = open_on("body 12") + + view:select_change_here(1) + vim.wait(3000, function() + return view.panel.cur_item[1] ~= view.panel.entries[1] + end) + + eq(view.panel.entries[1], view.panel.cur_item[1]) + eq("body 12", line_at(main)) + end) + + it("walks toward the newer commits", function() + open_on("body 5 rewritten") + + -- c2 is the last commit before the rewrite, so `body 5` still holds its + -- original text there. + view:set_file(view.panel.entries[3].files[1]) + wait_for_entry(3, 50) + local main = main_win() + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { 35, 0 }) + eq("body 5", line_at(main)) + + view:select_change_here(-1) + + wait_for_entry(2, 50) + eq("body 5 rewritten", line_at(main_win())) + end) + + it("passes a commit that leaves the file alone", function() + -- c3b touches `other.txt` only, so it cannot have changed `body 12` and the + -- walk must not come to rest on it. Nothing older changes the line either, + -- so the reader stays on c4. + local main = open_on("body 12", {}, 5) + + view:select_change_here(1) + vim.wait(3000, function() + return view.panel.cur_item[1] ~= view.panel.entries[1] + end) + + eq(view.panel.entries[1], view.panel.cur_item[1]) + eq("file.txt", view.panel.cur_item[2].path) + eq("body 12", line_at(main)) + end) + + it("says the history is still loading rather than claiming nothing changes the line", function() + local main = open_on("body 12") + local utils = require("diffview.utils") + local original_info, message = utils.info, nil + utils.info = function(msg) + message = msg + end + -- The panel appends entries as the log streams in; mid-load the end of the + -- list is the frontier, not the end of the history. + view.panel.updating = true + + view:select_change_here(1) + + local got = vim.wait(10000, function() + return message ~= nil + end) + utils.info = original_info + view.panel.updating = false + + assert.is_true(got, "the walk never reported anything") + assert.is_truthy(message:match("still loading")) + eq("body 12", line_at(main)) + end) + + it("runs the walk through the registered action", function() + open_on("body 5 rewritten") + + require("diffview.actions").select_next_change_here() + + wait_for_entry(2, 50) + eq("body 5 rewritten", line_at(main_win())) + end) +end) + +-- A history whose commits carry more than one file. `a_other.txt` sorts before +-- `file.txt`, so it is `entry.files[1]` wherever both appear: a walk that reads +-- the entry's first file rather than the one under the cursor lands here. +-- +-- m1 a_other.txt + file.txt body 1..20 (20 lines) +-- m2 a_other.txt + file.txt body 5 rewritten (20) +-- m3 a_other.txt file.txt untouched +-- m4 file.txt head 1..10 + body (30) +local function make_multi_repo() + local repo = helpers.init_repo() + local lines = body("body", 20) + + write(repo, "a_other.txt", body("other", 8)) + write(repo, "file.txt", lines) + commit(repo, "m1") + + lines[5] = "body 5 rewritten" + write(repo, "a_other.txt", body("other", 12)) + write(repo, "file.txt", lines) + commit(repo, "m2") + + write(repo, "a_other.txt", body("other", 16)) + commit(repo, "m3") + + lines = vim.list_extend(body("head", 10), lines) + write(repo, "file.txt", lines) + commit(repo, "m4") + + return repo +end + +describe("select_change_here across multi-file commits", function() + local repo, cwd, view, original_config + + before_each(function() + original_config = vim.deepcopy(config.get_config()) + config.get_config().use_icons = false + repo = make_multi_repo() + cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo)) + end) + + after_each(function() + vim.cmd("cd " .. vim.fn.fnameescape(cwd)) + helpers.close_view(view) + view = nil + helpers.cleanup_repo(repo) + config.setup(original_config) + end) + + local function main_win() + return view.cur_layout:get_main_win().id + end + + ---@param idx integer # Panel entry index. + ---@return FileEntry # That entry's `file.txt`. + local function file_txt_in(idx) + for _, f in ipairs(view.panel.entries[idx].files) do + if f.path == "file.txt" then + return f + end + end + error("entry " .. idx .. " carries no file.txt") + end + + ---Open the unfiltered history on m4 with the cursor on `body 5 rewritten`. + ---@return integer main_win + local function open_on_head() + view = lib.file_history(nil, {}) + assert.is_not_nil(view) + view:open() + + assert.is_true( + vim.wait(10000, function() + return view.ready and #view.panel.entries >= 4 and view.cur_layout ~= nil + end), + "view never became ready" + ) + assert.is_true( + vim.wait(10000, function() + return api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) >= 30 + end), + "the b-side buffer never loaded" + ) + + local main = main_win() + eq("file.txt", view.panel.cur_item[2].path) + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { 15, 0 }) + eq("body 5 rewritten", line_at(main)) + + return main + end + + it("reads the file under the cursor, not the commit's first file", function() + open_on_head() + + view:select_change_here(1) + + -- m3 carries no `file.txt` and is passed over. m4 only prepends, so the + -- line reads the same in m2, and reading m1 is what tells the walk that m2 + -- is where the text changed. Both m1 and m2 list `a_other.txt` first, which + -- is the file a first-file walk would have opened instead. + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[3] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 20 + end), + "the walk never came to rest on m2" + ) + + eq("file.txt", view.panel.cur_item[2].path) + eq("body 5 rewritten", line_at(main_win())) + end) + + it("finds the same file walking toward the newer commits", function() + open_on_head() + + -- m1 is the last commit before the rewrite. The panel moves before the + -- buffer swap finishes, and the walk reads the cursor and the buffer it + -- starts from, so wait for m1 to be fully open. + view:set_file(file_txt_in(4)) + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[4] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 20 + end), + "m1 never opened" + ) + vim.wait(200) + + local main = main_win() + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { 5, 0 }) + eq("body 5", line_at(main)) + + view:select_change_here(-1) + + -- Entries run newest first, so m2 is `entries[3]`. It lists `a_other.txt` + -- first, and the walk has to come back to `file.txt` regardless. + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[3] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 20 + end), + "the walk never reached m2" + ) + + eq("file.txt", view.panel.cur_item[2].path) + eq("body 5 rewritten", line_at(main_win())) + end) +end) + +---A four-line function block: signature, body, `end`, blank. +---@param name string +---@param ret integer? +---@return string[] +local function fn(name, ret) + return { ("local function %s()"):format(name), (" return %d"):format(ret or 0), "end", "" } +end + +---@param ... string[] +---@return string[] +local function blocks(...) + local out = {} + for _, b in ipairs({ ... }) do + vim.list_extend(out, b) + end + return out +end + +-- Every function body is the same text, so a diff between two distant +-- revisions has more than one honest way to line them up. The walk has no such +-- trouble: it maps the cursor hop by hop, and each hop's diff is small enough +-- to have only one answer. +-- +-- r1 fn_a fn_b fn_c fn_d (16 lines) +-- r2 fn_c returns 3 (16) +-- r3 fn_b dropped (12) +-- r4 fn_y fn_z prepended (20) +-- r5 fn_e appended (24) +local function make_repeat_repo() + local repo = helpers.init_repo() + local a, b, c, d = fn("fn_a"), fn("fn_b"), fn("fn_c"), fn("fn_d") + + write(repo, "repeat.txt", blocks(a, b, c, d)) + commit(repo, "r1") + + c = fn("fn_c", 3) + write(repo, "repeat.txt", blocks(a, b, c, d)) + commit(repo, "r2") + + write(repo, "repeat.txt", blocks(a, c, d)) + commit(repo, "r3") + + write(repo, "repeat.txt", blocks(fn("fn_y"), fn("fn_z"), a, c, d)) + commit(repo, "r4") + + write(repo, "repeat.txt", blocks(fn("fn_y"), fn("fn_z"), a, c, d, fn("fn_e"))) + commit(repo, "r5") + + return repo +end + +describe("select_change_here across identical bodies", function() + local repo, cwd, view, original_config + + before_each(function() + original_config = vim.deepcopy(config.get_config()) + config.get_config().use_icons = false + repo = make_repeat_repo() + cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo)) + end) + + after_each(function() + vim.cmd("cd " .. vim.fn.fnameescape(cwd)) + helpers.close_view(view) + view = nil + helpers.cleanup_repo(repo) + config.setup(original_config) + end) + + local function main_win() + return view.cur_layout:get_main_win().id + end + + it("lands on the line the walk mapped, not the one an end-to-end diff finds", function() + view = lib.file_history(nil, { "repeat.txt" }) + assert.is_not_nil(view) + view:open() + + assert.is_true( + vim.wait(10000, function() + return view.ready and #view.panel.entries >= 5 and view.cur_layout ~= nil + end), + "view never became ready" + ) + assert.is_true( + vim.wait(10000, function() + return api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 24 + end), + "the b-side buffer never loaded" + ) + + local main = main_win() + api.nvim_set_current_win(main) + -- `fn_c`'s body in r5, the only line in the file that reads `return 3`. + api.nvim_win_set_cursor(main, { 14, 0 }) + eq(" return 3", line_at(main)) + + view:select_change_here(1) + + -- r4 and r3 only shift the line. r2 is where `fn_c` started returning 3, + -- which reading r1 is what reveals, so the walk comes to rest on r2. + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[4] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 16 + end), + "the walk never came to rest on r2" + ) + vim.wait(200) + + -- Line 10 is `fn_c`'s body. Diffing r5 against r2 in one step instead has + -- more than one honest alignment of the identical bodies and leaves the + -- cursor off the line the walk followed. + eq(10, api.nvim_win_get_cursor(main_win())[1]) + eq(" return 3", line_at(main_win())) + end) +end) + +-- A multi-file history in which the file under the cursor is renamed partway +-- through. Commits older than the rename list the old path and newer ones the +-- new path, so a walk that matches on one name alone goes blind at the rename +-- and runs off the end of the history. +-- +-- n1 a_other.txt + keep.txt body 1..20 (20 lines) +-- n2 a_other.txt + keep.txt body 5 rewritten (20) +-- n3 a_other.txt + keep.txt -> moved.txt pure rename (20) +-- n4 a_other.txt + moved.txt head 1..10 (30) +local function make_rename_repo() + local repo = helpers.init_repo() + local lines = body("body", 20) + + write(repo, "a_other.txt", body("other", 8)) + write(repo, "keep.txt", lines) + commit(repo, "n1") + + lines[5] = "body 5 rewritten" + write(repo, "a_other.txt", body("other", 12)) + write(repo, "keep.txt", lines) + commit(repo, "n2") + + helpers.run({ "git", "mv", "keep.txt", "moved.txt" }, repo) + write(repo, "a_other.txt", body("other", 16)) + commit(repo, "n3") + + lines = vim.list_extend(body("head", 10), lines) + write(repo, "moved.txt", lines) + commit(repo, "n4") + + return repo +end + +describe("select_change_here across a rename", function() + local repo, cwd, view, original_config + + before_each(function() + original_config = vim.deepcopy(config.get_config()) + config.get_config().use_icons = false + repo = make_rename_repo() + cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo)) + end) + + after_each(function() + vim.cmd("cd " .. vim.fn.fnameescape(cwd)) + helpers.close_view(view) + view = nil + helpers.cleanup_repo(repo) + config.setup(original_config) + end) + + local function main_win() + return view.cur_layout:get_main_win().id + end + + ---Open the unfiltered history and wait for n4. + local function open_history() + view = lib.file_history(nil, {}) + assert.is_not_nil(view) + view:open() + + assert.is_true( + vim.wait(10000, function() + return view.ready and #view.panel.entries >= 4 and view.cur_layout ~= nil + end), + "view never became ready" + ) + assert.is_true( + vim.wait(10000, function() + return api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 30 + end), + "the b-side buffer never loaded" + ) + end + + ---@param idx integer + ---@param path string + ---@param lines integer + local function wait_for(idx, path, lines) + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[idx] + and view.panel.cur_item[2].path == path + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == lines + end), + ("the walk never came to rest on entry %d at %s"):format(idx, path) + ) + vim.wait(200) + end + + ---Open `path` in `entries[idx]` and put the cursor on `text`. + ---@return integer main_win + local function go_to(idx, path, text, lines) + local target + for _, f in ipairs(view.panel.entries[idx].files) do + if f.path == path then + target = f + end + end + view:set_file(assert(target)) + wait_for(idx, path, lines) + + local main = main_win() + local buf = api.nvim_buf_get_lines(api.nvim_win_get_buf(main), 0, -1, false) + local row = vim.fn.index(buf, text) + 1 + assert.is_true(row > 0, ("%q is not in %s"):format(text, path)) + + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { row, 0 }) + eq(text, line_at(main)) + + return main + end + + it("stops on the rename walking toward the older commits", function() + open_history() + -- n4's prepend is the only thing between the cursor and the rename, and it + -- leaves the line's text alone. + go_to(1, "moved.txt", "body 5 rewritten", 30) + + view:select_change_here(1) + + -- n3 renames the file without touching a line of it. The cursor's line + -- means something else under another path, so the walk opens n3 rather + -- than reading past it -- and everything older lists `keep.txt`, which a + -- walk matching on `moved.txt` alone would skip all the way off the end. + wait_for(2, "moved.txt", 20) + eq("body 5 rewritten", line_at(main_win())) + end) + + it("stops on the rename walking toward the newer commits", function() + open_history() + go_to(3, "keep.txt", "body 12", 20) + + view:select_change_here(-1) + + -- Same rename from the other side: n3 lists the file under its new name, + -- so only `oldpath` connects it to the `keep.txt` under the cursor. + wait_for(2, "moved.txt", 20) + end) +end) diff --git a/lua/diffview/tests/functional/cursor_carry_spec.lua b/lua/diffview/tests/functional/cursor_carry_spec.lua index eb350da3..3a28366f 100644 --- a/lua/diffview/tests/functional/cursor_carry_spec.lua +++ b/lua/diffview/tests/functional/cursor_carry_spec.lua @@ -1,5 +1,6 @@ local helpers = require("diffview.tests.helpers") local StandardView = require("diffview.scene.views.standard.standard_view").StandardView +local line_map = require("diffview.line_map") local api = vim.api local eq = helpers.eq @@ -45,37 +46,97 @@ local function make_view(a_win, b_win) }, { __index = StandardView }) end -describe("diffview.standard_view _map_lnum", function() +describe("diffview.line_map map", function() it("leaves a line preceding every hunk untouched", function() - eq(3, StandardView._map_lnum({ { 10, 0, 11, 5 } }, 3)) + eq(3, line_map.map({ { 10, 0, 11, 5 } }, 3)) end) it("shifts a line following an insertion by the inserted count", function() - eq(35, StandardView._map_lnum({ { 0, 0, 1, 30 } }, 5)) + eq(35, line_map.map({ { 0, 0, 1, 30 } }, 5)) end) it("shifts a line following a deletion back by the deleted count", function() - eq(5, StandardView._map_lnum({ { 1, 30, 0, 0 } }, 35)) + eq(5, line_map.map({ { 1, 30, 0, 0 } }, 35)) end) it("maps a line inside a changed hunk to the hunk start on the new side", function() - eq(12, StandardView._map_lnum({ { 10, 4, 12, 6 } }, 11)) + eq(12, line_map.map({ { 10, 4, 12, 6 } }, 11)) end) it("maps a line inside a deleted hunk to the last surviving line before it", function() - eq(4, StandardView._map_lnum({ { 5, 3, 4, 0 } }, 6)) + eq(4, line_map.map({ { 5, 3, 4, 0 } }, 6)) end) it("accumulates deltas across several preceding hunks", function() - eq(14, StandardView._map_lnum({ { 0, 0, 1, 3 }, { 5, 4, 9, 2 } }, 13)) + eq(14, line_map.map({ { 0, 0, 1, 3 }, { 5, 4, 9, 2 } }, 13)) + end) + + it("reports a line inside a changed hunk as touched", function() + local _, touched = line_map.map({ { 10, 4, 12, 6 } }, 11) + assert.is_true(touched) + end) + + it("reports a line merely shifted by a hunk as untouched", function() + local _, touched = line_map.map({ { 0, 0, 1, 30 } }, 5) + assert.is_false(touched) + end) + + it("reports a line inside a deleted hunk as touched", function() + local _, touched = line_map.map({ { 5, 3, 4, 0 } }, 6) + assert.is_true(touched) + end) + + it("reports a line preceding every hunk as untouched", function() + local _, touched = line_map.map({ { 10, 0, 11, 5 } }, 3) + assert.is_false(touched) end) it("returns the line unchanged for an empty diff", function() - eq(7, StandardView._map_lnum({}, 7)) + eq(7, line_map.map({}, 7)) end) it("never returns a line below 1", function() - eq(1, StandardView._map_lnum({ { 1, 3, 0, 0 } }, 2)) + eq(1, line_map.map({ { 1, 3, 0, 0 } }, 2)) + end) +end) + +describe("diffview.line_map between", function() + it("reports a line the next revision rewrote as touched", function() + local from = body("body", 20) + local to = body("body", 20) + to[5] = "body 5 rewritten" + + local lnum, touched = line_map.between(from, to, 5) + + eq(5, lnum) + assert.is_true(touched) + end) + + it("reports a line the next revision only shifted as untouched", function() + local from = body("body", 20) + local to = vim.list_extend(body("head", 30), body("body", 20)) + + local lnum, touched = line_map.between(from, to, 5) + + eq(35, lnum) + assert.is_false(touched) + end) + + it("reports a line the next revision deleted as touched", function() + local from = body("body", 20) + local to = vim.list_slice(body("body", 20), 1, 10) + + local lnum, touched = line_map.between(from, to, 15) + + eq(10, lnum) + assert.is_true(touched) + end) + + it("reports every line of an unchanged revision as untouched", function() + local lnum, touched = line_map.between(body("body", 20), body("body", 20), 12) + + eq(12, lnum) + assert.is_false(touched) end) end) From 6497dcfde9d0a13108c37d68800b3e0dbd8bffc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Fri, 11 Sep 2026 03:48:57 +0200 Subject: [PATCH 2/6] feat(file-history): step across renames Stepping to the next change follows the file under its former name once the walk passes the commit that renamed it. A pure rename touches no line, so it is passed over like any other commit that leaves the line alone; a rename that also edits the line is where the walk stops. Before, the walk opened the renaming commit without a mapped line, because older entries list the file under a name it no longer matched on. Git reports a rename only when both names lie inside the history's pathspec, so wherever a rename shows up, the older entries are in the list too, and nothing is left to guard against. --- .../views/file_history/file_history_view.lua | 26 ++++---- .../tests/functional/change_here_spec.lua | 60 ++++++++++++------- 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/lua/diffview/scene/views/file_history/file_history_view.lua b/lua/diffview/scene/views/file_history/file_history_view.lua index ab663796..bdebbd6b 100644 --- a/lua/diffview/scene/views/file_history/file_history_view.lua +++ b/lua/diffview/scene/views/file_history/file_history_view.lua @@ -272,6 +272,8 @@ FileHistoryView.select_change_here = async.void(function(self, dir) -- The panel appends entries as the history loads, so running out of them -- means the end of the history only once it has stopped. local still_loading = false + -- The name the file goes by in the entries ahead. A rename changes it. + local path = cur_file.path while true do idx = idx + dir @@ -283,22 +285,14 @@ FileHistoryView.select_change_here = async.void(function(self, dir) -- A commit that leaves the path alone cannot have changed the code under -- the cursor, so it is passed over without reading a revision at all. - local candidate = pick_change_here_target(self, entry, cur_file.path) + local candidate = pick_change_here_target(self, entry, path) if candidate then local file = candidate:main_file() - -- A rename read from the new name toward the old one: the candidate - -- still sits at the path under the cursor, and only `oldpath` says the - -- line is about to mean something else. - local renamed = candidate.oldpath ~= nil and candidate.oldpath ~= candidate.path - - -- Nothing to compare against. The cursor's line means something else - -- under another path -- a rename, reached from either side, since a - -- commit that merely skips the path never becomes a candidate -- and a - -- binary or unreadable revision has no lines at all. Open it and let the - -- reader judge. - if not file or file.binary or candidate.path ~= cur_file.path or renamed then + -- Nothing to compare against: a binary or unreadable revision has no + -- lines. Open it and let the reader judge. + if not file or file.binary then found = candidate break end @@ -314,6 +308,14 @@ FileHistoryView.select_change_here = async.void(function(self, dir) lnum, touched = line_map.between(lines, next_lines, lnum) lines = next_lines + -- A rename lists the file under both names, and the entries beyond it in + -- this direction use only one of them: the older ones the old name, the + -- newer ones the new one. The content was read under the name the + -- candidate itself uses, so the rename is judged like any other commit. + if candidate.oldpath and candidate.oldpath ~= candidate.path then + path = dir > 0 and candidate.oldpath or candidate.path + end + -- Walking toward the newer commits the difference belongs to the -- candidate itself. Walking toward the older ones it belongs to the -- entry the walk read before it, and on the first step that entry is diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua index dee72ee9..6761a4f3 100644 --- a/lua/diffview/tests/functional/change_here_spec.lua +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -477,15 +477,17 @@ describe("select_change_here across identical bodies", function() end) end) --- A multi-file history in which the file under the cursor is renamed partway --- through. Commits older than the rename list the old path and newer ones the --- new path, so a walk that matches on one name alone goes blind at the rename --- and runs off the end of the history. +-- A history in which the file under the cursor is renamed partway through. +-- Commits older than the rename list the old path and newer ones the new path, +-- so a walk that matches on one name alone goes blind at the rename and runs +-- off the end of the history. The rename itself touches no line, so a walk +-- that crosses it has to pass it over like any other commit that leaves the +-- line alone. -- --- n1 a_other.txt + keep.txt body 1..20 (20 lines) --- n2 a_other.txt + keep.txt body 5 rewritten (20) --- n3 a_other.txt + keep.txt -> moved.txt pure rename (20) --- n4 a_other.txt + moved.txt head 1..10 (30) +-- n1 a_other.txt + keep.txt body 1..20 (20 lines) +-- n2 a_other.txt + keep.txt body 5 rewritten (20) +-- n3 a_other.txt + keep.txt -> moved.txt pure rename (20) +-- n4 a_other.txt + moved.txt head 1..10, body 12 rewritten (30) local function make_rename_repo() local repo = helpers.init_repo() local lines = body("body", 20) @@ -504,6 +506,7 @@ local function make_rename_repo() commit(repo, "n3") lines = vim.list_extend(body("head", 10), lines) + lines[22] = "body 12 rewritten" write(repo, "moved.txt", lines) commit(repo, "n4") @@ -533,9 +536,10 @@ describe("select_change_here across a rename", function() return view.cur_layout:get_main_win().id end - ---Open the unfiltered history and wait for n4. - local function open_history() - view = lib.file_history(nil, {}) + ---Open the history and wait for n4. + ---@param paths string[]? # Path filter. Defaults to the whole repo. + local function open_history(paths) + view = lib.file_history(nil, paths or {}) assert.is_not_nil(view) view:open() @@ -592,7 +596,7 @@ describe("select_change_here across a rename", function() return main end - it("stops on the rename walking toward the older commits", function() + it("crosses the rename walking toward the older commits", function() open_history() -- n4's prepend is the only thing between the cursor and the rename, and it -- leaves the line's text alone. @@ -600,22 +604,36 @@ describe("select_change_here across a rename", function() view:select_change_here(1) - -- n3 renames the file without touching a line of it. The cursor's line - -- means something else under another path, so the walk opens n3 rather - -- than reading past it -- and everything older lists `keep.txt`, which a - -- walk matching on `moved.txt` alone would skip all the way off the end. - wait_for(2, "moved.txt", 20) + -- n3 renames the file without touching a line of it, so it is passed over + -- like any other commit that leaves the line alone. Everything older lists + -- `keep.txt`, which the walk has to follow the file to: n2 is where the + -- line was rewritten, and reading n1 is what tells. + wait_for(3, "keep.txt", 20) eq("body 5 rewritten", line_at(main_win())) end) - it("stops on the rename walking toward the newer commits", function() + it("crosses the rename walking toward the newer commits", function() open_history() - go_to(3, "keep.txt", "body 12", 20) + go_to(4, "keep.txt", "body 12", 20) view:select_change_here(-1) -- Same rename from the other side: n3 lists the file under its new name, - -- so only `oldpath` connects it to the `keep.txt` under the cursor. - wait_for(2, "moved.txt", 20) + -- so only `oldpath` connects it to the `keep.txt` under the cursor, and + -- past it the file goes by `moved.txt`. n4 is where the line changed. + wait_for(1, "moved.txt", 30) + eq("body 12 rewritten", line_at(main_win())) + end) + + it("crosses the rename in a single-file history", function() + -- `--follow` lists n2 and n1 under `keep.txt` even though the history was + -- asked for `moved.txt`. + open_history({ "moved.txt" }) + go_to(1, "moved.txt", "body 5 rewritten", 30) + + view:select_change_here(1) + + wait_for(3, "keep.txt", 20) + eq("body 5 rewritten", line_at(main_win())) end) end) From 92a0527b9c24e934b26ab9beb505e587825f90ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Sat, 12 Sep 2026 10:23:08 +0200 Subject: [PATCH 3/6] fixup! feat(file-history): step across renames --- .../views/file_history/file_history_view.lua | 7 +- .../tests/functional/change_here_spec.lua | 100 ++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/lua/diffview/scene/views/file_history/file_history_view.lua b/lua/diffview/scene/views/file_history/file_history_view.lua index bdebbd6b..5a7eb00c 100644 --- a/lua/diffview/scene/views/file_history/file_history_view.lua +++ b/lua/diffview/scene/views/file_history/file_history_view.lua @@ -239,7 +239,9 @@ local function pick_change_here_target(self, entry, path) -- under the new one, and only the renaming commit carries both. Matching on -- `path` alone loses the file at that seam and then skips every commit past -- it, which reads to the caller as a history where nothing changes the line. - if f.path == path or f.oldpath == path then + -- `oldpath` also names the source of a copy (status `C`), which is another + -- file, so that pairing is not followed. + if f.path == path or (f.oldpath == path and f.status ~= "C") then return f end end @@ -298,6 +300,7 @@ FileHistoryView.select_change_here = async.void(function(self, dir) end local err, next_lines = await(file.adapter:show(file.path, file.rev)) + if err or not next_lines then found = candidate break @@ -312,7 +315,7 @@ FileHistoryView.select_change_here = async.void(function(self, dir) -- this direction use only one of them: the older ones the old name, the -- newer ones the new one. The content was read under the name the -- candidate itself uses, so the rename is judged like any other commit. - if candidate.oldpath and candidate.oldpath ~= candidate.path then + if candidate.oldpath and candidate.oldpath ~= candidate.path and candidate.status ~= "C" then path = dir > 0 and candidate.oldpath or candidate.path end diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua index 6761a4f3..8f0a95f3 100644 --- a/lua/diffview/tests/functional/change_here_spec.lua +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -637,3 +637,103 @@ describe("select_change_here across a rename", function() eq("body 5 rewritten", line_at(main_win())) end) end) + +-- A copy carries `oldpath` too, naming the file it was copied from. Git only +-- reports copies when asked, and only for sources modified in the same commit, +-- so `keep.txt` changes as it is copied. `copy.txt` sorts first, so a walk that +-- takes `oldpath` at face value picks the copy and loses the original. +-- +-- p1 keep.txt body 1..20 (20 lines) +-- p2 keep.txt -> copy.txt (C100) copy of p1's text (20) +-- keep.txt body 12 rewritten (20) +local function make_copy_repo() + local repo = helpers.init_repo() + helpers.run({ "git", "config", "diff.renames", "copies" }, repo) + local lines = body("body", 20) + + write(repo, "keep.txt", lines) + commit(repo, "p1") + + write(repo, "copy.txt", lines) + lines[12] = "body 12 rewritten" + write(repo, "keep.txt", lines) + commit(repo, "p2") + + return repo +end + +describe("select_change_here across a copy", function() + local repo, cwd, view, original_config + + before_each(function() + original_config = vim.deepcopy(config.get_config()) + config.get_config().use_icons = false + repo = make_copy_repo() + cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo)) + end) + + after_each(function() + vim.cmd("cd " .. vim.fn.fnameescape(cwd)) + helpers.close_view(view) + view = nil + helpers.cleanup_repo(repo) + config.setup(original_config) + end) + + local function main_win() + return view.cur_layout:get_main_win().id + end + + it("stays with the original rather than following the copy", function() + view = lib.file_history(nil, {}) + assert.is_not_nil(view) + view:open() + + assert.is_true( + vim.wait(10000, function() + return view.ready and #view.panel.entries >= 2 and view.cur_layout ~= nil + end), + "view never became ready" + ) + + -- The fixture only holds if git reported the copy as one. + local copy + for _, f in ipairs(view.panel.entries[1].files) do + if f.path == "copy.txt" then + copy = f + end + end + eq("C", assert(copy).status) + eq("keep.txt", copy.oldpath) + + -- Open p1 with the cursor on the line p2 rewrites in `keep.txt`. + view:set_file(view.panel.entries[2].files[1]) + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[2] + and api.nvim_buf_line_count(api.nvim_win_get_buf(main_win())) == 20 + end), + "p1 never opened" + ) + vim.wait(200) + local main = main_win() + api.nvim_set_current_win(main) + api.nvim_win_set_cursor(main, { 12, 0 }) + eq("body 12", line_at(main)) + + view:select_change_here(-1) + + -- p2 lists `copy.txt` first, with `oldpath` naming `keep.txt`. The copy + -- still reads `body 12`; the rewrite happened in `keep.txt`. + assert.is_true( + vim.wait(20000, function() + return view.panel.cur_item[1] == view.panel.entries[1] + and view.panel.cur_item[2].path == "keep.txt" + end), + "the walk never came to rest on keep.txt in p2" + ) + vim.wait(200) + eq("body 12 rewritten", line_at(main_win())) + end) +end) From 6a41cd8f3d0a3520b9d593bd4ab2a35fbe0321e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:06:14 +0200 Subject: [PATCH 4/6] fixup! feat(file-history): step across renames --- .../views/file_history/file_history_view.lua | 12 ++++++---- .../scene/views/standard/standard_view.lua | 4 ++++ .../tests/functional/change_here_spec.lua | 24 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/lua/diffview/scene/views/file_history/file_history_view.lua b/lua/diffview/scene/views/file_history/file_history_view.lua index 5a7eb00c..79ead1ee 100644 --- a/lua/diffview/scene/views/file_history/file_history_view.lua +++ b/lua/diffview/scene/views/file_history/file_history_view.lua @@ -301,6 +301,14 @@ FileHistoryView.select_change_here = async.void(function(self, dir) local err, next_lines = await(file.adapter:show(file.path, file.rev)) + -- The read yielded, and the view may have closed or lost its tabpage in + -- the meantime. `show` resumes us in a fast event context, where that + -- cannot be asked. + await(async.scheduler()) + if self:swap_cancelled() then + return + end + if err or not next_lines then found = candidate break @@ -354,10 +362,6 @@ FileHistoryView.select_change_here = async.void(function(self, dir) end end - -- `adapter:show` resumes us in its job's `on_exit`, which is a fast event - -- context. Everything below touches the API. - await(async.scheduler()) - -- Nothing ahead changes this line, so the reader stays where they are rather -- than being dropped at the far end of the history. if not found then diff --git a/lua/diffview/scene/views/standard/standard_view.lua b/lua/diffview/scene/views/standard/standard_view.lua index 8e28f4ab..f48aa526 100644 --- a/lua/diffview/scene/views/standard/standard_view.lua +++ b/lua/diffview/scene/views/standard/standard_view.lua @@ -43,6 +43,10 @@ end ---@field package _set_file_pending FileEntry? # Newest file queued while `_set_file_in_flight` is set; the worker picks it up before terminating. local StandardView = oop.create_class("StandardView", View.__get()) +---Exposed for the file-history walk, whose reads yield the same way the +---swap does and must stop for the same reasons. +StandardView.swap_cancelled = swap_cancelled + ---The key the arriving entry will look its state up under, when a rename links ---it to the entry being left. `--follow` lists a file under its old name in ---every commit older than the rename, so a step across that commit leaves one diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua index 8f0a95f3..1b2cde3c 100644 --- a/lua/diffview/tests/functional/change_here_spec.lua +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -195,6 +195,30 @@ describe("select_change_here", function() eq("body 12", line_at(main)) end) + it("gives up when the view loses its tabpage", function() + -- Nothing older changes `body 12`, so a walk that runs to the end would + -- report that; giving up early reports nothing. + open_on("body 12") + local utils = require("diffview.utils") + local original_info, message = utils.info, nil + utils.info = function(msg) + message = msg + end + -- The walk yields on every read, and a view whose tabpage is no longer + -- current must not move the cursor or report anything when it resumes. + vim.cmd("tabnew") + + view:select_change_here(1) + vim.wait(2000, function() + return message ~= nil or view.panel.cur_item[1] ~= view.panel.entries[1] + end) + utils.info = original_info + vim.cmd("tabclose") + + eq(nil, message) + eq(view.panel.entries[1], view.panel.cur_item[1]) + end) + it("runs the walk through the registered action", function() open_on("body 5 rewritten") From eb9625013f55256030398b2f9547d1e896fe56d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:06:14 +0200 Subject: [PATCH 5/6] fixup! feat(file-history): step across renames --- .../views/file_history/file_history_view.lua | 7 +++--- .../tests/functional/change_here_spec.lua | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lua/diffview/scene/views/file_history/file_history_view.lua b/lua/diffview/scene/views/file_history/file_history_view.lua index 79ead1ee..327f123d 100644 --- a/lua/diffview/scene/views/file_history/file_history_view.lua +++ b/lua/diffview/scene/views/file_history/file_history_view.lua @@ -271,8 +271,9 @@ FileHistoryView.select_change_here = async.void(function(self, dir) -- them, so walking toward the older commits the answer is the entry read -- before this one: `lines` is its content, and `lnum` the cursor in it. local prev - -- The panel appends entries as the history loads, so running out of them - -- means the end of the history only once it has stopped. + -- The panel appends older entries as the history loads, so running out of + -- them in that direction means the end of the history only once it has + -- stopped. The newest entry is in place from the start. local still_loading = false -- The name the file goes by in the entries ahead. A rename changes it. local path = cur_file.path @@ -281,7 +282,7 @@ FileHistoryView.select_change_here = async.void(function(self, dir) idx = idx + dir local entry = self.panel.entries[idx] if not entry then - still_loading = self.panel.updating + still_loading = dir > 0 and self.panel.updating break end diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua index 1b2cde3c..e991c790 100644 --- a/lua/diffview/tests/functional/change_here_spec.lua +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -195,6 +195,30 @@ describe("select_change_here", function() eq("body 12", line_at(main)) end) + it("reports the end of the history walking toward the newer commits, even mid-load", function() + local main = open_on("body 12") + local utils = require("diffview.utils") + local original_info, message = utils.info, nil + utils.info = function(msg) + message = msg + end + -- Entries are appended at the older end, so the newest commit is in place + -- from the start and nothing newer can still be on its way. + view.panel.updating = true + + view:select_change_here(-1) + + local got = vim.wait(10000, function() + return message ~= nil + end) + utils.info = original_info + view.panel.updating = false + + assert.is_true(got, "the walk never reported anything") + assert.is_falsy(message:match("still loading")) + eq("body 12", line_at(main)) + end) + it("gives up when the view loses its tabpage", function() -- Nothing older changes `body 12`, so a walk that runs to the end would -- report that; giving up early reports nothing. From bde249df7d02caa898d4c1452ffb91bd7446cbbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Fl=C3=BCgge?= <952313+seflue@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:06:14 +0200 Subject: [PATCH 6/6] fixup! feat(file-history): step across renames --- .../scene/views/standard/standard_view.lua | 16 +++++++++++++++- .../tests/functional/change_here_spec.lua | 6 ++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lua/diffview/scene/views/standard/standard_view.lua b/lua/diffview/scene/views/standard/standard_view.lua index f48aa526..42d654e5 100644 --- a/lua/diffview/scene/views/standard/standard_view.lua +++ b/lua/diffview/scene/views/standard/standard_view.lua @@ -118,7 +118,14 @@ function StandardView:init(opt) -- saves keep cursor + viewport for every visited file. self.emitter:on("file_open_pre", function(_, target, cur_entry) if cur_entry and cur_entry.path then - self:snapshot_main_view(cur_entry.path, StandardView._rename_alias(cur_entry, target)) + local alias = StandardView._rename_alias(cur_entry, target) + -- A caller that resolved a line for `target` followed the file there + -- itself, across whatever renames lie between, so the two names hold + -- one file even when neither entry lists the other's. + if not alias and target and target.path ~= cur_entry.path and self:has_carry_lnum(target) then + alias = target.path + end + self:snapshot_main_view(cur_entry.path, alias) end end) @@ -281,6 +288,13 @@ function StandardView:set_carry_lnum(file, lnum) self._carry_lnum = { file = file, lnum = lnum } end +---Whether a caller has resolved a line for `file` that the next open consumes. +---@param file FileEntry +---@return boolean +function StandardView:has_carry_lnum(file) + return self._carry_lnum ~= nil and self._carry_lnum.file == file +end + ---Pop and apply the saved view state for `path`. Diffing the snapshotted ---buffer against the arriving one moves the cursor line with its code, so a ---step lands on the same line of code rather than the same line number. diff --git a/lua/diffview/tests/functional/change_here_spec.lua b/lua/diffview/tests/functional/change_here_spec.lua index e991c790..2f077ff8 100644 --- a/lua/diffview/tests/functional/change_here_spec.lua +++ b/lua/diffview/tests/functional/change_here_spec.lua @@ -530,10 +530,11 @@ end) -- so a walk that matches on one name alone goes blind at the rename and runs -- off the end of the history. The rename itself touches no line, so a walk -- that crosses it has to pass it over like any other commit that leaves the --- line alone. +-- line alone. n2 rewrites a line above the one walked to, so the tests can +-- tell the walked line from the first change. -- -- n1 a_other.txt + keep.txt body 1..20 (20 lines) --- n2 a_other.txt + keep.txt body 5 rewritten (20) +-- n2 a_other.txt + keep.txt body 2 and 5 rewritten (20) -- n3 a_other.txt + keep.txt -> moved.txt pure rename (20) -- n4 a_other.txt + moved.txt head 1..10, body 12 rewritten (30) local function make_rename_repo() @@ -544,6 +545,7 @@ local function make_rename_repo() write(repo, "keep.txt", lines) commit(repo, "n1") + lines[2] = "body 2 rewritten" lines[5] = "body 5 rewritten" write(repo, "a_other.txt", body("other", 12)) write(repo, "keep.txt", lines)