Skip to content

refactor(tui): yazi-style Miller columns, clipboard, debug tracing#23

Closed
engalar wants to merge 16 commits intomendixlabs:mainfrom
engalar:pr/tui-improvements
Closed

refactor(tui): yazi-style Miller columns, clipboard, debug tracing#23
engalar wants to merge 16 commits intomendixlabs:mainfrom
engalar:pr/tui-improvements

Conversation

@engalar
Copy link
Contributor

@engalar engalar commented Mar 23, 2026

Summary

  • Refactor TUI to yazi-style Miller columns with tab-based navigation, async preview, and borderless UI
  • Add clipboard copy support and NDSL/MDL format switching in overlay
  • Add content-aware column widths with soft wrap and preview on drill-in
  • Add debug trace logging via MXCLI_TUI_DEBUG=1 environment variable
  • Fix animation issues: forward animTickMsg to miller, remove column-width animation for instant transitions

Test plan

  • Run mxcli tui -p <project.mpr> and verify Miller column navigation works
  • Test Tab key switches between columns
  • Test clipboard copy (y key) in overlay view
  • Test NDSL/MDL switch in overlay
  • Test MXCLI_TUI_DEBUG=1 mxcli tui -p <project.mpr> produces trace output
  • Verify column widths adapt to content without animation glitches

🤖 Generated with Claude Code

engalar added 6 commits March 23, 2026 14:36
- y key copies content to clipboard in overlay and compare view
- OSC 52 terminal escape sequence for SSH/tmux environments,
  with fallback to pbcopy/wl-copy/xclip/xsel
- "✓ Copied!" flash disappears after 1 second
- Tab key switches between NDSL and MDL views in overlay
  when opened via b (BSON) or m (MDL) on a selected node
…rderless UI

Major TUI rewrite from 2-panel + overlay to yazi-inspired design:
- Miller 3-column layout (parent/current/preview) with responsive ratios
- Tab system: t/T/W/1-9/[/] for multi-location and cross-project tabs
- Async preview engine with cache, context cancellation, syntax highlighting
- Borderless minimalist style with dim separators, no box borders
- Context-sensitive key hint bar (HUD) at bottom
- Mouse support: click parent=back, current=drill in, preview=forward
- Scroll wheel in all columns including MDL/NDSL content preview
- Soft line wrapping for preview content (no truncation)
- Slide animation on column transitions (width-based, 5 frames)
- Line numbers in preview pane with scroll percentage indicator
- Clipboard copy (y) from preview content
- Banner stripping for WARNING: and Connected to: lines

New files: app.go, tab.go, miller.go, column.go, preview.go, keys.go,
tabbar.go, statusbar.go, hintbar.go, icons.go
Deleted: model.go, layout.go, panels/ (6 files)
- Column widths now adapt to content (IdealWidth with lipgloss.Width)
- Parent capped at 30%, current at 35%, preview min 25%
- Soft line wrapping in MDL/NDSL preview (wrapVisual) instead of truncation
- Scroll calculations based on visual line count after wrapping
- Preview auto-populates on drill-in/go-back (no more "No preview")
- Animation slowed to 8 frames × 50ms = 400ms, proportional shift
- Output height clamped to prevent overflow affecting other columns
- Separator rendered as exact-height column of │ characters
Writes to ~/.mxcli/tui-debug.log when MXCLI_TUI_DEBUG=1.
Traces: key/mouse events, resize, navigation, preview requests,
column width calculations, animation state.
animTickMsg was not in the forwarding list in app.go Update(),
so animation frames never decremented and parent column stayed
permanently compressed.
Width-based animation caused all text to reflow simultaneously,
creating a dizzying effect. Replaced with clean instant column
shifts matching yazi's native behavior.
Copy link
Collaborator

@ako ako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: refactor(tui): yazi-style Miller columns, clipboard, debug tracing

Overview

Major TUI rewrite (29 files, +4024/-1561 lines) moving from a 2-panel layout to yazi-inspired Miller columns with tabs, async preview, and clipboard support. The old model.go, layout.go, and panels/ directory are deleted and replaced with app.go, miller.go, column.go, preview.go, tab.go, and supporting files.


Critical Issues

1. itoa() is broken for numbers >= 10 (tui/tab.go)

func itoa(n int) string {
    return string(rune('0'+n%10)) + ""
}

For n=12 this returns "2", not "12". Use strconv.Itoa(n).

