diff --git a/lua/diffview/scene/view.lua b/lua/diffview/scene/view.lua index 5af5fd48..6669e065 100644 --- a/lua/diffview/scene/view.lua +++ b/lua/diffview/scene/view.lua @@ -188,13 +188,11 @@ function View:open() apply_diffopt(self) DiffviewGlobal.emitter:emit("view_opened", self) DiffviewGlobal.emitter:emit("view_enter", self) - -- Drain `post_open`'s scheduled entry setup before returning. Otherwise - -- typeahead after the command can reach a buffer whose diffview keymap - -- hasn't attached yet and falls through to native `:diffget` (see #262). - vim.wait(2000, function() - ---@diagnostic disable-next-line: undefined-field - return self.ready and self.cur_entry ~= nil - end) + -- No wait guard here: `init_layout` synchronously drops `File.NULL_FILE` + -- into every diffview window, and `_get_null_buffer` installs buffer-local + -- `` mappings for `do`/`dp`/`[1-3]do`. Typeahead landing on the + -- placeholder before the real diff buffer is swapped in can no longer fall + -- through to native `:diffget` (see #262). end ---@param opts? diffview.View.CloseOpts # Forwarded to subclass overrides; ignored at the base level. diff --git a/lua/diffview/tests/functional/file_merge_view_spec.lua b/lua/diffview/tests/functional/file_merge_view_spec.lua index 9a0139d8..b3084099 100644 --- a/lua/diffview/tests/functional/file_merge_view_spec.lua +++ b/lua/diffview/tests/functional/file_merge_view_spec.lua @@ -287,11 +287,21 @@ describe("diffview.scene.views.diff.file_merge_view", function() end) end) - -- Regression: `View:open` waits for the scheduled entry setup so - -- typeahead after `:DiffviewMergeFiles` reaches the real MERGED buffer - -- with its `Ndo` keymap attached (issue #262). - describe("FileMergeView:open (issue #262 race guard)", function() - it("populates `cur_entry` and `ready` before returning", function() + -- Regression: after `:DiffviewMergeFiles`, every diff-layout window + -- must have `do` mapped away from native `:diffget` so typeahead can't + -- fall through to E99/E101 (issue #262). The invariant lives on + -- `File.NULL_FILE`, which `init_layout` installs synchronously. + describe("FileMergeView:open (issue #262)", function() + local function buf_do_rhs(bufnr) + for _, m in ipairs(vim.api.nvim_buf_get_keymap(bufnr, "n")) do + if m.lhs == "do" then + return m.rhs + end + end + return nil + end + + it("guards `do` on every diff-layout window before returning", function() local output = tmpfile("<<<<<<< HEAD\nc\n=======\ne\n>>>>>>> what\n") local base = tmpfile("a\n") local left = tmpfile("c\n") @@ -301,8 +311,13 @@ describe("diffview.scene.views.diff.file_merge_view", function() assert.is_not_nil(view) view:open() - assert.is_true(view.ready) - assert.is_not_nil(view.cur_entry) + assert.is_truthy(view.cur_layout) + assert.is_true(#view.cur_layout.windows > 0) + for _, win in ipairs(view.cur_layout.windows) do + assert.is_true(win:is_valid()) + local bufnr = vim.api.nvim_win_get_buf(win.id) + assert.are.equal("", buf_do_rhs(bufnr)) + end helpers.close_view(view) os.remove(output) diff --git a/lua/diffview/tests/functional/open_race_guard_spec.lua b/lua/diffview/tests/functional/open_race_guard_spec.lua index 91ab2adb..13d16744 100644 --- a/lua/diffview/tests/functional/open_race_guard_spec.lua +++ b/lua/diffview/tests/functional/open_race_guard_spec.lua @@ -1,18 +1,36 @@ --- Regression: `View:open` waits for the scheduled entry setup so typeahead --- after `:DiffviewOpen` on a conflict reaches the real file buffer with its --- `Ndo` keymap attached (issue #262). Mirrors the `FileMergeView` case in --- `file_merge_view_spec.lua` from the `DiffView` / git-adapter side. +-- Regression: typeahead after `:DiffviewOpen` on a conflict must not fall +-- through to native `:diffget` (E99/E101). The invariant is now provided by +-- `File._get_null_buffer` installing buffer-local `` mappings on the +-- shared null placeholder, which `init_layout` puts into every diffview +-- window synchronously before `View:open` returns (see #262). The prompt- +-- return case (#289) drops out for free: `View:open` no longer waits. local config = require("diffview.config") local helpers = require("diffview.tests.helpers") local DiffView = require("diffview.scene.views.diff.diff_view").DiffView local EventEmitter = require("diffview.events").EventEmitter +local File = require("diffview.vcs.file").File local GitAdapter = require("diffview.vcs.adapters.git").GitAdapter local GitRev = require("diffview.vcs.adapters.git.rev").GitRev local RevType = require("diffview.vcs.rev").RevType local run = helpers.run +---Return the `rhs` of a normal-mode buffer-local mapping for `lhs` on `bufnr`, +---or nil if none exists. +---@param bufnr integer +---@param lhs string +---@return string? +local function buf_nmap_rhs(bufnr, lhs) + local maps = vim.api.nvim_buf_get_keymap(bufnr, "n") + for _, m in ipairs(maps) do + if m.lhs == lhs then + return m.rhs + end + end + return nil +end + local function make_conflict_repo() local repo = helpers.init_repo() local path = repo .. "/file.txt" @@ -42,7 +60,26 @@ local function make_conflict_repo() return repo end -describe("DiffView:open (issue #262 race guard)", function() +describe("null-placeholder typeahead guard (issue #262)", function() + it("shadows the built-in diff keys with `` on the shared null buffer", function() + local bn = File._get_null_buffer() + assert.is_true(vim.api.nvim_buf_is_loaded(bn)) + + for _, lhs in ipairs({ "do", "dp", "1do", "2do", "3do" }) do + local rhs = buf_nmap_rhs(bn, lhs) + assert.are.equal( + "", + rhs, + ("buffer-local mapping for %q on the null buffer should be `` (empty rhs), got %s"):format( + lhs, + vim.inspect(rhs) + ) + ) + end + end) +end) + +describe("DiffView:open (issue #262 / #289)", function() local orig_emitter, original_config before_each(function() @@ -57,7 +94,76 @@ describe("DiffView:open (issue #262 race guard)", function() config.setup(original_config) end) - it("populates `cur_entry` and `ready` before returning on a git conflict", function() + it("guards `do` on every diff-layout window's buffer before returning", function() + local repo = make_conflict_repo() + local view + + local ok, err = pcall(function() + view = DiffView({ + adapter = GitAdapter({ toplevel = repo, cpath = repo, path_args = {} }), + rev_arg = nil, + path_args = {}, + left = GitRev(RevType.STAGE, 0), + right = GitRev(RevType.LOCAL), + options = {}, + }) + view:open() + + -- The invariant that fixes #262: every diff-layout window that + -- typeahead could reach must already have `do` mapped away from the + -- built-in `:diffget`. `DiffView` focuses the file panel after + -- `init_layout`, so the *current* window is the panel; the racy + -- windows are the layout's diff windows, all still holding + -- `File.NULL_FILE`. + assert.is_truthy(view.cur_layout) + assert.is_true(#view.cur_layout.windows > 0) + for _, win in ipairs(view.cur_layout.windows) do + assert.is_true(win:is_valid()) + local bufnr = vim.api.nvim_win_get_buf(win.id) + assert.are.equal( + "", + buf_nmap_rhs(bufnr, "do"), + ("layout window %d buffer %d missing `do` guard"):format(win.id, bufnr) + ) + end + end) + + helpers.close_view(view) + helpers.cleanup_repo(repo) + if not ok then + error(err) + end + end) + + it("returns promptly on a clean worktree", function() + local repo = helpers.make_repo() + local view + + local ok, err = pcall(function() + view = DiffView({ + adapter = GitAdapter({ toplevel = repo, cpath = repo, path_args = {} }), + rev_arg = nil, + path_args = {}, + left = GitRev(RevType.STAGE, 0), + right = GitRev(RevType.LOCAL), + options = {}, + }) + local start = vim.uv.hrtime() + view:open() + local elapsed_ms = (vim.uv.hrtime() - start) / 1e6 + -- `View:open` no longer waits on anything; the previous 2000 ms + -- guard could burn the full timeout on a clean worktree (#289). + assert.is_true(elapsed_ms < 1500, ("open took %.0f ms"):format(elapsed_ms)) + end) + + helpers.close_view(view) + helpers.cleanup_repo(repo) + if not ok then + error(err) + end + end) + + it("flips `ready` once `post_open`'s scheduled work runs", function() local repo = make_conflict_repo() local view @@ -71,8 +177,11 @@ describe("DiffView:open (issue #262 race guard)", function() options = {}, }) view:open() - assert.is_true(view.ready) - assert.is_not_nil(view.cur_entry) + -- `ready` is now an "initial post_open dispatched" gate consumed by + -- `listeners.lua`'s `tab_enter`; it flips inside a `vim.schedule`. + assert.is_true(vim.wait(2000, function() + return view.ready + end)) end) helpers.close_view(view) diff --git a/lua/diffview/vcs/file.lua b/lua/diffview/vcs/file.lua index a045cfb5..f4c7df5e 100644 --- a/lua/diffview/vcs/file.lua +++ b/lua/diffview/vcs/file.lua @@ -726,6 +726,25 @@ function File.safe_delete_buf(bufnr) pcall(api.nvim_buf_delete, bufnr, { force = true }) end +-- Vim built-in diff keys that need to be swallowed while the null placeholder +-- is visible. Diff2 layouts use `do`/`dp`; Diff3/Diff4 layers `[1-3]do` on top +-- via the layout keymaps. Guarding them all keeps typeahead like `2do` from +-- resolving to native `:diffget` on an empty buffer between `init_layout` and +-- the first real `set_file` (see #262). +local NULL_GUARD_KEYS = { "do", "dp", "1do", "2do", "3do" } + +---@param bufnr integer +local function install_null_buffer_guards(bufnr) + for _, lhs in ipairs(NULL_GUARD_KEYS) do + vim.keymap.set({ "n", "x" }, lhs, "", { + buffer = bufnr, + nowait = true, + silent = true, + desc = "diffview: swallow typeahead on the null placeholder", + }) + end +end + ---@static Get the bufid of the null buffer. Create it if it's not loaded. ---@return integer function File._get_null_buffer() @@ -759,6 +778,11 @@ function File._get_null_buffer() File.NULL_FILE.bufnr = bn end + -- Re-install on every call so an adopted buffer whose earlier session was + -- torn down (and whose buffer-local maps went with it) still ends up + -- guarded. `vim.keymap.set` is idempotent for the same `{ mode, lhs, opts }`. + install_null_buffer_guards(File.NULL_FILE.bufnr) + return File.NULL_FILE.bufnr end