Git diff: commit history + diff-in-a-tab reshape (#117)#1534
Open
adradr wants to merge 44 commits into
Open
Conversation
|
| Filename | Overview |
|---|---|
| packages/app/src/panels/diff-panel.tsx | New 921-line diff tab panel: renders collapsible file sections, owns inline review wiring per DiffTarget, and hosts the toolbar. Has a P1 bug where commit review keys vary with the whitespace preference, causing drafted comments to disappear on toggle. |
| packages/app/src/git/use-diff-files.ts | New unified hook that resolves any DiffTarget (working or commit) to ParsedDiffFile[]. Correctly keeps both hook branches always called; fans out per-file commit queries via useQueries; no issues found. |
| packages/server/src/utils/checkout-git.ts | Adds listCheckoutCommits and getCommitFileDiff. Git output is parsed with RS/NUL delimiters to avoid conflicts with arbitrary commit subjects; path injection is guarded before reaching git show; binary-only diffs return null. No issues found. |
| packages/server/src/server/session/checkout/checkout-session.ts | Adds handlers for checkout.commits.list and checkout.commits.file_diff. Validates SHA with assertSafeGitRef and path with a new assertSafeCommitFilePath (no empty, no absolute, no .. segments). No issues found. |
| packages/app/src/workspace-tabs/identity.ts | Extends tab identity with the new "diff" kind. focusPath is correctly excluded from identity (deduplication) but included in workspaceTabTargetsFullyEqual for refresh detection. Clean design. |
| packages/app/src/stores/workspace-layout-actions.ts | Adds stripEphemeralTabsFromLayout to drop commit diff tabs before persisting; uses workspaceTabTargetsFullyEqual in updateExistingTabTarget so focusPath-only re-opens correctly scroll to the new file. No issues found. |
| packages/app/src/git/commits-section/commits-section.tsx | New commits sidebar section with collapsible list and resizable bottom drawer. Drag resize uses pointer capture and DOM listeners correctly cleaned up on unmount. No issues found. |
| packages/app/src/review/store.ts | Adds optional commitSha to review draft key and attachment snapshot, appended at the end so legacy keys remain byte-identical. Additive and backwards-compatible. |
| packages/app/src/attachments/workspace-attachments-store.ts | Adds multi-source support (keyed by sourceKey) to attachment scopes so each diff tab can register its own review attachment without clobbering the sidebar's. Maintains a flattened cache to keep selector references stable. No issues found. |
| packages/protocol/src/messages.ts | Adds CheckoutCommitsList and CheckoutCommitFileDiff request/response schemas, exports ParsedDiffFile from the protocol package, and adds commitsList capability flag. All are additive; existing peers ignore them. |
| packages/app/src/git/diff-target.ts | New minimal module defining DiffTarget discriminated union and diffTargetKey for stable tab identity. ignoreWhitespace is deliberately excluded from the key. Clean, well-tested. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Sidebar as Changes Sidebar
participant Store as WorkspaceLayoutStore
participant Panel as DiffPanel Tab
participant Server as CheckoutSession
Sidebar->>Store: openTabFocused diff target + focusPath
Store-->>Panel: mount DiffPanelBody
alt Working diff
Panel->>Server: useCheckoutDiffQuery existing RPC
Server-->>Panel: ParsedDiffFile array
else Commit diff
Panel->>Server: listCheckoutCommits
Server-->>Panel: commit file list
Panel->>Server: getCommitFileDiff per file parallel
Server-->>Panel: ParsedDiffFile per file
end
Panel->>Panel: buildReviewDraftKey with commitSha
Panel->>Panel: useInlineReviewController reviewDraftKey
Panel->>Store: setWorkspaceAttachments sourceKey equals reviewDraftKey
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Sidebar as Changes Sidebar
participant Store as WorkspaceLayoutStore
participant Panel as DiffPanel Tab
participant Server as CheckoutSession
Sidebar->>Store: openTabFocused diff target + focusPath
Store-->>Panel: mount DiffPanelBody
alt Working diff
Panel->>Server: useCheckoutDiffQuery existing RPC
Server-->>Panel: ParsedDiffFile array
else Commit diff
Panel->>Server: listCheckoutCommits
Server-->>Panel: commit file list
Panel->>Server: getCommitFileDiff per file parallel
Server-->>Panel: ParsedDiffFile per file
end
Panel->>Panel: buildReviewDraftKey with commitSha
Panel->>Panel: useInlineReviewController reviewDraftKey
Panel->>Store: setWorkspaceAttachments sourceKey equals reviewDraftKey
Reviews (6): Last reviewed commit: "feat(app): make the on-remote commit dot..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…d-commits # Conflicts: # packages/server/src/server/session.ts
…d-commits # Conflicts: # packages/app/src/git/diff-pane.tsx # packages/protocol/src/messages.ts # packages/server/src/server/session.ts # packages/server/src/server/websocket-server.ts
…move to the diff tab
…ed per-commit view
…d-commits # Conflicts: # packages/server/src/server/session/checkout/checkout-session.test.ts # packages/server/src/server/session/checkout/checkout-session.ts
adradr
changed the title
Re-adds the resize handle removed in the sidebar reshape, grafted onto the listing-only commits section (rows still open diff tabs, no inline file expansion). Desktop-web only; height persists via commitsSectionHeight (default 240) and clamps to keep a 160px diff reserve. Render path guards finiteness + lower bound, and a re-entrancy guard ignores a second concurrent pointerdown.
Dims the filled green remote dot (row + legend) to ~0.55 opacity so the local-only ring stays the state that draws the eye.
Comment on lines
+356
to
+367
| focusPath, | ||
| }: { | ||
| serverId: string; | ||
| workspaceId: string; | ||
| cwd: string; | ||
| target: DiffTarget; | ||
| focusPath?: string; | ||
| }) { | ||
| const { t } = useTranslation(); | ||
| const { settings } = useAppSettings(); | ||
| const { preferences, updatePreferences } = useChangesPreferences(); | ||
| const isCompact = useIsCompactFormFactor(); |
There was a problem hiding this comment.
resolveReviewTargetParams returns ignoreWhitespace: hideWhitespace for commit targets even though getCommitFileDiff has no whitespace parameter — the commit diff content is identical regardless of the preference. As a result, every time a user clicks the "hide whitespace" toolbar button while a commit diff tab is open, reviewDraftKey (which encodes ignoreWhitespace) changes, useInlineReviewController re-subscribes to an empty new key, and all drafted inline comments disappear from the tab and from the composer attachment. The comments are not lost (they remain in the store under the old key) but they are no longer reachable through the current view. Any review the user sends after the toggle contains an empty attachment even if they had added comments before it.
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
1 participant
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.
Adds commit history + a reshaped diff experience to the git Changes view, following the direction in the maintainer's reply (diff moves into a tab, sidebar stays listing-only).
The diff lives in a Tab now
The actual diff moved out of the sidebar into a workspace Tab, so the same surface can show different diff targets:
The diff tab reuses the existing tab system (like the browser/file tabs) — desktop split-pane and compact full-screen come for free. One stable working tab; one tab per commit (re-clicking a commit focuses its tab). Commit tabs are ephemeral on reload.
The sidebar is listing-only
The Changes sidebar is now just a list:
+/-stats (toggle).◌) or pushed (●).No inline diffs, no per-commit file tree — to see what's in a commit you click it and view the detailed diff tab. The unified/split + wrap + whitespace controls moved into the diff tab's toolbar.
Inline comments
Inline review comments live in the diff tab, for both working changes and commit diffs (commit comments are namespaced by SHA), and flow to the composer like the existing review flow.
Notes
listCheckoutCommits+ per-file diff into the same renderer the working diff uses, so they look identical.checkout.commits.*) are capability-gated; older hosts simply don't show the commit list.Refs #117