Skip to content

Keep current file selected after staging #1760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aitai wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Keep current file selected after staging #1760

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a57913
fix(diffview): Keep current file selected after staging
Aitai Jun 7, 2025
094a432
fix(diffview): Use correct git path
Aitai Jun 7, 2025
4ccbb92
fix(diffview): Refresh git state before updating files
Aitai Jun 8, 2025
7ade26e
fix formatting
Aitai Jun 8, 2025
0e85360
fix(diffview): correctly handle file selection on refresh
Aitai Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions lua/neogit/integrations/diffview.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ local dv_utils = require("diffview.utils")

local Watcher = require("neogit.watcher")
local git = require("neogit.lib.git")
local a = require("plenary.async")

local function get_local_diff_view(section_name, item_name, opts)
local left = Rev(RevType.STAGE)
Expand All @@ -18,23 +17,33 @@ local function get_local_diff_view(section_name, item_name, opts)
section_name = "working"
end

local function update_files()
local function update_files(current_file_path)

@CKolkey CKolkey Jun 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like this argument will always be nil? I don't see any callers passing it.

@Aitai Aitai Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, that argument is never a file path as its name implied. My initial implementation incorrectly assumed I needed to manage the selected state during refreshes. I later realized diffview actually passes its view object to the update callback, and the old code was only working due to a subtle side-effect: the truthy view object prevented my fallback logic from running, which implicitly preserved the selection.

I've made a new commit which aligns correctly with the diffview API. The selected flag is now set only on the initial file list passed to the constructor. The update_files function acts as a pure data provider for refreshes, which properly leaves the selection management to diffview itself.

local files = {}

git.repo:dispatch_refresh {
source = "diffview_update",
callback = function() end,
}

local repo_state = git.repo.state
if not repo_state then
return files
end

@CKolkey CKolkey Jun 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this - the git repo should always have state at this point. How would you get here with an uninitialized repository?

@Aitai Aitai Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just defensive programming and isn't actually needed in this context. I've removed it.


local sections = {
conflicting = {
items = vim.tbl_filter(function(item)
return item.mode and item.mode:sub(2, 2) == "U"
end, git.repo.state.untracked.items),
end, repo_state.untracked and repo_state.untracked.items or {}),
},
working = git.repo.state.unstaged,
staged = git.repo.state.staged,
working = repo_state.unstaged or { items = {} },
staged = repo_state.staged or { items = {} },
}

for kind, section in pairs(sections) do
files[kind] = {}

for idx, item in ipairs(section.items) do
for idx, item in ipairs(section.items or {}) do
local file = {
path = item.name,
status = item.mode and item.mode:sub(1, 1),
Expand All @@ -44,7 +53,9 @@ local function get_local_diff_view(section_name, item_name, opts)
} or nil,
left_null = vim.tbl_contains({ "A", "?" }, item.mode),
right_null = false,
selected = (item_name and item.name == item_name) or (not item_name and idx == 1),
selected = (current_file_path and item.name == current_file_path)
or (item_name and item.name == item_name)
or (not item_name and not current_file_path and idx == 1),
}

-- restrict diff to only a particular section
Expand Down Expand Up @@ -84,11 +95,11 @@ local function get_local_diff_view(section_name, item_name, opts)
end,
}

view:on_files_staged(a.void(function(_)
Watcher.instance():dispatch_refresh()
view:update_files()
end))

view:on_files_staged(function()
vim.schedule(function()
Watcher.instance():dispatch_refresh()
end)
end)

@CKolkey CKolkey Jun 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this needs to be scheduled, instead of non-blocking async? Not sure I follow.

@Aitai Aitai Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally added vim.schedule because I was concerned about a potential race condition and thought I needed to manually defer the async refresh until after diffview's synchronous operations were complete.

However, I realized that dispatch_refresh already uses plenary.a.void internally.

dv_lib.add_view(view)

return view
Expand Down
Loading