2. LoadTreeMsg always loads into the active tab (race condition, tui/app.go)

When a cross-project tab is created via PickerDoneMsg, a LoadTreeMsg is dispatched async. If the user switches tabs before it arrives, the handler updates the wrong tab. The message should carry a tab ID.

3. String truncation corrupts multi-byte characters (tui/column.go)

label = label[:contentWidth-2]

Byte-level slicing on strings containing emoji icons (from icons.go) will produce invalid UTF-8. Use lipgloss.Truncate() or rune-aware truncation.

4. View() mutations on value receivers are silently discarded (tui/miller.go)

View() has a value receiver and calls m.parent.SetSize(...), m.current.SetSize(...). These mutations are lost on the copy. Same issue with viewZen(). This works by accident because bubbletea re-renders frequently, but it's fragile and wasteful.

5. PreviewEngine goroutines leak on tab close and app quit

  • PreviewEngine.cancelFunc is never called on app exit — only CloseTrace() is called
  • When a tab is closed via W, its PreviewEngine (and any in-flight exec.CommandContext subprocesses) are never cancelled
  • Should add a Close() method to PreviewEngine and call it on tab close + app quit

Moderate Issues

6. ~315 lines of dead code in tui/keys.go

DefaultListKeys(), DefaultOverlayKeys(), DefaultCompareKeys(), DefaultTabKeys(), DefaultGlobalKeys() and their key map types are defined but never used anywhere. All key handling uses raw msg.String() comparisons. This entire file should either be integrated or removed.

7. Dead code in tui/tab.go and tui/miller.go

  • NavState type is defined but unused
  • MillerFocusPreview constant exists but focus is never set to it
  • GoBack() and ToggleZen() exported methods are never called

8. CloseTrace() doesn't reset traceActive (tui/trace.go)

A Trace() call after CloseTrace() would attempt to write to a closed file.

9. //nolint:govet suppression (tui/miller.go)

