feat(tui): interactive diff view with unified/side-by-side/plain modes by engalar · Pull Request #27 · mendixlabs/mxcli

engalar · 2026-03-25T04:02:23Z

Summary

Add interactive DiffView component with sticky line numbers, hunk navigation (]c/[c), horizontal scroll, and search
Add three display modes: Unified, Side-by-Side, and Plain Diff (LLM-friendly unified diff for yanking)
Introduce View interface and ViewStack to replace scattered boolean visibility flags in app.go
Add type-prefixed search in FuzzyList (e.g. e:Customer to filter entities)
Fix preview auto-trigger, scroll behavior, and hunk navigation

Code Review Fixes (post-review commit)

Remove dead leftOffset/rightOffset/syncScroll/focus scaffold fields
Replace time.Sleep with tea.Tick in flash-clear cmd
Consolidate duplicate stripANSI/stripAnsi into single function
Extract scrollbarGeometry/scrollbarChar helpers (3× duplication removed)
Move diff color palette to theme.go as AdaptiveColor pairs for light/dark support
Add y:yank to DiffViewHints; add hslice OSC limitation comment
go mod tidy — promote sergi/go-diff to direct dependency

Test Plan

go test ./cmd/mxcli/tui/... passes
Unified mode: scroll, hunk nav, search, yank work
Side-by-side mode: both panes render correctly
Plain Diff mode: output matches standard unified diff format
Tab cycles through all three modes
y copies plain diff to clipboard
Type-prefixed search (e.g. e:, mf:) filters correctly in FuzzyList

Add a reusable TUI diff viewer supporting: - Unified and side-by-side views (Tab to toggle) - Word-level inline diff highlighting (Lipgloss segments) - Sticky line numbers during horizontal scroll - Mouse wheel vertical and horizontal scrolling - Vim-style navigation (j/k, h/l, g/G, ]/[ hunk jump) - Search with live matching (/, n/N) - Chroma syntax highlighting for unchanged lines Uses go-difflib SequenceMatcher for high-quality line pairing and sergi/go-diff for word-level segments within paired lines. Integrated via CompareView 'D' key: pick two objects in compare mode, press D to diff their content with word-level precision. Closes #17

Add third DiffView mode (Tab cycles: Unified → Side-by-Side → Plain Diff): - Plain Diff renders standard unified diff format (--- a/, +++ b/, @@ @@) - No ANSI colors — tmux capture-pane produces LLM-readable output - y key copies unified diff to clipboard for pasting to LLM LLM agents can see the "Tab mode" hint, press Tab to switch to Plain Diff, then tmux capture the standard diff format.

