refactor(view): swap StandardView.cur_layout atomically

lua/diffview/scene/layout.lua

@@ -370,7 +370,11 @@ function Layout:is_nulled()
   return true
 end
 
----Validate the layout and recover if necessary.
+---Validate the layout and recover if necessary. Recover is only ever
+---triggered against the `StandardView.cur_layout`, and `use_entry`
+---publishes a new layout to `cur_layout` only AFTER its `create` future
+---has fully resolved (and the old layout has been destroyed), so a
+---call to `ensure` cannot land on a layout that is still mid-swap.
 function Layout:ensure()
   local state = self:validate()
 

lua/diffview/scene/views/standard/standard_view.lua

@@ -13,7 +13,7 @@ local oop = lazy.require("diffview.oop") ---@module "diffview.oop"
 local utils = lazy.require("diffview.utils") ---@module "diffview.utils"
 
 local api = vim.api
-local await = async.await
+local await, pawait = async.await, async.pawait
 
 local M = {}
 
@@ -200,12 +200,22 @@ function StandardView:ensure_layout()
   end
 end
 
+---Clone `layout`, cache the clone in `self.layouts`, and attach a
+---`pivot_producer` to it — without touching `self.cur_layout`. Reuses
+---an existing cache entry for the same class so pinned variants keep
+---their file identities across swaps.
 ---@param layout Layout
-function StandardView:use_layout(layout)
-  self.cur_layout = layout:clone()
-  self.layouts[layout.class] = self.cur_layout
+---@return Layout
+function StandardView:_stage_layout(layout)
+  local staged = self.layouts[layout.class]
+  if staged then
+    return staged
+  end
+
+  staged = layout:clone()
+  self.layouts[layout.class] = staged
 
-  self.cur_layout.pivot_producer = function()
+  staged.pivot_producer = function()
     local was_open = self.panel:is_open()
     local was_only_win = was_open and #utils.tabpage_list_normal_wins(self.tabpage) == 1
     self.panel:close()
@@ -224,6 +234,13 @@ function StandardView:use_layout(layout)
 
     return pivot
   end
+
+  return staged
+end
+
+---@param layout Layout
+function StandardView:use_layout(layout)
+  self.cur_layout = self:_stage_layout(layout)
 end
 
 ---Save the panel cursor position for later restoration.
@@ -288,21 +305,43 @@ StandardView.use_entry = async.void(function(self, entry)
     self.cur_layout.emitter = entry.layout.emitter
     await(self.cur_layout:use_entry(entry))
   else
-    if self.layouts[entry.layout.class] then
-      self.cur_layout = self.layouts[entry.layout.class]
-      self.cur_layout.emitter = entry.layout.emitter
-    else
-      self:use_layout(entry.layout)
-      self.cur_layout.emitter = entry.layout.emitter
+    -- Atomic swap: `self.cur_layout` must always expose a valid,
+    -- fully-created layout, since observers can read it at any point
+    -- (notably `update_files_impl`'s debounced `ensure_layout` and any
+    -- autocmd that `Window:close` fires synchronously during teardown).
+    -- So: build the incoming layout off to the side, publish it only
+    -- once `create` has resolved, and destroy the old one only after
+    -- publishing. A concurrent `Layout:ensure` then never sees a
+    -- half-built or half-destroyed layout and cannot trip `recover`
+    -- against one.
+    --
+    -- Destroying the old layout AFTER the new create also lets
+    -- `find_pivot` split one of the old windows for the new layout's
+    -- pivot, anchoring the new windows inside the current diff area
+    -- rather than the null split `Panel:close` fabricates on a
+    -- last-window close.
+    local new_layout = self:_stage_layout(entry.layout)
+    new_layout.emitter = entry.layout.emitter
+
+    -- Guard the staging phase: if `use_entry` or `create` errors
+    -- part-way through, any windows `create_wins` already made would
+    -- linger as orphans and the half-built `new_layout` would stay in
+    -- `self.layouts` for the next swap to reuse. Tear it down and drop
+    -- the cache entry before rethrowing so `_stage_layout` re-clones
+    -- next time.
+    local ok, err = pawait(async.void(function()
+      await(new_layout:use_entry(entry))
+      await(new_layout:create())
+    end))
+    if not ok then
+      new_layout:destroy()
+      self.layouts[entry.layout.class] = nil
+      error(err)
     end
 
-    await(self.cur_layout:use_entry(entry))
-    local future = self.cur_layout:create()
+    self.cur_layout = new_layout
     old_layout:destroy()
 
-    -- Wait for files to be created + opened
-    await(future)
-
     if not vim.o.equalalways then
       vim.cmd("wincmd =")
     end

lua/diffview/tests/functional/restore_focus_after_layout_swap_spec.lua

@@ -8,9 +8,10 @@ local Diff2Hor = require("diffview.scene.layouts.diff_2_hor").Diff2Hor
 local EventEmitter = require("diffview.events").EventEmitter
 local GitAdapter = require("diffview.vcs.adapters.git").GitAdapter
 local GitRev = require("diffview.vcs.adapters.git.rev").GitRev
+local Layout = require("diffview.scene.layout").Layout
 local RevType = require("diffview.vcs.rev").RevType
 
-local await = async.await
+local await, pawait = async.await, async.pawait
 local eq = helpers.eq
 local run = helpers.run
 local cleanup_repo = helpers.cleanup_repo
@@ -150,4 +151,97 @@ describe("StandardView.use_entry layout-swap focus (integration)", function()
       end
     end)
   )