func (m MillerView) handleMouse(msg tea.MouseMsg) (MillerView, tea.Cmd) { //nolint:govet

Suppresses the legitimate vet warning about value receiver semantics instead of fixing the underlying issue.


Minor Issues

10. Magic numbers scattered throughout

  • millerH := a.height - 3 — magic 3 for chrome height
  • Column width thresholds: 80, 50, 30, 35, 25, 15, 8 in columnWidths()
  • Scroll step 3 hardcoded in multiple mouse handlers

11. openDiagram — temp files never cleaned up, WriteString error ignored

Pre-existing issue but worth noting. Also nodeType and elkJSON are interpolated into HTML without sanitization (low risk since it's a local file).

12. tabbar.gorebuildZones() in SetTabs() is redundant

View() clears and rebuilds zones from scratch, making the earlier call unnecessary.

13. Plan docs included in PR

docs/plans/2026-03-22-bson-ndsl-format.md (463 lines) and docs/plans/2026-03-23-tui-yazi-refactor-design.md (376 lines) are implementation plans. Consider whether these belong in the repo long-term or should be removed before merge.


What Looks Good

  • The overall architecture (Miller columns, async preview with caching + cancellation, tab system) is well-designed
  • Clipboard implementation correctly uses stdin pipe (no command injection) with OSC 52 fallback for remote sessions
  • sync.Once protection on trace initialization
  • Preview cache prevents redundant mxcli subprocess spawns
  • Clean separation of concerns across the new files

Recommendation

Request changes — fix the critical bugs (#1-5) and remove dead code (#6-7) before merging. The value-receiver-mutation pattern (#4) is the most architecturally concerning issue and could cause subtle rendering bugs.

🤖 Generated with Claude Code

engalar added 10 commits March 24, 2026 09:54
- buildDescribeCmd: add systemoverview → SHOW STRUCTURE DEPTH 2
- buildDescribeCmd: return "" for virtual container nodes (security,
  category, domainmodel, navigation, projectsecurity, navprofile)
- fetch(): show "Select a document to preview" when cmd is empty
- Add TestBuildDescribeCmd coverage for systemoverview and virtual nodes

fix(sdk/mpr): resolve module names through folder hierarchy in MPR v2

In MPR v2, documents (pages, microflows, etc.) live inside Projects$Folder
units, so their ContainerID points to a folder, not the module directly.
The previous single-level moduleMap[ContainerID] lookup always missed,
causing bson dump to fail with "page not found: Module.Name".

Add resolveModuleName() which walks the container hierarchy upward until
it finds a module unit, mirroring the catalog's findModuleID logic.
Fix both GetRawUnitByName and ListRawUnits.
- Add imagecollection → Images$ImageCollection mapping in GetRawUnitByName
  and ListRawUnits (reader_units.go) so bson dump --type imagecollection works
- Add imagecollection to inferBsonType() so NDSL preview mode can dump it
- Add imagecollection to buildDescribeCmd() no-MDL-command list (returns "")
  since DESCRIBE IMAGECOLLECTION has no MDL syntax; shows friendly placeholder
- Add TDD test cases for imagecollection in TestBuildDescribeCmd (RED→GREEN)
Add executor handlers for image collection statements with module
lookup, duplicate detection, and hierarchy invalidation.
…E COLLECTION

Parse Image.Data, Format, and ID fields from BSON in parser_misc.go.
Write image data to /tmp/mxcli-preview/ and display IMAGE lines with
file paths in DESCRIBE output.
…nd security fix

- Add COLLECTION keyword to LSP completions
- Add visitor_imagecollection.go for CREATE/DROP IMAGE COLLECTION AST handling
- Fix writer_security.go to handle MaybeGeneralization in ReconcileMemberAccesses
- Add implementation plan doc
@engalar engalar closed this Mar 24, 2026
engalar added a commit to engalar/mxcli that referenced this pull request Mar 24, 2026
…d/mxcli/

Fix all critical and moderate issues from code review:
- Fix itoa() for tab numbers >= 10 (#1)
- Add TabID to LoadTreeMsg to prevent race condition (#2)
- Fix UTF-8 truncation with runewidth (#3)
- Change View()/viewZen() to pointer receivers (#4)
- Cancel PreviewEngine on tab close and app quit (#5)
- Remove ~315 lines dead code in keys.go (#6)
- Remove unused NavState, MillerFocusPreview, GoBack, ToggleZen (#7)
- Reset traceActive in CloseTrace() (#8)
- Remove nolint:govet suppression (#9)
- Extract magic numbers as named constants (#10)
- Handle openDiagram temp file cleanup and WriteString error (#11)
- Remove redundant rebuildZones() call (#12)
- Fallback NDSL preview to MDL DESCRIBE for unsupported types
- Move bson/ and tui/ packages into cmd/mxcli/ per reviewer suggestion
engalar added a commit to engalar/mxcli that referenced this pull request Mar 24, 2026
…d/mxcli/

Fix all critical and moderate issues from code review:
- Fix itoa() for tab numbers >= 10 (#1)
- Add TabID to LoadTreeMsg to prevent race condition (#2)
- Fix UTF-8 truncation with runewidth (#3)
- Change View()/viewZen() to pointer receivers (#4)
- Cancel PreviewEngine on tab close and app quit (#5)
- Remove ~315 lines dead code in keys.go (#6)
- Remove unused NavState, MillerFocusPreview, GoBack, ToggleZen (#7)
- Reset traceActive in CloseTrace() (#8)
- Remove nolint:govet suppression (#9)
- Extract magic numbers as named constants (#10)
- Handle openDiagram temp file cleanup and WriteString error (#11)
- Remove redundant rebuildZones() call (#12)
- Fallback NDSL preview to MDL DESCRIBE for unsupported types
- Move bson/ and tui/ packages into cmd/mxcli/ per reviewer suggestion
ako pushed a commit that referenced this pull request Mar 24, 2026
Fix all critical and moderate issues from code review:
- Fix itoa() for tab numbers >= 10 (#1)
- Add TabID to LoadTreeMsg to prevent race condition (#2)
- Fix UTF-8 truncation with runewidth (#3)
- Change View()/viewZen() to pointer receivers (#4)
- Cancel PreviewEngine on tab close and app quit (#5)
- Remove ~315 lines dead code in keys.go (#6)
- Remove unused NavState, MillerFocusPreview, GoBack, ToggleZen (#7)
- Reset traceActive in CloseTrace() (#8)
- Remove nolint:govet suppression (#9)
- Extract magic numbers as named constants (#10)
- Handle openDiagram temp file cleanup and WriteString error (#11)
- Remove redundant rebuildZones() call (#12)
- Fallback NDSL preview to MDL DESCRIBE for unsupported types
- Move bson/ and tui/ packages into cmd/mxcli/ per reviewer suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants