Fix stale git diff chip and code review button#11242
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
Reviewed PR #11242, which prevents the GitDiffStats shell fallback from overwriting watcher-driven values after the git status watcher attaches, and clears repo-derived prompt/header state when leaving a repository.
Concerns
- No blocking correctness, spec-alignment, or security concerns found in the annotated diff. The PR description includes user-visible demo evidence for the behavior change.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // GitDiffStats has two value sources that can race when entering a repo: | ||
| // this shell fallback (`git diff --shortstat HEAD`, tracked changes only) | ||
| // and a repo-status watcher that also counts untracked files. If the | ||
| // watcher attached while this fallback was in flight, drop the fallback's | ||
| // result | ||
| if matches!(chip_kind, ContextChipKind::GitDiffStats) | ||
| && me.is_updated_externally(&chip_kind) | ||
| { | ||
| return; | ||
| } |
vkodithala
left a comment
vkodithala
left a comment
There was a problem hiding this comment.
This looks great, thanks for attaching a demo! Left a small note, non-blocking.
| // Redraw on repo exit so the code review button drops its diff annotation immediately. | ||
| self.refresh_pane_header(ctx); | ||
| ctx.emit(Event::TerminalViewStateChanged); | ||
| ctx.notify(); |
There was a problem hiding this comment.
Makes sense to me that we emit a TerminalViewStateChanged (since this updates the diff chip on the workspace header) and re-render the active terminal view - but a little confused at why we're refreshing the pane header? Is there any dependency between the pane header <> repo status?
There was a problem hiding this comment.
Thanks for the call out!
No need to refresh the pane header here, and also re-rendering the active terminal view is also redundant (set_git_repo_status already re-renders). Updated!
johnturcoo
deleted the
johnturco/remote-1699-git-diff-chip-not-updating-until-terminal-command-is-run
branch
## Description Resolves warpdotdev#11016 The `GitDiffStats` prompt chip and code review button could show stale state when entering or leaving a git repository. Problem 1 — Flicker on entering a repo: The chip has two value sources: a shell fallback (git diff --shortstat HEAD, tracked changes only) and a repo-status watcher that also counts untracked files. On cd into a repo there is a race that can cause untracked files to not appear in the diff chip. Problem 2 — Stale state on leaving a repo: When cd'ing out of a repository, the chip's last value and the code review button's diff annotation are not immediately cleared. This PR resolves both of these issues ## Changes - `fetch_chip_value_once()`: If the repo-status watcher attached while a GitDiffStats shell-fallback command was in flight, discard the fallback's result so it can't overwrite the watcher's accurate value. - `set_git_repo_status()`: On repo detach, immediately abort in-flight handlers and clear the cached GitDiffStats value, then signal a re-render. - `clear_git_repo_status()` : On repo exit, refresh the pane header and emit TerminalViewStateChanged so the code review button drops its diff annotation immediately. ## Linked Issue https://linear.app/warpdotdev/issue/REMOTE-1699/git-diff-chip-not-updating-until-terminal-command-is-run ## Testing - [x] I have manually tested my changes locally with `./script/run` ### Screenshots / Videos [Demo of issue](https://www.loom.com/share/42e8748176f04510a530727cb7fe7f61) (Note that the code review button also shows stale diffs upon leaving a directory) [Demo of fix ](https://www.loom.com/share/c1b06e3c8804498ab3d958570f6d0c71) (Untracked file changes now appear right away in the diff chip. Diff chip and code review button are both cleared upon exiting a repo) ## Changelog Entries for Stable CHANGELOG-BUG-FIX: Fixed the git diff prompt chip flickering on entering a repository, and stale git diff state on the prompt chip and code review button when leaving a repository.
2 participants
Description
Resolves #11016
The
GitDiffStatsprompt chip and code review button could show stale state when entering or leaving a git repository.Problem 1 — Flicker on entering a repo: The chip has two value sources: a shell fallback (git diff --shortstat HEAD, tracked changes only) and a repo-status watcher that also counts untracked files. On cd into a repo there is a race that can cause untracked files to not appear in the diff chip.
Problem 2 — Stale state on leaving a repo: When cd'ing out of a repository, the chip's last value and the code review button's diff annotation are not immediately cleared.
This PR resolves both of these issues
Changes
fetch_chip_value_once(): If the repo-status watcher attached while a GitDiffStats shell-fallback command was in flight, discard the fallback's result so it can't overwrite the watcher's accurate value.set_git_repo_status(): On repo detach, immediately abort in-flight handlers and clear the cached GitDiffStats value, then signal a re-render.clear_git_repo_status(): On repo exit, refresh the pane header and emit TerminalViewStateChanged so the code review button drops its diff annotation immediately.Linked Issue
https://linear.app/warpdotdev/issue/REMOTE-1699/git-diff-chip-not-updating-until-terminal-command-is-run
Testing
./script/runScreenshots / Videos
Demo of issue (Note that the code review button also shows stale diffs upon leaving a directory)
Demo of fix (Untracked file changes now appear right away in the diff chip. Diff chip and code review button are both cleared upon exiting a repo)
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Fixed the git diff prompt chip flickering on entering a repository, and stale git diff state on the prompt chip and code review button when leaving a repository.