Keep current file selected after staging #1760
Conversation
|
I'm going to do my best to review all your PR's soon, just bear in mind I have three kids, two of which are six months old 😅 |
|
No worries! Take your time, I know how demanding twins can be. |
lua/neogit/integrations/diffview.lua
Outdated
| @@ -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) | |||
There was a problem hiding this comment.
it looks like this argument will always be nil? I don't see any callers passing it.
There was a problem hiding this comment.
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.
lua/neogit/integrations/diffview.lua
Outdated
| git.repo:dispatch_refresh { | ||
| source = "diffview_update", | ||
| callback = function() end, | ||
| } | ||
|
|
||
| local repo_state = git.repo.state | ||
| if not repo_state then | ||
| return files | ||
| end |
There was a problem hiding this comment.
Not sure about this - the git repo should always have state at this point. How would you get here with an uninitialized repository?
There was a problem hiding this comment.
This was just defensive programming and isn't actually needed in this context. I've removed it.
lua/neogit/integrations/diffview.lua
Outdated
| view:on_files_staged(function() | ||
| vim.schedule(function() | ||
| Watcher.instance():dispatch_refresh() | ||
| end) | ||
| end) |
There was a problem hiding this comment.
Can you explain why this needs to be scheduled, instead of non-blocking async? Not sure I follow.
There was a problem hiding this comment.
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.
This PR fixes #1448 where Diffview would jump back to the first file after saving.
To fix this in Neogit, I've updated the Diffview integration:
update_files, which then uses it to ensure the correct file remains selected.Note on a related Diffview issue:
I also noticed that Diffview itself doesn't always handle writing edits to staged files correctly (changes made in the diff view to a staged file might not reflect in the worktree, appearing lost on refresh). I've fixed this in my fork of Diffview.