feat: Allow for patch stage/unstage in Git diff viewer#585
feat: Allow for patch stage/unstage in Git diff viewer#585atreat wants to merge 20 commits intoDimillian:mainfrom
Conversation
…ining actionable hunks
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfee6d8a7e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let (diff_args, reverse_apply): (&[&str], bool) = match source { | ||
| "unstaged" => (&["diff", "--no-color", "-U0", "--"], false), | ||
| "staged" => (&["diff", "--cached", "--no-color", "-U0", "--"], true), | ||
| _ => unreachable!(), |
There was a problem hiding this comment.
When git_diff_ignore_whitespace_changes is enabled, get_git_diffs_inner computes displayHunkId values from git diff -w -U0 hunks, but this branch rebuilds the source patch without -w. In hunks that mix whitespace-only edits with real edits, the hunk header tuple (old_start/old_count/new_start/new_count) can differ, so build_display_hunk_patch cannot find the ID and returns “Display hunk no longer matches…”, breaking hunk stage/unstage in that mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d17749363c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if new_line < insertion_point { | ||
| break; | ||
| } | ||
| delta -= hunk.old_count as isize; |
There was a problem hiding this comment.
map_new_to_old_line_clamped handles staged deletion hunks (new_count == 0) by subtracting old_count from delta, but this mapper is used to convert unstaged hunk old-line numbers (index coordinates) back into display old-line numbers (HEAD coordinates). After a staged deletion above the target, the old-line number should move forward, not backward; with the current sign, later unstaged hunks are mapped to the wrong range and find_display_hunk_span can’t locate them, so some hunks in mixed staged/unstaged files never get actionable Stage controls.
Useful? React with 👍 / 👎.
| let mut args = vec!["diff"]; | ||
| if cached { | ||
| args.push("--cached"); |
There was a problem hiding this comment.
source_diff_for_path always shells out to regular git diff ... -- <path>, which compares index vs working tree. git diff -h shows untracked path diffs require the separate --no-index <path> <path> mode; without that, new files produce an empty patch. Since status still marks them as unstaged, these files end up with missing unstagedDiff/displayHunks, leaving line-level stage permanently unavailable for brand-new files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 627fbe5aa4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let mut args = vec!["diff"]; | ||
| if source == "staged" { | ||
| args.push("--cached"); | ||
| } | ||
| args.push("--no-color"); |
There was a problem hiding this comment.
--no-index when applying untracked display hunks
get_git_diffs_inner now creates display hunks for untracked files using no-index diffs, but this path rebuilds the source patch with plain git diff for source == "unstaged". For WT_NEW files that command returns an empty patch, so clicking the new Stage hunk action always errors with “No changes available for the requested display hunk.” and the feature is unusable for brand-new files.
Useful? React with 👍 / 👎.
| } else if body_line.starts_with(' ') { | ||
| old_cursor += 1; | ||
| new_cursor += 1; | ||
| } |
There was a problem hiding this comment.
This parser only tracks +, -, and context-prefixed lines and silently drops \ No newline at end of file hunk markers, so rebuilt patches from build_display_hunk_patch/build_selected_patch lose required metadata. In hunks touching files without trailing newlines, git apply --cached --unidiff-zero can no longer match the source hunk and stage/unstage fails.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if let Some(text) = body_line.strip_prefix('+') { | ||
| if !body_line.starts_with("+++") { |
There was a problem hiding this comment.
+++/--- content lines
The starts_with("+++")/starts_with("---") filters run inside hunk bodies, so real changed content that begins with those prefixes (for example an added line +++title, represented as ++++title in patch text) is discarded as if it were a file header. That undercounts hunk lines and can produce mismatched display-hunk IDs or incomplete patches when applying the action.
Useful? React with 👍 / 👎.
xkonjin
left a comment
xkonjin
left a comment
There was a problem hiding this comment.
Automated review focusing on bugs, security, and tests.
Overall: I reviewed the diff and did not spot obvious bugs or security risks. Please ensure tests cover new logic and edge cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f586b57c9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ParsedDisplayLineType::Add | ParsedDisplayLineType::Del => current.push((index, line)), | ||
| _ => { | ||
| if !current.is_empty() { |
There was a problem hiding this comment.
build_display_segments terminates a segment on every non-add/delete line, so ParsedDisplayLineType::Meta (\ No newline at end of file) splits what is logically one change hunk into separate pieces. For a replacement at EOF without trailing newline, the parsed display stream becomes del, meta, add, meta; find_display_hunk_span then cannot satisfy the source hunk's expected add+del counts and drops that display hunk entirely, so line-level Stage/Unstage controls disappear for those edits.
Useful? React with 👍 / 👎.
2 participants
Changes
Idea is to keep action buttons near the center of the scrollable region so see can skim through changes and quickly stage/unstage hunks.
Screenshots:
stage hunk:
unstage hunk:
unstage deletion only:
