From 3ee0b79a155fbb40278eb31e261f9c94e9340270 Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Fri, 6 Mar 2026 23:03:21 -0500 Subject: [PATCH] fix(render): store valid diff structure for single-pane views show_untracked_file() and similar single-pane functions stored {} as stored_diff_result, missing the required .changes and .moves fields. When resume_diff() reused this value after tab cycling, render_diff() crashed on ipairs(nil) because {}.changes is nil. Replace {} with {changes={}, moves={}} in side_by_side.lua and inline_view.lua to conform to the diff result schema. Add E2E test that reproduces the exact PR #309 scenario. Closes #309 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lua/codediff/ui/view/inline_view.lua | 4 +- lua/codediff/ui/view/side_by_side.lua | 2 +- tests/e2e/tab_cycle_untracked.lua | 133 ++++++++++++++++++++++++++ tests/ui/layout_spec.lua | 4 +- 4 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/tab_cycle_untracked.lua diff --git a/lua/codediff/ui/view/inline_view.lua b/lua/codediff/ui/view/inline_view.lua index 44a913ae..0567eb52 100644 --- a/lua/codediff/ui/view/inline_view.lua +++ b/lua/codediff/ui/view/inline_view.lua @@ -596,7 +596,7 @@ function M.show_single_file(tabpage, file_path, opts) lifecycle.update_buffers(tabpage, orig_bufnr, mod_bufnr) lifecycle.update_paths(tabpage, original_path, modified_path) lifecycle.update_revisions(tabpage, original_revision, modified_revision) - lifecycle.update_diff_result(tabpage, {}) + lifecycle.update_diff_result(tabpage, { changes = {}, moves = {} }) local view_keymaps = require("codediff.ui.view.keymaps") view_keymaps.setup_all_keymaps(tabpage, orig_bufnr, mod_bufnr, session.mode == "explorer") @@ -633,7 +633,7 @@ function M.show_welcome(tabpage, load_bufnr) lifecycle.update_buffers(tabpage, empty_buf, load_bufnr) lifecycle.update_paths(tabpage, "", "") lifecycle.update_revisions(tabpage, nil, nil) - lifecycle.update_diff_result(tabpage, {}) + lifecycle.update_diff_result(tabpage, { changes = {}, moves = {} }) local view_keymaps = require("codediff.ui.view.keymaps") view_keymaps.setup_all_keymaps(tabpage, empty_buf, load_bufnr, session.mode == "explorer") diff --git a/lua/codediff/ui/view/side_by_side.lua b/lua/codediff/ui/view/side_by_side.lua index 307db9a3..0006c976 100644 --- a/lua/codediff/ui/view/side_by_side.lua +++ b/lua/codediff/ui/view/side_by_side.lua @@ -766,7 +766,7 @@ local function show_single_file(tabpage, opts) lifecycle.update_buffers(tabpage, orig_bufnr, mod_bufnr) lifecycle.update_paths(tabpage, opts.original_path or "", opts.modified_path or "") lifecycle.update_revisions(tabpage, opts.original_revision, opts.modified_revision) - lifecycle.update_diff_result(tabpage, {}) + lifecycle.update_diff_result(tabpage, { changes = {}, moves = {} }) local view_keymaps = require("codediff.ui.view.keymaps") view_keymaps.setup_all_keymaps(tabpage, orig_bufnr, mod_bufnr, session.mode == "explorer") diff --git a/tests/e2e/tab_cycle_untracked.lua b/tests/e2e/tab_cycle_untracked.lua new file mode 100644 index 00000000..660a7e61 --- /dev/null +++ b/tests/e2e/tab_cycle_untracked.lua @@ -0,0 +1,133 @@ +-- E2E Scenario: Tab cycling with untracked file should not crash (PR #309) +-- +-- Root cause: show_untracked_file() stored {} as stored_diff_result instead of +-- {changes={}, moves={}}. When resume_diff() reuses that value (no recompute +-- needed), render_diff() crashes on ipairs(nil) because {}.changes is nil. +-- +-- This test validates the invariant directly: after selecting an untracked file, +-- stored_diff_result.changes must be a table (not nil). It then performs a full +-- tab cycle to exercise the resume_diff path end-to-end. +return { + setup = function(ctx, e2e) + ctx.repo = e2e.create_temp_git_repo() + ctx.repo.write_file("tracked.txt", { "hello world" }) + ctx.repo.git("add . && git commit -m 'initial'") + ctx.repo.write_file("untracked.txt", { "I am untracked" }) + vim.cmd("edit " .. ctx.repo.path("tracked.txt")) + end, + + run = function(ctx, e2e) + e2e.exec("CodeDiff") + e2e.wait_for_explorer(5000) + e2e.wait_for_diff_ready(5000) + + -- Find the untracked file in the explorer tree + local function find_untracked() + local lines = e2e.get_explorer_files() + ctx.explorer_lines = lines + if not lines then return nil end + for i, line in ipairs(lines) do + if line:find("untracked.txt") then return i end + end + return nil + end + + local untracked_line = find_untracked() + + -- If section is collapsed, expand it first + if not untracked_line and ctx.explorer_lines then + for i, line in ipairs(ctx.explorer_lines) do + if line:find("ntracked") then + e2e.select_explorer_item(i) + vim.wait(500) + break + end + end + untracked_line = find_untracked() + end + + ctx.found_untracked = untracked_line ~= nil + if not untracked_line then + ctx.error = "Could not find untracked.txt in explorer" + return + end + + -- Select the untracked file → triggers show_untracked_file → single-pane view + e2e.select_explorer_item(untracked_line) + vim.wait(1000) + + -- KEY CHECK: Capture stored_diff_result IMMEDIATELY after show_untracked_file. + -- Before the fix this was {}, meaning .changes and .moves were nil. + -- After the fix this is {changes={}, moves={}}. + local session = e2e.get_diff_session() + if session and session.stored_diff_result then + ctx.immediate_has_changes = session.stored_diff_result.changes ~= nil + ctx.immediate_has_moves = session.stored_diff_result.moves ~= nil + ctx.immediate_changes_type = type(session.stored_diff_result.changes) + else + ctx.immediate_has_changes = false + ctx.immediate_has_moves = false + end + + -- Now exercise the full tab-cycle path (suspend → resume → render) + ctx.codediff_tabnr = vim.fn.tabpagenr() + ctx.codediff_tabpage = vim.api.nvim_get_current_tabpage() + + vim.cmd("tabnew") + vim.wait(500) + + -- Cycle back via tabnext (triggers TabEnter → vim.schedule → resume_diff) + ctx.cycle_ok, ctx.cycle_err = pcall(function() + vim.cmd("tabnext " .. ctx.codediff_tabnr) + end) + + -- Let TabEnter → vim.schedule → resume_diff complete + local lifecycle = require("codediff.ui.lifecycle") + vim.wait(3000, function() + local s = lifecycle.get_session(ctx.codediff_tabpage) + return s and not s.suspended + end, 50) + + -- Capture state after the full cycle + local after = lifecycle.get_session(ctx.codediff_tabpage) + ctx.session_alive = after ~= nil + if after then + ctx.after_suspended = after.suspended + ctx.after_mod_win_valid = after.modified_win and vim.api.nvim_win_is_valid(after.modified_win) + end + end, + + validate = function(ctx, e2e) + local ok = true + + ok = ok and e2e.assert_true(ctx.found_untracked, + "Should find untracked.txt in explorer (lines: " .. vim.inspect(ctx.explorer_lines) .. ")") + if not ctx.found_untracked then return false end + + ok = ok and e2e.assert_true(ctx.error == nil, "No error: " .. tostring(ctx.error)) + + -- Core invariant: stored_diff_result must have .changes right after show_untracked_file + ok = ok and e2e.assert_true(ctx.immediate_has_changes, + "stored_diff_result.changes must not be nil immediately after show_untracked_file (was: " + .. tostring(ctx.immediate_changes_type) .. ")") + + ok = ok and e2e.assert_true(ctx.immediate_has_moves, + "stored_diff_result.moves must not be nil immediately after show_untracked_file") + + -- Tab cycle should not crash + ok = ok and e2e.assert_true(ctx.cycle_ok ~= false, + "Tab cycle should not error: " .. tostring(ctx.cycle_err)) + + -- Session survives the cycle + ok = ok and e2e.assert_true(ctx.session_alive, "Session should exist after tab cycle") + ok = ok and e2e.assert_true(ctx.after_suspended == false, "Session should resume after tab cycle") + ok = ok and e2e.assert_true(ctx.after_mod_win_valid, "Modified window should be valid after tab cycle") + + return ok + end, + + cleanup = function(ctx, e2e) + pcall(function() e2e.cleanup_tabs() end) + if ctx.repo then ctx.repo.cleanup() end + end, +} diff --git a/tests/ui/layout_spec.lua b/tests/ui/layout_spec.lua index 5db67d43..dd79d668 100644 --- a/tests/ui/layout_spec.lua +++ b/tests/ui/layout_spec.lua @@ -944,7 +944,7 @@ describe("Layout Manager", function() -- Validate: empty diff result (no highlights) assert.is_truthy(session.stored_diff_result, "Should have diff result") - assert.is_nil(session.stored_diff_result.changes, "Should have no changes (empty diff)") + assert.same(session.stored_diff_result.changes, {}, "Should have no changes (empty diff)") vim.fn.delete(tmp_file) cleanup_mock_session(tabpage) @@ -1082,7 +1082,7 @@ describe("Layout Manager", function() -- Validate: empty diff result assert.is_truthy(session.stored_diff_result, "Should have diff result") - assert.is_nil(session.stored_diff_result.changes, "Should have no changes") + assert.same(session.stored_diff_result.changes, {}, "Should have no changes") cleanup_mock_session(tabpage) end)