Skip to content

Add ANSI-aware terminal screen overlays#31

Merged
mariusvniekerk merged 3 commits into
mainfrom
feat/tui-screen-overlays
Jul 9, 2026
Merged

Add ANSI-aware terminal screen overlays#31
mariusvniekerk merged 3 commits into
mainfrom
feat/tui-screen-overlays

Conversation

@mariusvniekerk

Copy link
Copy Markdown
Contributor

Why

msgvault and kata both need to splice modal and positioned panels into already-rendered terminal views. Keeping separate private implementations makes ANSI corruption, wide-grapheme bugs, and inconsistent offscreen behavior likely as the callers evolve. Pulling in Bubble Tea or lipgloss at the shared layer would also make otherwise reusable mechanics depend on a specific UI framework.

Design context

The new tui/screen subpackage gives rendered-string callers one zero-based coordinate and clipping model while preserving tui as a namespace for related shared work. It deliberately uses the established charmbracelet ANSI parser instead of a CSI/OSC-only parser so grapheme width and complete control-sequence handling remain consistent with both consumers. The package documentation calls out malformed input and stateful cursor-control limitations.

Validation

  • Full repository suite: 655 tests passed, 2 platform-specific skips
  • Focused golangci-lint, testify-helper, and nilaway checks passed
  • Module checksums verified

generated by a clanker

@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (93efb1b)

Synthesis unavailable. Showing individual review outputs.

Review 1 (done)

Review Findings

  • Severity: Medium
  • Location: tui/screen/screen.go:20
  • Problem: Left/right cuts that cross a wide grapheme drop the grapheme but do not preserve the extra cell it occupied, so later content shifts left. For example, clipping replacement "界X" from column -1 should leave a blank visible cell before X, but Suffix returns only "X" and Splice writes it at column 0.
  • Fix: Use a cut helper that returns both the suffix and the actual skipped cell width, then insert spaces for actualSkipped - requestedSkip before the suffix/replacement. Add tests for left-clipped wide replacements and right-boundary cuts through wide background graphemes.

Summary

The change adds a new ANSI-aware tui/screen package for width measurement, splicing, and viewport overlays.


Review 2 (done)

Summary: Reviewed the new tui/screen ANSI-aware overlay helpers and dependency changes for security-relevant behavior. No concrete vulnerability with a visible exploit path is introduced by this diff.

No issues found.

@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (dc5bff7)

Medium-severity issue found in ANSI-aware wide-grapheme clipping.

Medium

  • tui/screen/screen.go:22 — Compensation padding for a clipped wide grapheme is emitted outside the clipped text’s carried ANSI state. Left, background, and right padding can therefore produce unstyled or unlinked blank cells when clipping background-colored or linked content, creating visible holes. Return the carried ANSI prefix separately from the clipping helper and insert compensation spaces after it; apply the same treatment to all compensation padding paths.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 2m34s

@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (b479006)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 2m7s

mariusvniekerk and others added 3 commits July 9, 2026 18:38
Rendered terminal UIs in msgvault and kata need the same cell-aware splicing mechanics. Keeping private copies risks corrupting escape sequences and lets clipping behavior diverge, while taking a Bubble Tea or lipgloss dependency would make kit's reusable layer framework-specific.

An app-neutral tui/screen package gives callers one defined coordinate and clipping model for already-rendered views. Using the established ANSI parser also keeps grapheme width and control-sequence handling aligned across consumers, with malformed-input and stateful-control limitations documented for future callers.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
Screen coordinates are defined in terminal cells, but suffix cuts inside a two-cell grapheme consumed the full glyph without retaining the overshoot. That shifted remaining background or panel content left and broke clipping guarantees for CJK characters and emoji.

Carry the actual skipped width through internal splice paths so omitted half-cells become blanks at either boundary. The package contract now also requires per-line ANSI state balance and makes side-effecting control replay an explicit caller responsibility.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
Cell-preserving cuts replace an unrenderable half of a wide grapheme with a blank. Emitting that blank outside the carried SGR state left a visible hole whenever the glyph used a terminal background color.

Separate carried and trailing controls from printable suffix and prefix content so compensating cells are written after state replay but before its closing controls. This keeps both coordinates and styling intact across left, right, replacement, and viewport cuts without changing the public API.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk mariusvniekerk force-pushed the feat/tui-screen-overlays branch from b479006 to ea00980 Compare July 9, 2026 22:39
@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (ea00980)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 2m56s

@mariusvniekerk mariusvniekerk merged commit 857441f into main Jul 9, 2026
8 checks passed
@mariusvniekerk mariusvniekerk deleted the feat/tui-screen-overlays branch July 9, 2026 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant