fix: show the empty state when the selected file leaves the working copy - #31
Merged
Merged
Conversation
`resolveSection` returns null once the path is in no list — its last change was committed or discarded, so the file is no longer part of the working copy. But `diffKey` tested only `selectedFile`, so it stayed non-empty and the effect fetched an unstaged diff for a file with nothing to diff. No row could highlight, since the row test already requires a matching section, while the pane rendered an empty diff instead of "Select a file to view its diff." Gate both on `section !== null` rather than clearing the selection: no state written from a derived or an effect, and it keeps the "follow the file wherever it went" behaviour the section resolution exists for — if the path comes back, the selection resumes instead of having been silently dropped. Reported by Codex review on the v0.4.0 promotion (#30). Verified by reading plus `npm run check`; there is no WorkingCopyView test harness, and the path is behind `isTauri()` so the browser preview cannot reach it either. npm run check 530 files 0 errors; npm test 388 passed; cargo test 232 + 7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported by Codex review on the v0.4.0 promotion (#30).
nextis protected by a ruleset requiring a PR, so the fix lands here and #30 picks it up.The bug
resolveSectionreturnsnullonce the selected path is in no list — its last change was committed or discarded, so the file has left the working copy. ButdiffKeytested onlyselectedFile:so the key stayed non-empty and the effect fetched an unstaged diff for a file with nothing to diff. The pane then rendered an empty diff instead of the nothing-selected state, while no row could highlight — the row test at
:180/:187already requires a matching section, which is the one place that got it right.(Small correction to the report: it says the template tests only
selectedFile. That is true of the pane branch at:573, but not of the row highlight, which does checksection. The net effect described is right.)The fix
Gate
diffKeyand the pane branch onsection !== null.I took that option over the other one offered — clearing the selection — for two reasons:
Verification
npm run check530 files / 0 errors ·npm test388 passed ·cargo test232 + 7.Being straight about the limits: there is no
WorkingCopyViewtest harness, and the diff fetch sits behindisTauri(), so neither vitest nor the browser preview can exercise this path. The change is two conditions verified by reading and the type gate. Worth a click in a running build — select a file, discard its last change, and confirm the pane says "Select a file to view its diff." rather than showing an empty diff.🤖 Generated with Claude Code