feat: simplify diff views to side-by-side and split modes with word-level highlighting #2108
Conversation
Add a toggle to switch between inline and side-by-side diff views in the ApplyView component. The side-by-side view shows original content on the left and modified content on the right, with word-level diff highlighting. - Add diffViewMode setting to persist user preference - Add Tabs UI in header to switch between views - Create SideBySideBlock component for two-column diff display - Support word-level highlighting for paired add/remove lines Co-Authored-By: Claude Opus 4.5 <[email protected]>
![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: f160da5cb9
ℹ️ 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".
| const removedLines = block.filter((c) => c.removed); | ||
| const addedLines = block.filter((c) => c.added); | ||
| const unchangedLines = block.filter((c) => !c.added && !c.removed); | ||
|
|
||
| // If there are no changes, just show the unchanged content on both sides | ||
| if (removedLines.length === 0 && addedLines.length === 0) { | ||
| return ( | ||
| <div className="tw-grid tw-grid-cols-2 tw-gap-2"> | ||
| <div className="tw-rounded-md tw-border tw-border-solid tw-border-border tw-bg-primary tw-p-2"> | ||
| {unchangedLines.map((change, idx) => ( | ||
| <div | ||
| key={idx} | ||
| className="tw-whitespace-pre-wrap tw-font-mono tw-text-sm tw-text-normal" | ||
| > | ||
| {change.value} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <div className="tw-rounded-md tw-border tw-border-solid tw-border-border tw-bg-primary tw-p-2"> | ||
| {unchangedLines.map((change, idx) => ( | ||
| <div | ||
| key={idx} | ||
| className="tw-whitespace-pre-wrap tw-font-mono tw-text-sm tw-text-normal" | ||
| > | ||
| {change.value} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Build paired lines for side-by-side comparison | ||
| const maxPairs = Math.max(removedLines.length, addedLines.length); | ||
| const pairedRows: { original: Change | null; modified: Change | null }[] = []; | ||
|
|
||
| for (let i = 0; i < maxPairs; i++) { | ||
| pairedRows.push({ | ||
| original: removedLines[i] || null, | ||
| modified: addedLines[i] || null, | ||
| }); |
There was a problem hiding this comment.
The side‑by‑side view loses the original order of changes by splitting the block into removedLines and addedLines and pairing by index. For diffs where multiple deletions occur before insertions (e.g., delete two lines, then add one unrelated line), this will pair unrelated lines and show a word‑level “replacement” that never happened. That can mislead users into accepting/rejecting the wrong change because the visual diff is incorrect. A safer approach is to iterate the original block in order and build rows that preserve sequence, inserting placeholders when a removal/addition has no immediate counterpart.
Useful? React with 👍 / 👎.
Fix issue where splitting block into removedLines and addedLines and pairing by index would incorrectly pair unrelated lines. Now iterates through the block in order and only pairs removed+added when they are adjacent (indicating an actual replacement). Co-Authored-By: Claude Opus 4.5 <[email protected]>
wenzhengjiang
changed the title
…evel highlighting - Remove inline diff mode, keeping only side-by-side and split modes - Add word-level highlighting to both modes for clearer change visualization - Split mode shows original and modified content separately with highlighted words - Side-by-side mode pairs adjacent changes with word-level diff - Default to split mode for cleaner diff view on large changes - Add migration for users who had inline mode saved in settings Co-Authored-By: Claude Opus 4.5 <[email protected]>
wenzhengjiang
force-pushed
the
feature/side-by-side-diff-view
branch
from
a727273 to
415fc04
Compare
|
@codex Review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 415fc0490e
ℹ️ 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".
- Add JSDoc documentation to the wordLevelDiff function to comply with project coding standards - Add nullish coalescing fallback for diffViewMode setting so users with pre-existing settings get a deterministic default value Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Extract buildDiffRows function to eliminate duplicate row-building logic - Create WordDiffSpan and DiffCell components for unified diff rendering - Simplify tokenizer using regex-based split for better performance - Add useMemo to prevent unnecessary recalculations - Reduce file from ~813 to ~707 lines (~13% reduction) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5bde9bda8
ℹ️ 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".
| } else if (current.removed) { | ||
| // Check if next item is an added chunk (replacement pair) | ||
| const next = block[i + 1]; | ||
| if (next?.added) { | ||
| // Split both chunks into lines and pair by index |
There was a problem hiding this comment.
The pairing logic assumes each Change.value is a complete line chunk and simply pairs a removed entry with the next added entry. However, the diff source uses diffTrimmedLines(..., { newlineIsToken: true }) (see src/tools/ComposerTools.ts), which emits standalone "\n" changes between line tokens. In that case the "removed newline" token becomes the immediate predecessor of the added line, so this code pairs the newline with the added line and leaves the actual removed line unpaired. The result is misaligned word-level highlighting and spurious blank rows whenever a line changes (e.g., a replacement displays as empty→new line). Consider collapsing newline tokens into the line values or disabling newlineIsToken before building rows.
Useful? React with 👍 / 👎.
Split Mode
Side-by-side Mode