Replace ad-hoc view routing (bool/pointer priority chain) with a unified View interface and ViewStack pattern. All views (Browser, Overlay, Compare, Diff, Jumper) implement the same interface, enabling declarative chrome rendering and eliminating 30+ manual sync calls. Key changes: - View interface + ViewStack for composable view management - BrowserView wraps MillerView, absorbs action keys from app.go - OverlayView handles NDSL/MDL tab switching self-contained - Semantic color system (AdaptiveColor) replacing monochrome styles - Focus indicators: blue title + edge bar on focused column, faint unfocused - Global fuzzy jump (Space key) to navigate to any project node - Preview debounce (150ms) to prevent subprocess flooding during fast scroll - LLM anchor lines ([mxcli:mode]) for tmux capture-pane parsing - Clickable breadcrumb in status bar for mouse navigation - Enhanced chrome: mode badge, context summary, compact hint bar Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…efixed search - Fix preview not auto-triggering on cursor move: previewDebounceMsg was silently discarded by BrowserView.Update() type switch - Fix Miller columns scroll bug: value-semantics caused SetSize() to operate on copies, leaving height=0 and maxVisible()=1 - Fix hunk navigation in Plain Diff mode: compute separate hunk indices from @@ headers, use activeHunkStarts() per view mode - Fix ]c/[c keybinding: implement Vim-style two-key sequence with pendingKey state instead of single-key ]/[ - Fix Plain Diff UTF-8 truncation: use hslice instead of byte-based len() for multi-byte character safety - Extract shared FuzzyList component from CompareView and JumperView - Add type-prefixed fuzzy search (e.g. mf:query filters by Microflow) using dynamic fuzzyScore matching against NodeType, no hardcoded map - Add unit tests for DiffEngine (14), DiffRenderer (16), FuzzyList (10)

- Remove dead leftOffset/rightOffset/syncScroll/focus fields from DiffView - Replace time.Sleep with tea.Tick in CompareView flash clear - Consolidate duplicate stripANSI/stripAnsi into single stripAnsi (clipboard.go) - Add OSC sequence limitation comment to hslice - Add y:yank hint to DiffViewHints - Extract scrollbarGeometry/scrollbarChar helpers, remove 3x duplicated scrollbar code - Move diff color palette from diffrender.go to theme.go as AdaptiveColor pairs - Run go mod tidy (promote sergi/go-diff to direct dependency)

ako

Code Review: feat(tui): interactive diff view with unified/side-by-side/plain modes

30 files, +4880/-629, 5 commits. Adds DiffView component with three display modes, introduces View interface/ViewStack pattern, adds type-prefixed fuzzy search.

Critical Issues

1. Overlay Tab-switching is broken (browserview.go / app.go)

BrowserView.handleKey for "b" and "m" keys sets bv.overlayQName, bv.overlayNodeType, bv.overlayIsNDSL on the value receiver, then emits OpenOverlayMsg. But App handles OpenOverlayMsg with:

ov := NewOverlayView(msg.Title, msg.Content, a.width, a.height, OverlayViewOpts{})

The opts are always empty — Switchable is false, QName is empty. NDSL/MDL Tab-switching in the overlay is dead.

2. DiffView search broken in PlainDiff mode (diffview.go)

buildMatchLines() searches dv.result.Lines (structural diff lines), but PlainDiff mode renders dv.plainLines which have different indices (includes @@ headers, ---/+++ lines). Search match indices don't correspond to visible lines, causing scroll-to-match to jump to wrong positions.

3. Key hint mismatch: `d` shown but `D` required (hintbar.go / compare.go)

CompareHints shows {Key: "d", Label: "diff"} but the handler checks case "D":. Users will press d and nothing happens.

Moderate Issues

4. `hslice`/`truncateToWidth` don't handle wide characters (diffrender.go)

hslice increments visW by 1 per rune. East Asian characters take 2 columns. mattn/go-runewidth is added as a dependency but not used in these functions. Diff view will misalign with CJK text.

5. `handleBrowserAppKeys` returns nil-cmd goroutines (app.go)

Many cases return func() tea.Msg { return nil } as a "handled" sentinel. Each spawns a goroutine that does nothing. Should use a no-op constant or change the return signature.

6. CompareView `visible` field redundant with ViewStack (compare.go)

updateInternal starts with if !c.visible { return c, nil } — all updates are no-ops unless Show() was called. This is redundant with being on the ViewStack and fragile if someone forgets to call Show().

Dead Code

7. `BrowserView.overlayQName`, `overlayNodeType`, `overlayIsNDSL` fields — set but never read (browserview.go)

These are set in handleKey but the OpenOverlayMsg handler creates overlays with empty opts. Remove these fields.

8. `BrowserView.handleKey` `"c"` and `"r"` cases unreachable (browserview.go)

Both keys are intercepted by App.handleBrowserAppKeys before reaching BrowserView.Update. The BrowserView cases are dead.

9. `BrowserView.compareItems` field — set but never read (browserview.go)

Assigned in syncBrowserView() and LoadTreeMsg handler but never used by BrowserView.

Minor Issues

10. Magic numbers

150ms debounce delay (miller.go)
3 for divider width in side-by-side (diffrender.go)
8 for horizontal scroll step (diffview.go)

11. `renderContextSummary` naive pluralization (app.go)

strings.ToLower(k)+"s" produces wrong plurals for some types (e.g., "Index" → "indexs").

12. Duplicated `loadBsonNDSL` across BrowserView and CompareView

Identical implementations in both views.

What Looks Good

The View interface + ViewStack pattern is a solid architectural improvement over the scattered boolean flags — makes view routing declarative and composable
DiffEngine with word-level inline highlighting using SequenceMatcher + sergi/go-diff is well-implemented
Type-prefixed fuzzy search (e:Customer, mf:query) is a nice UX touch using dynamic fuzzyScore matching
40 unit tests for DiffEngine, DiffRenderer, and FuzzyList
The post-review cleanup commit addressing dead fields, time.Sleep → tea.Tick, scrollbar dedup, and theme extraction shows good self-review discipline
Plain Diff mode for LLM-friendly output is a thoughtful addition

Workflow Note

The AI review workflow ran but returned an empty response from models.github.ai — likely the 6k-line diff exceeded the request size limit. The pull-requests: write permission was also not granted (only Contents: read, Metadata: read, Models: read appeared in the token). Will fix the workflow separately.

Recommendation

Request changes — fix the critical issues (#1-3) before merging. The overlay Tab-switching being completely broken and the search/hint mismatches are user-facing bugs. The dead code (#7-9) should also be cleaned up since it was identified during the ViewStack refactor.

🤖 Generated with Claude Code

Add ExecView (key 'x') for executing MDL scripts from within the TUI: - Textarea for pasting/typing MDL with Ctrl+E to execute - Inline file picker (Ctrl+O) with path completion filtering .mdl files - Results displayed in overlay, project tree auto-refreshes on success Closes #30

Watch MPR project files for external changes (fsnotify) and automatically refresh the tree and run mx check. Results are shown as a status bar badge (✗ 8E / ✓) and a scrollable overlay via the ! key. - Add Watcher with debounce (500ms), suppress window for self-modifications, and recursive mprcontents/ monitoring - Add checker that parses mx check output format and renders error/warning details in a line-number-free overlay - Export ResolveMx and add ~/.mxcli/mxbuild/ fallback lookup - Add HideLineNumbers option to ContentView/OverlayView

… coverage - Add Base(), SetBase(), ModeNames() to ViewStack; replace direct field access in app.go - Clarify value-receiver Render() intent in DiffView and OverlayView with comments - Add 12 ExecView tests: file picker, cursor navigation, scroll, editor commands - Add 3 ViewStack tests for new Base/SetBase/ModeNames methods - Document watcher extension filter intent for MPR v2 extensionless files

…e cleanup - Critical: fix overlay tab switching by passing OverlayViewOpts through OpenOverlayMsg - Critical: fix PlainDiff search by making buildMatchLines mode-aware - Critical: fix compare hint key d → D - Moderate: fix CJK wide-char handling in hslice/truncateToWidth with runewidth - Moderate: replace 10 nil-cmd goroutines with shared handledCmd variable - Moderate: remove redundant CompareView.visible field (ViewStack handles visibility) - Dead code: remove overlayQName/overlayNodeType/overlayIsNDSL fields - Dead code: remove unreachable "c"/"r" key cases in BrowserView.handleKey - Dead code: remove unused compareItems field and assignments - Minor: extract magic numbers to horizontalScrollStep / previewDebounceDelay constants - Minor: fix pluralization bug ("indexs" → "indexes") with irregularPlurals map - Minor: deduplicate loadBsonNDSL into shared package-level function

- Critical: fix Watcher.Close() race condition with sync.Once - Fix renderCheckResults nil/empty ambiguity (nil = no check run yet) - Fix fsnotify error value being discarded in watcher - Add value-receiver intent comments on ExecView.Render - Add terminal compatibility comment for "\\!" key binding - Document ResolveMx lexicographic version sort limitation Check overlay improvements: - Add "r" key to rerun mx check from within the overlay - Switch from text parsing to mx check -j JSON output for richer location info (document-name, element-name, module-name) - Format locations as qualified names: MyModule.PageName (Page) - Live-update overlay content during check run (spinner → results) - Add refreshable/RefreshMsg plumbing to OverlayView and Overlay

ako

Follow-up Review: fixes verified

All 3 critical issues from the initial review are fixed:

Overlay Tab-switching — FIXED. OpenOverlayMsg now carries OverlayViewOpts populated via bv.overlayOpts(). Tab toggles isNDSL and calls reloadContent().
PlainDiff search — FIXED. buildMatchLines() is now mode-aware, searches plainLines in PlainDiff mode. Rebuilds on mode toggle.
Hint key mismatch — FIXED. Changed to {Key: "D", Label: "diff"} matching the case "D": handler.

Also fixed: CompareView visible removed (ViewStack handles it), dead code cleaned up (overlay fields, unreachable key cases, compareItems), magic numbers extracted, pluralization bug fixed.

Remaining minor items (acceptable as follow-ups):

hslice CJK wide-char boundary handling still imprecise
handledCmd still spawns a nil-returning goroutine (allocation avoided, goroutine not)
Vertical scroll step 3 still a magic number
Overlay BSON/MDL reload duplicates the shared loading pattern (different message type)

Approving — the critical and moderate issues are resolved.

🤖 Generated with Claude Code

- Replace handledCmd nil-returning goroutine with pre-allocated handledNoop Msg to avoid per-call goroutine allocation - Extract mouse scroll step magic number 3 to mouseScrollStep constant (reuse existing definition from column.go)

Bug #27: RETURN values containing qualified enum literals (e.g., Module.Enum.Value) were incorrectly prefixed with $, producing $Module.Enum.Value which causes CE0109 on roundtrip. Enum literals contain dots and should not be treated as variable names.

26 unit tests covering: - #18: loopEndKeyword (WHILE vs LOOP), WHILE loop header formatting - #19: Long type mapping (convertASTToMicroflowDataType, GetTypeName) - #25: DESCRIBE CONSTANT COMMENT output, escaping, absence - #26: Date vs DateTime type mapping and constant formatting - #27: isQualifiedEnumLiteral, enum RETURN without $ prefix - #28: inline if-then-else parsing and expressionToString serialization - #23: deriveColumnName from attribute, caption, fallback, special chars Issue #20 (XPath tokens) already covered by bugfix_test.go.

, #28) Regenerated ANTLR parser from merged grammar to resolve conflicts in auto-generated files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

engalar and others added 5 commits March 25, 2026 11:14

ako reviewed Mar 25, 2026

View reviewed changes

engalar added 5 commits March 25, 2026 18:30

ako approved these changes Mar 25, 2026

View reviewed changes

ako merged commit 95649c4 into mendixlabs:main Mar 25, 2026
2 checks passed

engalar mentioned this pull request Mar 26, 2026

feat(tui): check enhancements, command palette, session restore #31

Merged

8 tasks

ako mentioned this pull request Mar 26, 2026

Fix batch of reported issues (#18, #19, #23, #25, #26, #27, #28) #35

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tui): interactive diff view with unified/side-by-side/plain modes#27

feat(tui): interactive diff view with unified/side-by-side/plain modes#27
ako merged 10 commits intomendixlabs:mainfrom
engalar:feat/tui-diff-view

engalar commented Mar 25, 2026

Uh oh!

ako left a comment

Uh oh!

ako left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

engalar commented Mar 25, 2026

Summary

Code Review Fixes (post-review commit)

Test Plan

Uh oh!

ako left a comment

Choose a reason for hiding this comment

Code Review: feat(tui): interactive diff view with unified/side-by-side/plain modes

Critical Issues

1. Overlay Tab-switching is broken (browserview.go / app.go)

2. DiffView search broken in PlainDiff mode (diffview.go)

3. Key hint mismatch: d shown but D required (hintbar.go / compare.go)

Moderate Issues

4. hslice/truncateToWidth don't handle wide characters (diffrender.go)

5. handleBrowserAppKeys returns nil-cmd goroutines (app.go)

6. CompareView visible field redundant with ViewStack (compare.go)

Dead Code

7. BrowserView.overlayQName, overlayNodeType, overlayIsNDSL fields — set but never read (browserview.go)

8. BrowserView.handleKey "c" and "r" cases unreachable (browserview.go)

9. BrowserView.compareItems field — set but never read (browserview.go)

Minor Issues

10. Magic numbers

11. renderContextSummary naive pluralization (app.go)

12. Duplicated loadBsonNDSL across BrowserView and CompareView

What Looks Good

Workflow Note

Recommendation

Uh oh!

ako left a comment

Choose a reason for hiding this comment

Follow-up Review: fixes verified

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

3. Key hint mismatch: `d` shown but `D` required (hintbar.go / compare.go)

4. `hslice`/`truncateToWidth` don't handle wide characters (diffrender.go)

5. `handleBrowserAppKeys` returns nil-cmd goroutines (app.go)

6. CompareView `visible` field redundant with ViewStack (compare.go)

7. `BrowserView.overlayQName`, `overlayNodeType`, `overlayIsNDSL` fields — set but never read (browserview.go)

8. `BrowserView.handleKey` `"c"` and `"r"` cases unreachable (browserview.go)

9. `BrowserView.compareItems` field — set but never read (browserview.go)

11. `renderContextSummary` naive pluralization (app.go)

12. Duplicated `loadBsonNDSL` across BrowserView and CompareView