-
Notifications
You must be signed in to change notification settings - Fork 21
ProjectDiffScreen: Better navigation, hunk reset, UX improvements, and file list display bugfixes #433
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.
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
base: main
Are you sure you want to change the base?
Conversation
2ffd0f0 to
4750b9c
Compare
Add configurable keymap to switch between file list and diff preview. When switching to diff, cursor jumps to first hunk.
Configurable alignment when jumping to hunks: 'top', 'center', 'bottom'. Defaults to 'center' for better context visibility.
Focus on changes the user hasn't reviewed yet. Typical workflow: stage some hunks, exit to fix something, then re-enter project diff. Cursor should resume at unstaged changes, not staged ones.
Previously, configure_row() set header_win_plot.row to row + 1, and HeaderElement:mount() compensated by subtracting 1. Now the logic is straightforward: header stays at original row, content is pushed down. Also adds documentation files for known issues and conventions.
When staging the last hunk in a file, jump to the next unstaged file rather than staying on the now-staged entry.
When unstaging the last hunk in a file, jump to the next staged file rather than staying on the now-unstaged entry.
Allows discarding individual hunks from the diff pane. Default keybinding 'r', buffer_reset moved to 'R'.
Without nowait, single-key bindings like 'd' can feel sluggish if the user has operator-pending mappings. Vim waits for potential follow-up keys (dd, dw, etc.) before executing.
Generalized navigation keybinds (default J/K): - In diff pane: navigate between hunks across all files - In file list: navigate between files (skipping folders) Both wrap around and jump to first/last hunk when crossing files.
35965a1 to
fbb0849
Compare
|
Rebased. Would you like me to split anything out into separate PRs? |
fbb0849 to
81d6612
Compare
Appreciate all your efforts, one PR should be good enough. Apologies for the delay on the review as I have been away travelling. I should have some time to sit down and review it this coming weekend! |
The diff view is a projection of the file's state. Quitting from the diff pane now jumps to the corresponding file position, keeping context stable.
Store vgit_last_entry_type buffer variable on quit so re-opening returns to the same staged/unstaged entry for the file.
`generate_unified_deleted` and `generate_split_deleted` crashed when passed empty hunks. Add early return guards matching the regular diff generators.
- Hunk operations now stay at same index (the next hunk) - File operations now go to next file in order, not first file
81d6612 to
d8b55c0
Compare
|
Removed a commit which fixed a visual display issue when a tabline exists in the buffers 'behind' the project diff screen, as it appears to introduce other visual issues. This one is especially hard to fix, still iterating on it. No rush at all on the review :) |
|
Managed to fix the display issue. That was a tough one. 😅 Pushed the commit, and included some screenshots of before / after. |
Hunk operations (stage/unstage/reset) stay at the same index position, effectively moving to the next hunk. When a file is fully staged or unstaged, jump to the next file of the same type rather than the first.
Dispatch sync event to git_buffer_store when exiting to a file, ensuring gutter signs reflect any staging changes made in the screen.
When showtabline > 0, Neovim's floating windows with relative='editor' don't properly account for the tabline taking up screen space. This causes the first line(s) of content to be hidden behind the tabline. The fix prepends empty "padding" lines to buffer content when the tabline is visible: - FoldableListComponent (file list): 1 padding line - DiffView (diff pane): 2 padding lines (extra line because the HeaderElement pushes content down) All extmark positions and navigation functions are adjusted by the padding offset to maintain correct behavior.
02cf880 to
acdfc63
Compare
tanvirtin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all these improvements. Loving the new J/K keybinds.
Left some small comments otherwise looking good!
| desc = 'Unstage hunk' | ||
| }, | ||
| buffer_hunk_reset = { | ||
| key = 'r', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| key = 'r', | |
| key = 'gr', |
| }, | ||
| buffer_reset = { | ||
| key = 'r', | ||
| key = 'R', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this as r and have buffer_hunk_reset be gr to avoid mapping conflict with reset_all and keymap consistency for hunk commands being prefixed by g.
| key = 'R', | |
| key = 'r', |
| local found = self:move_to(function(_, entry_type) | ||
| return entry_type == 'unstaged' | ||
| end) | ||
| if not found then | ||
| self:move_to(function(status) | ||
| return status.filename == filename | ||
| end) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition for next_file should cover moving to the next unstaged file (when there are NO other unstaged files and we just staged the last unstaged hunk).
Let me know otherwise, then we can also apply it to all other similar methods.
| local found = self:move_to(function(_, entry_type) | |
| return entry_type == 'unstaged' | |
| end) | |
| if not found then | |
| self:move_to(function(status) | |
| return status.filename == filename | |
| end) | |
| end | |
| self:move_to(function(status) | |
| return status.filename == filename | |
| end) |
| local new_entry = self.model:get_entry() | ||
| if new_entry | ||
| and new_entry.type == 'unstaged' | ||
| and new_entry.status.filename == filename then | ||
| local diff = self.model:get_diff() | ||
| if diff and diff.marks and #diff.marks > 0 then | ||
| local target = math.min(hunk_index, #diff.marks) | ||
| local hunk_alignment = project_diff_preview_setting:get('hunk_alignment') | ||
| self.diff_view:move_to_hunk(target, hunk_alignment) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has some potential to become a method for reusability.
| -- Compensates for tabline taking screen space but not being accounted for | ||
| -- in floating window positioning with relative='editor'. Needs 2 lines because | ||
| -- the diff content is pushed down by the HeaderElement. | ||
| function DiffView:get_tabline_padding() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.


ProjectDiffScreenUX Improvements, Features, and FixesDemo
Video link
In this demo you can see me pressing tab to toggle the cursor between the file list and the diff pane, using the next and prev bindings in the diff pane to move between hunks, use the next and prev bindings in the file list to move between files, and various changes which make it easy to stage a bunch of hunks quickly simply by pressing s in the diff pane. When opening the project diff screen, your curser is put over the first unstaged diff, so you can begin staging quickly; once all hunks in a file have been staged, you automatically jump to the first unstaged diff in the next file. The overall goal is to make it super fast and seamless to stage a bunch of hunks quickly.
Commits
feat(ProjectDiffScreen): Add Tab to toggle focus
Add configurable keymap to switch between file list and diff preview. When switching to diff, cursor jumps to first hunk.
feat(ProjectDiffScreen): Add hunk_alignment setting
Configurable alignment when jumping to hunks:
'top','center','bottom'. Defaults to'center'for better context visibility.feat(ProjectDiffScreen): Prefer unstaged entries
Focus on changes the user hasn't reviewed yet. Typical workflow: stage some hunks, exit to fix something, then re-enter project diff. Cursor should resume at unstaged changes, not staged ones.
fix: Simplify header row positioning logic
Previously,
configure_row()setheader_win_plot.rowtorow + 1, andHeaderElement:mount()compensated by subtracting 1. Now the logic is straightforward: header stays at original row, content is pushed down.feat: Jump to next unstaged file after staging
When staging the last hunk in a file, jump to the next unstaged file rather than staying on the now-staged entry.
feat: Jump to next staged file after unstaging
When unstaging the last hunk in a file, jump to the next staged file rather than staying on the now-unstaged entry.
feat(ProjectDiffScreen): Add reset_hunk command
Allows discarding individual hunks from the diff pane. Default keybinding
r,buffer_resetmoved toR.feat(keymap): Add nowait to buffer-local keymaps
Without
nowait, single-key bindings likedcan feel sluggish if the user has operator-pending mappings. Vim waits for potential follow-up keys (dd,dw, etc.) before executing.feat(ProjectDiffScreen): Add next/previous nav
Generalized navigation keybinds (default
J/K):Both wrap around and jump to first/last hunk when crossing files.
feat(ProjectDiffScreen): Jump to file position on quit
The diff view is a projection of the file's state. Quitting from the diff pane now jumps to the corresponding file position, keeping context stable.
feat: Remember entry type across sessions
Store
vgit_last_entry_typebuffer variable on quit so re-opening returns to the same staged/unstaged entry for the file.fix: Handle empty hunks in deleted file generators
generate_unified_deletedandgenerate_split_deletedcrashed when passed empty hunks. Add early return guards matching the regular diff generators.fix(ProjectDiffScreen): Stay at next hunk/file after staging
Hunk operations (stage/unstage/reset) stay at the same index position, effectively moving to the next hunk. When a file is fully staged or unstaged, jump to the next file of the same type rather than the first.
fix(ProjectDiffScreen): Refresh gutter signs on quit
Dispatch sync event to git_buffer_store when exiting to a file, ensuring gutter signs reflect any staging changes made in the screen.
fix: Content padding for tabline offset in floating windows
When
showtabline > 0, floating windows withrelative='editor'don't account for the tabline taking screen space, causing the first line(s) of content to be hidden. The fix prepends empty padding lines to buffer content when the tabline is visible (1 line for file list, 2 for diff pane).Before
"Staged Changes" header missing, can't see line 1 in the diff view
After
Can see "Staged Changes" header, as well as line 1 in the diff view