From 80a486719e2e1beeb9f6d97ee12699207b3e61d2 Mon Sep 17 00:00:00 2001 From: Oscar Wallberg Date: Tue, 28 Apr 2026 05:35:45 +0200 Subject: [PATCH] fix(git): resolve sidebar and diff-split rendering bugs --- lua/git/diff.lua | 120 ++++++++++++++++++++++++++------------------ lua/git/sidebar.lua | 80 ++++++++++++++++------------- 2 files changed, 117 insertions(+), 83 deletions(-) diff --git a/lua/git/diff.lua b/lua/git/diff.lua index 89f1607..5710ea6 100644 --- a/lua/git/diff.lua +++ b/lua/git/diff.lua @@ -59,36 +59,10 @@ local function attach_index_writer(buf, worktree, path) }) end ----Run `git show ` asynchronously and call `callback(lines?)` on the ----main loop. `lines` is nil on failure (the error is logged). ----@param worktree string ----@param revspec string anything `git show` accepts (e.g. `HEAD:foo`, `:foo`, blob SHA) ----@param callback fun(lines: string[]?) -local function read_show_async(worktree, revspec, callback) - vim.system( - { "git", "show", revspec }, - { cwd = worktree, text = true }, - vim.schedule_wrap(function(result) - if result.code ~= 0 then - log.error( - "git show %s failed: %s", - revspec, - vim.trim(result.stderr or "") - ) - callback(nil) - return - end - callback(util.split_lines(result.stdout or "")) - end) - ) -end - ----Internal builder: create a scratch buffer immediately and asynchronously ----fill it with the content of `git show `. Returning the buffer ----synchronously lets callers wire up windows / `:diffthis` right away; the ----diff updates when the content arrives. The buffer starts non-modifiable ----and the index `BufWriteCmd` is only attached after a successful load, so ----a premature `:w` can't blow away the index entry with empty content. +---Internal builder: create a scratch buffer and fill it with the output of +---`git show `. Synchronous so the buffer is ready by the time the +---caller wires up windows / `:diffthis`. An empty buffer briefly visible +---to the diff engine produces a spurious whole-file diff. ---@param worktree string ---@param revspec string ---@param is_index boolean @@ -99,22 +73,35 @@ local function build_show_buf(worktree, revspec, is_index, index_path) vim.bo[buf].buftype = "nofile" vim.bo[buf].bufhidden = "wipe" vim.bo[buf].swapfile = false - vim.bo[buf].modifiable = false + + local result = vim.system( + { "git", "show", revspec }, + { cwd = worktree, text = true } + ) + :wait() + if result.code ~= 0 then + log.error( + "git show %s failed: %s", + revspec, + vim.trim(result.stderr or "") + ) + else + vim.api.nvim_buf_set_lines( + buf, + 0, + -1, + false, + util.split_lines(result.stdout or "") + ) + end + + if is_index then + vim.bo[buf].buftype = "acwrite" + attach_index_writer(buf, worktree, assert(index_path)) + else + vim.bo[buf].modifiable = false + end vim.bo[buf].modified = false - read_show_async(worktree, revspec, function(lines) - if not vim.api.nvim_buf_is_valid(buf) then - return - end - vim.bo[buf].modifiable = true - vim.api.nvim_buf_set_lines(buf, 0, -1, false, lines or {}) - if is_index then - vim.bo[buf].buftype = "acwrite" - attach_index_writer(buf, worktree, assert(index_path)) - else - vim.bo[buf].modifiable = false - end - vim.bo[buf].modified = false - end) return buf end @@ -179,6 +166,28 @@ function M.set_buf_name_and_filetype(buf, name) end end +---Apply (or remove) the window-local options that `:diffthis` would +---normally set. Used by both `M.split` here and the sidebar so the two +---diff entry points behave consistently. Vim is supposed to save and +---restore these around the `'diff'` flag flip, but that round-trip is +---fragile when buffers are swapped under an already-diff window. +---@param win integer +---@param enabled boolean +function M.set_diff(win, enabled) + vim.wo[win].diff = enabled + if enabled then + vim.wo[win].foldmethod = "diff" + vim.wo[win].foldenable = true + vim.wo[win].foldlevel = 0 + vim.wo[win].scrollbind = true + vim.wo[win].cursorbind = true + vim.wo[win].wrap = false + else + vim.wo[win].scrollbind = false + vim.wo[win].cursorbind = false + end +end + ---@class ow.Git.SplitOpts ---@field ref string '' for index, 'HEAD' for HEAD ---@field vertical boolean @@ -191,6 +200,10 @@ function M.split(opts) log.warning("no file in current buffer") return end + if vim.bo[cur_buf].buftype ~= "" then + log.warning("cannot diff this buffer (not a worktree file)") + return + end local _, worktree = repo.resolve(cur_path) if not worktree then log.warning("not in a git repository") @@ -212,9 +225,20 @@ function M.split(opts) split = opts.vertical and "left" or "above", win = cur_win, }) - vim.wo[other_win].diff = true - vim.api.nvim_set_current_win(cur_win) - vim.wo[cur_win].diff = true + + -- The synthetic index/HEAD buffer can't run BufRead, so its filetype + -- detection in `set_buf_name_and_filetype` only catches + -- filename-pattern matches. Mirror cur_buf's filetype, since this is + -- the same logical file at a different version. Done after the + -- window opens so any BufWinEnter / BufEnter autocmds that fire on + -- nvim_open_win can't undo it. + local cur_ft = vim.bo[cur_buf].filetype + if cur_ft ~= "" then + vim.bo[other].filetype = cur_ft + end + + M.set_diff(cur_win, true) + M.set_diff(other_win, true) end return M diff --git a/lua/git/sidebar.lua b/lua/git/sidebar.lua index 2c0784e..5a0839a 100644 --- a/lua/git/sidebar.lua +++ b/lua/git/sidebar.lua @@ -34,6 +34,7 @@ local SIDEBAR_WIDTH = 50 ---@field worktree string ---@field lines table ---@field sidebar_win integer? +---@field invocation_win integer? window focused when the sidebar opened. The first diff repurposes it as the right pane ---@field diff_left_win integer? ---@field diff_right_win integer? ---@field user_aucmd integer? @@ -575,39 +576,30 @@ local function reset_diff_win(win) end) end ----@param sidebar_win integer +---Validate the window the user was in when the sidebar opened. The first +---diff repurposes it as the right pane, regardless of whether it holds an +---empty buffer or a real file. Returns nil if the user closed it, moved +---to another tabpage, or it's somehow the sidebar itself. +---@param s ow.Git.SidebarState ---@return integer? -local function find_default_main_win(sidebar_win) - local non_sidebar = {} - for _, win in ipairs(vim.api.nvim_tabpage_list_wins(0)) do - if win ~= sidebar_win then - table.insert(non_sidebar, win) - end - end - if #non_sidebar ~= 1 then +local function invocation_win_for(s) + local win = s.invocation_win + if not win or not vim.api.nvim_win_is_valid(win) then return nil end - local buf = vim.api.nvim_win_get_buf(non_sidebar[1]) - if - vim.api.nvim_buf_get_name(buf) == "" - and vim.bo[buf].buftype == "" - and not vim.bo[buf].modified - and vim.api.nvim_buf_line_count(buf) == 1 - and vim.api.nvim_buf_get_lines(buf, 0, 1, false)[1] == "" - then - return non_sidebar[1] + if win == s.sidebar_win then + return nil end + if + vim.api.nvim_win_get_tabpage(win) + ~= vim.api.nvim_get_current_tabpage() + then + return nil + end + return win end ----@param win integer ----@param enabled boolean -local function set_diff(win, enabled) - vim.wo[win].diff = enabled - if enabled then - vim.wo[win].foldenable = true - vim.wo[win].foldlevel = 0 - end -end +local set_diff = diff.set_diff ---@param s ow.Git.SidebarState ---@param sidebar_win integer @@ -696,18 +688,19 @@ local function show_diff(s, entry, focus_left) left_win = vsplit_at(right_win, "left") reset_diff_win(left_win) elseif not (left_win or right_win) then - local default_main = find_default_main_win(sidebar_win) - if default_main then - right_win = default_main + local target = invocation_win_for(s) + if target then + right_win = target reset_diff_win(right_win) left_win = vsplit_at(right_win, "left") reset_diff_win(left_win) else - -- No reusable default-empty window. Open the diff pair by - -- splitting from the sidebar. winfixwidth keeps the sidebar at 50 - -- when there are other windows to absorb the split; if the - -- sidebar is the only window in the tab, the split has to take - -- from the sidebar itself, so restore the width explicitly. + -- Invocation window is gone (closed or in another tabpage). + -- Open the diff pair by splitting from the sidebar. winfixwidth + -- keeps the sidebar at 50 when there are other windows to + -- absorb the split; if the sidebar is the only window in the + -- tab, the split has to take from the sidebar itself, so + -- restore the width explicitly. right_win = vsplit_at(sidebar_win, "right") reset_diff_win(right_win) left_win = vsplit_at(right_win, "left") @@ -725,6 +718,13 @@ local function show_diff(s, entry, focus_left) s.diff_left_win = left_win s.diff_right_win = right_win + -- Toggle diff off around the buffer swap so Vim tears down the old + -- diff group and re-establishes a fresh one against the new pair. + -- nvim_win_set_buf swaps the buffer pointer without invalidating + -- cached diff state, and :diffupdate alone doesn't reliably force a + -- recompute when no buffer contents have actually changed. + set_diff(left_win, false) + set_diff(right_win, false) vim.api.nvim_win_set_buf(left_win, pair.left.buf) vim.api.nvim_win_set_buf(right_win, pair.right.buf) for _, side in ipairs({ pair.left, pair.right }) do @@ -732,6 +732,15 @@ local function show_diff(s, entry, focus_left) diff.set_buf_name_and_filetype(side.buf, side.name) end end + -- Synthetic index/HEAD buffers never run BufRead, so modeline-only + -- filetypes aren't detected. Copy from the side that did resolve. + local left_ft = vim.bo[pair.left.buf].filetype + local right_ft = vim.bo[pair.right.buf].filetype + if left_ft == "" and right_ft ~= "" then + vim.bo[pair.left.buf].filetype = right_ft + elseif right_ft == "" and left_ft ~= "" then + vim.bo[pair.right.buf].filetype = left_ft + end set_diff(left_win, true) set_diff(right_win, true) s.last_shown_key = key @@ -897,6 +906,7 @@ local function open(worktree) worktree = worktree, lines = {}, sidebar_win = win, + invocation_win = previous_win, } local function k(lhs, rhs, desc)