Add inline diff highlighting and side by side rendering with bulk actions#517
Conversation
-add inline diff tokens with word + char-level refinement and size guards -render inline token spans in Apply View modified blocks -style inline add/remove highlights -add inline diff unit tests
-render modified blocks in a two-column grid -add responsive stacking and empty-side spacing styles
-gate char-level diff by word similarity to avoid odd partial highlights -merge added/removed tokens across whitespace for cleaner word-group highlights
-add padding/rounding to diff blocks and soften background intensity -add block spacing to prevent action overlap -pin accept badges inside block container
-change header actions to Accept All / Reject All with detailed tooltips -apply remaining changes correctly for accept/reject -keep paragraph spacing by tracking line counts in diff blocks
📝 WalkthroughWalkthroughThis PR introduces inline token-level diff rendering with a new tokenization system using longest common subsequence (LCS) and optional character-level diffs. The apply view's acceptance mechanism is refactored to replace single accept/reject actions with "Accept All Remaining" and "Reject All Remaining" options. DiffBlock types are extended with line counts and inline token arrays for both original and modified sides. CSS updates enable responsive grid layouts and container queries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@styles.css`:
- Around line 688-691: Empty diff placeholders are being coloured because
.smtcmp-diff-block.added/.removed override .smtcmp-diff-empty; change the rule
to a more specific selector so it wins when an element has both classes — e.g.
add a rule targeting .smtcmp-diff-block.added.smtcmp-diff-empty and
.smtcmp-diff-block.removed.smtcmp-diff-empty and set background: transparent
(avoid using !important).
🧹 Nitpick comments (1)
src/utils/chat/diff.ts (1)
176-193: Avoid UTF‑16 splitting in char-level diff.
split('')operates on UTF‑16 code units, which can split emoji/combining sequences and produce broken highlights. ConsiderArray.from(code points) orIntl.Segmenter(graphemes) for safer splitting.♻️ Suggested tweak (code points)
- const charOps = diffSequence(deletes.split(''), inserts.split('')) + const charOps = diffSequence(Array.from(deletes), Array.from(inserts))
| .smtcmp-diff-empty { | ||
| background: transparent; | ||
| min-height: var(--size-4-8); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "styles.css" -type fRepository: glowingjade/obsidian-smart-composer
Length of output: 91
🏁 Script executed:
wc -l ./styles.cssRepository: glowingjade/obsidian-smart-composer
Length of output: 96
🏁 Script executed:
sed -n '680,700p' ./styles.cssRepository: glowingjade/obsidian-smart-composer
Length of output: 456
🏁 Script executed:
rg '\.smtcmp-diff-block\.(added|removed)' ./styles.css -A 5Repository: glowingjade/obsidian-smart-composer
Length of output: 330
🏁 Script executed:
find . -type f \( -name "*.jsx" -o -name "*.js" -o -name "*.tsx" -o -name "*.ts" -o -name "*.html" \) | head -20Repository: glowingjade/obsidian-smart-composer
Length of output: 877
🏁 Script executed:
rg 'smtcmp-diff-empty' ./src --type ts --type tsx -B 3 -A 3Repository: glowingjade/obsidian-smart-composer
Length of output: 106
🏁 Script executed:
rg 'smtcmp-diff-empty' ./src -B 3 -A 3Repository: glowingjade/obsidian-smart-composer
Length of output: 646
🏁 Script executed:
rg 'smtcmp-diff-empty' ./src/components/apply-view/ApplyViewRoot.tsx -B 10 -A 10Repository: glowingjade/obsidian-smart-composer
Length of output: 678
Empty diff placeholders inherit added/removed tint due to CSS specificity.
When an empty diff block is rendered, it receives both .smtcmp-diff-block.added (or .removed) and .smtcmp-diff-empty classes. The compound selector .smtcmp-diff-block.added has specificity (0, 2, 0), while .smtcmp-diff-empty has only (0, 1, 0). This causes the colored background to override the transparent background.
🐛 Suggested CSS fix
-.smtcmp-diff-empty {
- background: transparent;
- min-height: var(--size-4-8);
-}
+.smtcmp-diff-block.smtcmp-diff-empty {
+ background-color: transparent;
+ min-height: var(--size-4-8);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .smtcmp-diff-empty { | |
| background: transparent; | |
| min-height: var(--size-4-8); | |
| } | |
| .smtcmp-diff-block.smtcmp-diff-empty { | |
| background-color: transparent; | |
| min-height: var(--size-4-8); | |
| } |
🤖 Prompt for AI Agents
In `@styles.css` around lines 688 - 691, Empty diff placeholders are being
coloured because .smtcmp-diff-block.added/.removed override .smtcmp-diff-empty;
change the rule to a more specific selector so it wins when an element has both
classes — e.g. add a rule targeting .smtcmp-diff-block.added.smtcmp-diff-empty
and .smtcmp-diff-block.removed.smtcmp-diff-empty and set background: transparent
(avoid using !important).
Description
This PR enhances the Apply view diff experience with several user-facing improvements: inline word/character highlighting for edits (with smarter similarity handling to avoid odd partial highlights), side‑by‑side diff rendering that collapses responsively in narrow panes, and clearer bulk actions that let you Accept All or Reject All remaining changes while preserving previously accepted/rejected blocks. It also refines the visual presentation with adjusted diff block styling and action placement, and ensures paragraph spacing is preserved when applying or rejecting changes.
Feat:
#478
Checklist before requesting a review
npm run lint:checkandnpm run type:check)npm run test)Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.