docs(diff): design for side-by-side diff view with syntax and word highlighting
This commit is contained in:
mika kuns
@@ -0,0 +1,225 @@
|
||||
# Diff Viewer: Side-by-Side, Syntax Highlighting, Word Diff
|
||||
|
||||
Date: 2026-08-07
|
||||
Status: approved (design), not implemented
|
||||
|
||||
## Problem
|
||||
|
||||
The diff viewer renders every change as a flat unified stream. Reading what actually changed
|
||||
inside a modified line means mentally aligning a `−` row with a `+` row several rows below it.
|
||||
The user reads diffs faster side by side.
|
||||
|
||||
Three gaps, all in the same surface:
|
||||
|
||||
1. No side-by-side mode.
|
||||
2. No syntax highlighting — the merge editor (`ConflictResolverView`) already has it via
|
||||
TextMate, the diff viewer does not.
|
||||
3. No intra-line (word) highlighting, so a one-character change looks like a whole-line rewrite.
|
||||
|
||||
## Current state
|
||||
|
||||
| Concern | Where |
|
||||
|---|---|
|
||||
| Parsing | `src/ClaudeDo.Ui/ViewModels/Modals/UnifiedDiffParser.cs` → `DiffFileViewModel.Lines` |
|
||||
| Models | `src/ClaudeDo.Ui/ViewModels/Modals/DiffModels.cs` (`DiffLineViewModel{Kind,OldNo,NewNo,Text}`, `DiffLineKind{Add,Del,Ctx,File}`) |
|
||||
| Rendering | `src/ClaudeDo.Ui/Views/Controls/DiffLinesView.axaml` — non-virtualized `ItemsControl`, one `Border`+`Grid`+4 `TextBlock`s per line, `TextWrapping="NoWrap"` |
|
||||
| Host | `src/ClaudeDo.Ui/Views/Modals/DiffViewerView.axaml` — Files mode (line 154, `SelectedFile.Lines`) and Planning mode (line 162, flattened `DiffLines` across all files) |
|
||||
| Highlighting reference | `src/ClaudeDo.Ui/Views/Conflicts/ConflictResolverView.axaml.cs:80-83` — `RegistryOptions(ThemeName.DarkPlus)` + `InstallTextMate` + `SetGrammar` by file extension |
|
||||
|
||||
`Avalonia.AvaloniaEdit`, `AvaloniaEdit.TextMate` and `TextMateSharp.Grammars` are already
|
||||
referenced in `ClaudeDo.Ui.csproj`.
|
||||
|
||||
## Decision
|
||||
|
||||
Replace `DiffLinesView` with an AvaloniaEdit-based control. TextMate highlighting is bound to
|
||||
the `TextEditor` control; it cannot be lifted into `TextBlock` inlines without reimplementing
|
||||
the scope→brush layer that `AvaloniaEdit.TextMate` already provides. Building split/wrap/word
|
||||
diff on the `TextBlock` model first and swapping the renderer later would be throwaway work.
|
||||
|
||||
Side effect worth having: AvaloniaEdit virtualizes, which removes the current non-virtualized
|
||||
`ItemsControl` as a scaling limit on large diffs.
|
||||
|
||||
Rejected: keeping the `ItemsControl` and hand-rolling highlighting from `TextMateSharp`
|
||||
tokenization — same output, materially more code, and a second highlighting path to maintain
|
||||
alongside the merge editor's.
|
||||
|
||||
## Layout semantics
|
||||
|
||||
Left pane is the **old** state, right pane is the **new** state — each side carries the complete
|
||||
version of the hunk, not "removals here, additions there".
|
||||
|
||||
```
|
||||
LEFT (old) RIGHT (new)
|
||||
12 public void Save() 12 public void Save()
|
||||
13 var x = 1; 13 var x = 2; ← word diff on `1` / `2`
|
||||
14 Log("old"); · (filler)
|
||||
· (filler) 14 Log("new");
|
||||
15 } 15 }
|
||||
```
|
||||
|
||||
## Components
|
||||
|
||||
### 1. `DiffAlignment` (new, pure)
|
||||
|
||||
`src/ClaudeDo.Ui/ViewModels/Modals/DiffAlignment.cs`
|
||||
|
||||
Turns `IReadOnlyList<DiffLineViewModel>` into a render-ready `AlignedDiff`. No Avalonia types,
|
||||
fully unit-testable.
|
||||
|
||||
```csharp
|
||||
public enum AlignedSide { Ctx, Del, Add, Filler, Gap }
|
||||
|
||||
public readonly record struct TextSpan(int Start, int Length);
|
||||
|
||||
public sealed record SplitRow(
|
||||
AlignedSide LeftKind, int? OldNo, string LeftText, IReadOnlyList<TextSpan> LeftSpans,
|
||||
AlignedSide RightKind, int? NewNo, string RightText, IReadOnlyList<TextSpan> RightSpans);
|
||||
|
||||
public sealed record UnifiedRow(
|
||||
AlignedSide Kind, int? OldNo, int? NewNo, string Text, IReadOnlyList<TextSpan> Spans);
|
||||
|
||||
public sealed record AlignedDiff(
|
||||
IReadOnlyList<SplitRow> SplitRows, string LeftText, string RightText,
|
||||
IReadOnlyList<UnifiedRow> UnifiedRows, string UnifiedText);
|
||||
```
|
||||
|
||||
Row index `i` maps to document line `i + 1` in the corresponding text. That mapping is the
|
||||
contract the margin and both renderers depend on.
|
||||
|
||||
**Pairing.** Walk the lines. A `Ctx` run emits rows with the same text on both sides. A change
|
||||
block (a `Del` run followed by an `Add` run) pairs index-wise up to `min(delCount, addCount)`;
|
||||
the overhang gets `Filler` rows on the opposite side.
|
||||
|
||||
**Gaps.** The parser drops `@@` headers, so a skipped region shows up as a jump in `OldNo`/`NewNo`
|
||||
between consecutive lines. `DiffAlignment` detects that jump and inserts a `Gap` row on both
|
||||
sides. The parser is not touched.
|
||||
|
||||
**Word diff.** Only for `(Del, Add)` rows that are paired 1:1. Tokenize each side into runs of
|
||||
word characters / whitespace / single punctuation, run an LCS over the tokens, and emit the
|
||||
changed token runs as character spans per side.
|
||||
|
||||
Two guards, both `const` and both covered by tests:
|
||||
- Skip when either side exceeds `MaxWordDiffChars = 2000` — LCS cost, and such lines are
|
||||
unreadable as word diffs anyway.
|
||||
- Skip when token similarity is below `MinWordDiffSimilarity = 0.5` (common tokens / max token
|
||||
count). Below that the two lines are unrelated rewrites and per-word tinting is noise.
|
||||
|
||||
### 2. `DiffTextView` (new control, replaces `DiffLinesView`)
|
||||
|
||||
`src/ClaudeDo.Ui/Views/Controls/DiffTextView.axaml` + `.axaml.cs`
|
||||
|
||||
Styled properties:
|
||||
|
||||
| Property | Type | Meaning |
|
||||
|---|---|---|
|
||||
| `File` | `DiffFileViewModel?` | Source; the control aligns it and caches the `AlignedDiff` per file instance |
|
||||
| `Mode` | `DiffViewMode` (`Unified`\|`Split`) | Layout |
|
||||
| `WrapLines` | `bool` | Bound to each editor's `WordWrap` |
|
||||
|
||||
Two `TextEditor`s in a two-column grid, both `IsReadOnly=true`, `ShowLineNumbers=false`.
|
||||
`Unified` mode collapses the right editor and spans the left one across both columns, feeding
|
||||
it `UnifiedText`. `Split` mode shows both, fed `LeftText` / `RightText`.
|
||||
|
||||
Per editor:
|
||||
- **TextMate**: one shared `RegistryOptions(ThemeName.DarkPlus)`; grammar resolved from
|
||||
`File.Path`'s extension via `GetLanguageByExtension` → `GetScopeByLanguageId` → `SetGrammar`,
|
||||
exactly as `ConflictResolverView.ApplyGrammar` does. No extension match → no grammar, plain text.
|
||||
- **`DiffLineNumberMargin : AbstractMargin`** — draws line numbers from the row list. Split: old
|
||||
numbers left, new numbers right. Unified: two number columns in one margin. `Filler` and `Gap`
|
||||
rows draw nothing.
|
||||
- **`DiffLineBackgroundRenderer : IBackgroundRenderer`** — full-width tint per visual line by
|
||||
row kind: add / del / filler / gap / ctx.
|
||||
- **`WordDiffRenderer : IBackgroundRenderer`** — stronger tint over the changed spans, via
|
||||
`BackgroundGeometryBuilder` at `lineStartOffset + span.Start`.
|
||||
|
||||
Both renderers resolve rows through a single `Func<int, RowInfo?>` keyed by document line.
|
||||
|
||||
**Highlighting on fragments.** Only hunks are in the document, not whole files, so TextMate's
|
||||
line-by-line state can be wrong at a fragment boundary (a line inside a block comment may be
|
||||
highlighted as code). Accepted — the same is true of every fragment-based diff viewer.
|
||||
|
||||
**Colors.** Line tints stay the existing low-alpha `RunningTintBrush` / `ErrorTintBrush` so
|
||||
syntax foregrounds remain legible. The per-line foreground recolor from `DiffLinesView`
|
||||
(green/red text) is dropped — syntax colors take over. `Filler` gets a new dim token brush,
|
||||
`Gap` renders as a dim `⋯` separator row.
|
||||
|
||||
**Scroll sync** (split only), modeled on `ConflictResolverView.HookScrollSync` — find each
|
||||
editor's descendant `ScrollViewer`, guard re-entry with a `_syncing` flag:
|
||||
- `WrapLines = false`: sync `Offset.Y` directly. Line heights match, so alignment is exact.
|
||||
- `WrapLines = true`: line heights diverge. Sync on the first visible document line instead
|
||||
(`ScrollToLine`), which keeps the top of the viewport aligned and lets rows drift downward.
|
||||
|
||||
### 3. Host changes
|
||||
|
||||
`DiffViewerView.axaml`:
|
||||
- Header gains a segmented Unified/Split toggle and a wrap toggle.
|
||||
- Files mode: `DiffLinesView Lines="{Binding SelectedFile.Lines}"` → `DiffTextView File="{Binding SelectedFile}"`.
|
||||
- Planning mode: the flattened single-stream `DiffLines` view is replaced by an `ItemsControl`
|
||||
over the subtask's parsed files, each item a file header plus its own `DiffTextView`. One
|
||||
editor can only carry one grammar, so per-file editors are required for highlighting to work
|
||||
at all here. `DiffViewerViewModel` exposes the parsed per-file list for the selected subtask;
|
||||
`DiffLines` and `UnifiedDiffParser.Flatten` lose their last consumer and are removed.
|
||||
|
||||
`DiffLinesView.axaml` + `.axaml.cs` are deleted once both usages are migrated.
|
||||
|
||||
### 4. Persistence
|
||||
|
||||
`src/ClaudeDo.Ui/AppSettings.cs` (`~/.todo-app/ui.config.json`) already holds UI-only
|
||||
preferences (`Language`, `AccentPreset`) with plain `Load()`/`Save()`. Two properties are added
|
||||
there:
|
||||
|
||||
```csharp
|
||||
public string DiffViewMode { get; set; } = "unified"; // "unified" | "split"
|
||||
public bool DiffWrapLines { get; set; }
|
||||
```
|
||||
|
||||
`DiffViewerViewModel` takes the injected `AppSettings`, seeds its toggles on open and calls
|
||||
`Save()` when either changes. No database column, no EF migration, no hub method — a view
|
||||
preference does not belong in `AppSettingsEntity`.
|
||||
|
||||
### 5. Localization
|
||||
|
||||
New keys in both `locales/en.json` and `locales/de.json` (Localization.Tests enforces parity):
|
||||
`diff.view.unified`, `diff.view.split`, `diff.view.wrap`.
|
||||
|
||||
## Testing
|
||||
|
||||
`tests/ClaudeDo.Ui.Tests` — `DiffAlignment` is pure and carries the logic worth testing:
|
||||
|
||||
- Context-only diff → identical rows on both sides, no fillers.
|
||||
- Equal-size change block → 1:1 pairing, no fillers.
|
||||
- Unequal change block (3 del / 5 add) → 3 paired rows + 2 right-side rows with left fillers.
|
||||
- Add-only and delete-only blocks → fillers on the opposite side throughout.
|
||||
- Non-contiguous line numbers → exactly one `Gap` row inserted.
|
||||
- Word diff: single-token change yields one span per side at the right offsets.
|
||||
- Word diff skipped above `MaxWordDiffChars` and below `MinWordDiffSimilarity`.
|
||||
- Row index ↔ document line mapping holds for both `SplitRows` and `UnifiedRows`.
|
||||
- Binary file and empty-content file → empty `AlignedDiff`, no crash.
|
||||
|
||||
`AppSettings` round-trip: persisted mode and wrap survive `Save()`/`Load()`.
|
||||
|
||||
Rendering (margin, both renderers, scroll sync, TextMate colors) is not unit-testable here and
|
||||
is an explicit manual visual pass — see Open items.
|
||||
|
||||
## Known limitations
|
||||
|
||||
1. **Wrap + split drift.** With wrap on, the two panes align at the top of the viewport but rows
|
||||
drift apart further down. Per-line vertical alignment as VS Code does it is out of scope.
|
||||
2. **Fragment highlighting.** See above — highlighting state can be wrong at hunk boundaries.
|
||||
3. **Editors per file in Planning mode.** A subtask touching many files instantiates one editor
|
||||
per file. Same cost as viewing those files individually in Files mode; not capped. If it
|
||||
proves slow, the fix is lazy instantiation on expand, not a silent truncation.
|
||||
|
||||
## Open items (manual verification)
|
||||
|
||||
- Visual pass on both modes: tints legible over DarkPlus syntax colors; line numbers aligned;
|
||||
filler and gap rows readable.
|
||||
- Scroll sync with wrap off (exact) and wrap on (top-anchored).
|
||||
- Planning mode with a multi-file subtask.
|
||||
- Toggle state survives an app restart.
|
||||
|
||||
## Docs to update on completion
|
||||
|
||||
- `src/ClaudeDo.Ui/CLAUDE.md` — Views/Controls list and the "Diff & Conflicts" section still
|
||||
name `DiffLinesView`.
|
||||
- `docs/explore-notes/review-merge.md` — diff stack description + "verified against" commit.
|
||||
Reference in New Issue
Block a user