+
+  -- The atomic-swap invariant: `self.cur_layout` always exposes a
+  -- valid layout to observers throughout the swap — the outgoing one
+  -- until `create` resolves, then the fully-created new one while
+  -- the old windows are torn down. A concurrent `ensure_layout` (the
+  -- debounced `update_files_impl` firing mid-swap, or any autocmd
+  -- that window teardown fires synchronously) therefore never sees a
+  -- half-built layout and cannot trip `Layout:recover`.
+  it(
+    "keeps cur_layout on a valid layout throughout the swap",
+    helpers.async_test(function()
+      config.setup({
+        use_icons = false,
+        view = {
+          default = { layout = "diff2_horizontal", focus_diff = false },
+          one_sided_layout = "raw",
+        },
+      })
+
+      local repo = make_repo()
+      local view
+
+      local ok, err = pcall(function()
+        local adapter = GitAdapter({ toplevel = repo, cpath = repo, path_args = {} })
+        view = DiffView({
+          adapter = adapter,
+          rev_arg = nil,
+          path_args = {},
+          left = GitRev(RevType.STAGE, 0),
+          right = GitRev(RevType.LOCAL),
+          options = { show_untracked = true },
+        })
+        assert.is_true(view:is_valid())
+
+        view:open()
+        local loaded = vim.wait(3000, function()
+          return view.initialized
+        end, 10)
+        assert.is_true(loaded, "view did not finish loading within 3s")
+
+        if view._set_file_in_flight then
+          await(view._set_file_in_flight)
+        end
+
+        local modified_entry, raw_entry
+        for _, f in view.files:iter() do
+          if f.path == "existing.txt" then
+            modified_entry = f
+          elseif f.path == "newfile.txt" then
+            raw_entry = f
+          end
+        end
+
+        -- Land on the Diff2Hor entry as the swap's outgoing layout.
+        await(view:set_file(modified_entry, false, false))
+        eq(Diff2Hor, view.cur_layout.class)
+
+        -- Spy on the invariant. Sample `self.cur_layout:is_valid()` on
+        -- every `Layout:destroy` entry that fires during the swap: at
+        -- each of those points the view must still expose a valid
+        -- layout to any observer (concurrent `ensure_layout`, autocmds
+        -- that window teardown fires synchronously, etc.). The specific
+        -- identity of that layout — outgoing vs. freshly-created —
+        -- doesn't matter to the invariant; only its validity does.
+        local snapshots = {}
+        local orig_destroy = Layout.destroy
+        Layout.destroy = function(layout_self)
+          snapshots[#snapshots + 1] = view.cur_layout and view.cur_layout:is_valid()
+          orig_destroy(layout_self)
+        end
+
+        local swap_ok, swap_err = pawait(view.set_file, view, raw_entry, false, false)
+        Layout.destroy = orig_destroy
+        assert(swap_ok, swap_err)
+
+        eq(Diff1Raw, view.cur_layout.class)
+
+        -- At least one destroy fires during the swap (the outgoing
+        -- Diff2Hor). Every snapshot taken while the swap was in
+        -- flight must show a valid `cur_layout`.
+        assert.is_true(#snapshots > 0, "expected at least one Layout:destroy call during the swap")
+        for i, valid in ipairs(snapshots) do
+          assert.is_true(valid, "cur_layout was invalid at destroy call #" .. i)
+        end
+      end)
+
+      close_view(view)
+      cleanup_repo(repo)
+      if not ok then
+        error(err)
+      end
+    end)
+  )
 end)