Swap the vt100 terminal engine for rio-vt, without a shim - #140
Conversation
1be0e21 to
af6a1cc
Compare
rio-vt upstream notesCollecting the rio-vt findings from doing this integration, since several of them explain why the Posting it here rather than as a file in the diff to keep the PR to one thing. @raphamorim, most of Full notes (12 items, with repros)Findings from integrating Versions. Line references are to Upstream: https://github.com/raphamorim/rio §1 and §12 are correctness bugs — §12 is an outright panic, and §1 likely affects Rio's own 1.
|
| Crate | Version | Status |
|---|---|---|
fuchsia-zircon |
0.3.3 | unmaintained, last published 2018 |
fuchsia-zircon-sys |
0.3.3 | unmaintained, last published 2018 |
iovec |
0.1.4 | deprecated in favour of std::io::IoSlice |
miow |
0.5.0 | pulls windows-sys 0.42.0 |
windows |
0.42.0 | 2022-era |
Plus teletypewriter itself (signal-hook, miow 0.6, a second windows-sys), and via other
paths regex, regex-automata, url/idna, flate2, phf.
This also makes rio-vt a harder sell to security-conscious downstreams: the ratty PR proposing the
swap argued it improved the dependency picture, and the opposite is true.
Ask: put the PTY driver behind a feature, e.g.
[features]
default = ["pty"]
pty = ["dep:teletypewriter", "dep:corcovado"]so default-features = false gives a pure VT-core build. The crate description ("VT state machine,
grid, PTY driver, selection, search") already frames these as separable concerns.
Secondary: parking_lot is pulled with features = ["nightly", "hardware-lock-elision"]
unconditionally. Both should be opt-in — embedders shouldn't inherit a nightly-flavoured lock
implementation transitively.
6. DA1 advertises capabilities the build may not have and the embedder may not implement
Severity: medium — protocol correctness.
Primary device attributes is a hardcoded string (:3255):
let text = String::from("\x1b[?62;4;6;22;52c");Two problems:
4= sixel graphics is reported even when thegraphicsfeature is off, in which case all
the image-decoding paths are#[cfg]-compiled out. Apps feature-detect sixel, emit it, and it
renders as nothing.52= OSC 52 clipboard access is not rio-vt's call to make. The clipboard is the embedder's
responsibility (RioEvent::ClipboardStore/ClipboardLoad), and an embedder that ignores those
events — the default, sinceVoidListenerdrops everything — will advertise clipboard support it
does not have. This one has a security dimension: OSC 52 read-back is a known exfiltration
vector, and terminals generally gate it behind explicit config.
Ask: derive the DA1 parameter list from compiled features, and let the embedder opt into
capability bits it actually implements (a Capabilities bitflag on Crosswords::new, or a setter).
At minimum, gate 4 behind #[cfg(feature = "graphics")].
7. Terminal identity is hardcoded to Rio
Severity: low — but wrong for every embedder.
- XTVERSION (
:3272-3276):format!("\x1bP>|Rio {version}\x1b\\")— hardcoded"Rio", versioned
withenv!("CARGO_PKG_VERSION"), i.e. rio-vt's own crate version. - Secondary DA (
:3260-3264):\x1b[>0;{version};1c, same version source.
Every embedder therefore identifies itself as Rio. Apps do branch on XTVERSION for
terminal-specific quirks, so this actively misroutes behavior.
Ask: an embedder-supplied name/version — Crosswords::new(..., identity: TerminalIdentity) or a
setter defaulting to Rio <rio-vt version> for backwards compatibility.
8. Reply-callback events have no documented contract
Severity: low-medium — DX / silent hangs.
RioEvent::ColorRequest, TextAreaSizeRequest, and ClipboardLoad carry
Arc<dyn Fn(...) -> String> callbacks whose results the embedder is expected to write back to the
PTY. Nothing in the trait docs says so, and VoidListener silently drops them.
The failure mode is bad: an app issues OSC 11 ; ? ST to detect a light/dark background (neovim,
delta, bat, fzf all do this), gets no reply, and blocks until its timeout. It looks like a hang in
the app, not a missing terminal feature.
Ask: document on EventListener which variants require a response, and consider splitting them
into a distinct fn request(&self, req: RioRequest) -> Option<String> so the type system makes the
obligation visible instead of leaving it as a runtime convention.
9. EventListener::event() is required but never called
Severity: low — dead API surface.
fn event(&self) -> (Option<RioEvent>, bool) is a required trait method. Grepping the entire crate
for .event() finds no call sites. Every implementor writes (None, false) and wonders what it is
for.
Ask: remove it, or give it a default body and document its intended role.
10. No X10 mouse reporting (CSI ? 9 h)
Severity: low.
ansi/mode.rs:111-113 maps 1000 / 1002 / 1003 only; there is no mode-9 bit, so an embedder
mirroring a fuller mouse-mode enum ends up with an unconstructable variant. X10 is largely
obsolete, so this may be a deliberate "won't fix" — worth saying so in the docs either way.
11. Screen-reading API surface is awkward for embedders
Severity: low — DX, but it is what produced §1 and §4 in practice.
Taken together the read path has some sharp edges:
Crosswords::gridis a barepubfield (:430), so embedders reach into internals for
grid.style_set.styles()andgrid.extras_table— both necessary to resolve a cell.- The two obvious "read the screen" entry points (
visible_rows,snapshot_visible) are the ones
broken by §1, andvisible_rows()additionally allocates and deep-copies the whole visible grid
every call. Measured: 5.0 µs per call on a 50×200 grid in release (~50 allocations, ~80 KB
copied). An embedder that reasonably assumes it is cheap — because invt100the equivalent was
a plain borrow — pays it several times per frame and per PTY chunk. - The correct, cheap path (index
grid[Line(row - display_offset)]and borrow) is not documented
anywhere, and requires knowing thatGrid'sIndex<Line>is display-offset-relative. Crosswords::newtakesWindowIdand aroute_id: usize, which are Rio-multiplexer concepts
leaking into an API sold as embeddable. Embedders passWindowId::from(0), 0and move on.
Ask: a documented, borrowing, correctness-by-default read API — something like
pub fn visible_row(&self, row: usize) -> Option<&Row<Square>> and
pub fn styles(&self) -> &[Style] — with visible_rows()'s allocation behavior called out in its
doc comment. That single pair of accessors would have prevented three of the bugs found during this
integration.
12. A double-width glyph in a single-column grid panics
Severity: high — panic on a valid grid size.
Writing any 2-cell character into a 1-column grid panics inside the grid layer. Narrow text is
fine, and two columns is fine, so it is specifically the wide-glyph placement path.
Repro:
let mut term = Crosswords::new(CrosswordsSize::new(1, 5), CursorShape::Block, listener,
WindowId::from(0), 0, 100);
Processor::default().advance(&mut term, "你".as_bytes());
// panicked at crosswords/grid/mod.rs:358: index out of bounds: the len is 1 but the index is 1| grid | narrow ASCII | one wide char | emoji |
|---|---|---|---|
| 1 column | ok | panic | panic |
| 2 columns | ok | ok | ok |
Single-row grids are fine — a 1×40 terminal survives wide characters, wrapping, scrolling, the
alternate screen, DECSTBM, and resize.
A terminal core should not panic on a grid the embedder is entitled to create. Window managers,
tiling layouts, and drag-resize all transiently produce very narrow terminals, and the embedder has
no way to know the floor is two columns without hitting it. Expected behaviour is to drop or
replace the glyph the way other terminals do.
Workaround in ratty: clamp the grid to at least two columns in resize_to_fit.
Summary
| § | Ask | Severity | Kind |
|---|---|---|---|
| 1 | visible_rows/snapshot_visible must not use scroll_region |
high | bug |
| 12 | wide glyph in a 1-column grid panics | high | bug |
| 2 | expose active kitty-keyboard flags | medium | API gap |
| 3 | model modifyOtherKeys, or surface unhandled CSI |
medium | API gap |
| 4 | accessor for full cell text incl. zero-width marks | medium | API gap |
| 5 | feature-gate teletypewriter / corcovado; make parking_lot features opt-in |
medium | packaging |
| 6 | derive DA1 from features + embedder capabilities | medium | protocol |
| 7 | embedder-supplied terminal identity for XTVERSION / DA2 | low | protocol |
| 8 | document reply-callback obligations | low-med | contract |
| 9 | remove or document EventListener::event() |
low | API |
| 10 | X10 mouse mode, or document as won't-fix | low | feature |
| 11 | documented borrowing screen-read API | low | DX |
§1, §2, §3, §5, and §12 are the ones that materially change whether the ratty integration is clean.
Note that §2 and §3 were independently identified by rio-vt's author in the
ratty PR #138 description as gaps they intended to
upstream.
Ratty's own workarounds live in src/vt.rs (§1, §2, §3, §4) and src/terminal.rs (§12), each
commented with the rio-vt behaviour that forces it, so they can be found and removed as fixes land
upstream.
7eadedb to
3d3df2d
Compare
|
Noice! Will address those @gold-silver-copper ! Only closed the old pr because didn't want to feel like "forcing" you folks to migrate lol |
3d3df2d to
fe96c24
Compare
|
@raphamorim Thanks, none of them are real blockers, most have workarounds. The best one to tackle first would be: 5. teletypewriter and corcovado are unconditional dependencies . Since this is something that we cant work around ourselves. We don't plan on getting rid of portable-pty yet, so being able to disable teletypewriter would be the biggest win for us. All the other issues are mainly nice to haves. |
|
Yea, that make sense. I was looking into that yesterday. Might take few versions for it, but will ping here back |
Test planSince the automated suite only reaches the VT boundary and the ratatui buffer, here's a plan for Two things worth pulling out of it before you open the fold: Build both engines and A/B them. The real question isn't "does it work" but "is it a cargo build --release # this branch
git worktree add /tmp/ratty-main origin/main
cargo build --release --manifest-path /tmp/ratty-main/Cargo.toml # vt100Two checks carry most of the weight. Phase 4 (vim/less/htop/tmux, watching the bottom rows and Phase 2 exists because CI is thinner than the green check suggests — it never runs
Full test plan (12 phases, with commands and pass criteria)Every step says what changed, what to run, and what a pass looks like. Phases 0–2 are Branch: Phase 0 — Setupgit fetch origin
git checkout rio-vt-native
git log --oneline -1 # expect: refactor(vt): swap the vt100 terminal engine for rio-vtBuild both sides so you can A/B against the old engine — this is the single highest-value cargo build --release # new engine -> target/release/ratty
git worktree add /tmp/ratty-main origin/main
cargo build --release --manifest-path /tmp/ratty-main/Cargo.toml # old vt100 buildKeep both binaries. Anywhere below that says compare against main, run the same input in Phase 1 — Automated checkscargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings
cargo test --lib # expect 99 passed, 0 failed
cargo check --all-targets
cargo check --all-targets --manifest-path widget/Cargo.tomlThe 32 tests added by this PR: cargo test --lib vt:: # 26 — engine boundary
cargo test --lib terminal:: # 3 — widget render through the ratatui buffer
cargo test --lib mouse:: # 7 — selection text (4 new here)Two of them are the regression guards that motivated the PR; they should fail against #138's cargo test --lib every_row_stays_reachable_with_a_scroll_region_set
cargo test --lib widget_draws_every_row_with_a_scroll_region_setPass: all green, zero warnings. Phase 2 — Cross-platform (fills a real CI gap)CI on this repo only runs This matters more than usual because rio-vt drags in # whatever targets you have toolchains for
cargo check --target x86_64-unknown-linux-gnu
cargo check --target aarch64-apple-darwin
cargo check --target x86_64-pc-windows-msvc
# and run the suite on at least one non-development platform
cargo test --libPass: builds on every platform you ship, and the suite passes on more than one. Phase 3 — Basic terminal sanityChanged: the entire PTY→parse→render path ( cargo run --releaseThen inside ratty: echo hello
ls -la
seq 1 200 # scrolling
printf 'a\tb\tc\n' # tabs
yes | head -5000 # bulk output, watch for tearing or lag
clearPass: text lands where expected, no dropped or duplicated lines, no visual corruption. Phase 4 — Scroll regions (the headline fix)Changed: Full-screen apps set a scroll region, which is exactly what the shim approach got wrong: vim /etc/hosts # :q to exit
less /usr/share/dict/words
htop # or top
tmux # then: create panes, split, exit
man lsLook specifically at:
Explicit DECSTBM check — set a region, then paint every row: printf '\e[2J\e[H'; printf '\e[3;20r'; for i in $(seq 1 24); do printf '\e[%d;1Hrow%d' $i $i; done; printf '\e[r\e[25;1H\n'Pass: all 24 rows show Phase 5 — UnicodeChanged: printf 'wide: 你好世界 ok\n'
printf 'combining: e\xcc\x81 a\xcc\x80 n\xcc\x83 -- should read: é à ñ\n'
printf 'emoji: 😀 👩💻 🇯🇵 ok\n'
# every line must start at column 0 — a label on only the first line
# shifts the top border and makes the box look broken in ANY terminal
printf '┌───┬───┐\n│ a │ b │\n├───┼───┤\n│ c │ d │\n└───┴───┘\n'
printf 'mixed: 日本語text混在\n'Pass: accents render on their letters (not as bare For the box, separate the two things that can go wrong:
Phase 6 — CursorChanged: visibility now comes from rio-vt's resolved printf '\e[?25l'; sleep 2; printf '\e[?25h' # hide 2s, then show
printf '\e[10;40Hcursor here\n' # absolute positioning
printf '\e[10;40f cursor here via HVP\n' # HVP — the old rewriter is deletedThen scroll into history with Pass: DECTCEM hides and restores the cursor; HVP positions identically to CUP (this exercises Phase 7 — Scrollback and resizeChanged: resize is now a native rio-vt reflow. The old path serialized only the visible screen and seq 1 5000
Then, the important one: seq 1 5000
# now drag the window narrower, then widerPass: scrollback survives the resize and long lines reflow to the new width. On main the Also check scrollback is inert on the alternate screen: vim # scroll keys should do nothing here, then :qPhase 8 — Mouse and selectionChanged:
Mouse reporting inside an app that uses it: htop # click column headers to sort; F10 to quit
tmux # click to switch panesPass: selection matches rendering exactly (especially wide chars); clicks register on the right Phase 9 — Protocol replies (new interception code)Changed: Save as #!/usr/bin/env bash
query() {
local seq="$1" reply="" ch saved
saved=$(stty -g)
stty raw -echo
printf '%b' "$seq" > /dev/tty
while IFS= read -r -s -n1 -t 0.3 ch; do reply+="$ch"; done
stty "$saved"
printf '%-12s %s\n' "$2" "$(printf '%s' "$reply" | cat -v)"
}
query '\e[0c' 'DA1'
query '\e[>0c' 'DA2'
query '\e[>0q' 'XTVERSION'
query '\e[5n' 'DSR'
query '\e[6n' 'CPR'Pass:
The last two matter as much as the first three: they prove the rewriter isn't mangling replies it Phase 10 — Keyboard protocolsChanged: kitty flags now read from rio-vt's mode bits; printf '\e[>4;2m' # enable modifyOtherKeys level 2
# type Ctrl+Enter, Ctrl+Tab, Ctrl+Shift+letters
printf '\e[>4m' # resetBest real-world exercise — apps that negotiate the kitty keyboard protocol: nvim # then try Ctrl+Enter, Ctrl+/ , Shift+Enter in insert modePass: modified keys reach the app; nothing is doubled or swallowed. Compare against main — Phase 11 — ratty-specific: images, RGP, 3DChanged: Kitty images (whatever you normally use): # e.g. an image via the kitty graphics protocol, including a Unicode-placeholder placementRGP inline objects, from printf '\e_ratty;g;r;id=7;fmt=obj;path=CairoSpinyMouse.obj\e\\'
printf '\e_ratty;g;p;id=7;row=5;col=10;w=3;h=2;animate=1;scale=1.0;depth=1.5;ry=30\e\\'
printf '\e_ratty;g;u;id=7;ry=180\e\\'
printf '\e_ratty;g;d;id=7\e\\'Then scroll the terminal with an object placed, and resize the window. 3D presentation modes:
Camera via protocol: printf '\e_ratty;g;c;id=0;set=1;type=Persp;fov=50;rx=10;ry=20\e\\'
printf '\e_ratty;g;c;id=0;set=1;type=Flat\e\\'Pass: images anchor to the right cells and track the text as it scrolls (this is the code Phase 12 — Degenerate sizes and stressChanged: the grid floor is now 2 columns × 1 row (was 2×2). rio-vt panics placing a double-width # drag the window as narrow and as short as it will go, with CJK on screen:
printf '你好世界\n'
# then resize to a sliver in both dimensionsPass: no panic, no crash. This is the one place rio-vt is known to abort, so push it. Throughput and font size: time (yes | head -200000)
cat /dev/urandom | head -c 100000 | base64 # dense output
# Ctrl+= / Ctrl+- to change font size, Ctrl+Alt+0 to resetPass: no stall or runaway memory. Compare against main — the render path no longer Sign-off checklist
Open questions for the maintainer that testing cannot settle: whether +37 packages |
|
1-4 ref: raphamorim/rio#1805 |
9763e65 to
fa0da2e
Compare
|
Epic 😎 |
|
@gold-silver-copper created a rio discord (again lmao), anything ping me there https://discord.gg/qTV8HmSRV |
|
fix event trait raphamorim/rio#1808 |
|
rio-vt: Support X10 mouse reporting (CSI ? 9 h) raphamorim/rio#1809 |
fa0da2e to
b6fd26f
Compare
Rebased onto rio-vt 0.5.10 — 7 of the 12 upstream notes are fixedThanks @raphamorim, that was fast. Adopted all of it:
Net −159 lines, and the part I was least comfortable shipping is gone: the The Re-verified after the refactor — a full buffer dump (symbol + fg + bg + modifiers, widths §1 note for Rio itself: Still open, in priority order for this integration:
|
|
I apologize for the AI autoposting replies lmao |
Replaces vt100 with rio-vt as ratty's VT state machine, written directly against rio-vt's API rather than behind a vt100-shaped compatibility shim. `TerminalRuntime` now owns a `Crosswords` plus a `Processor`, and the renderers borrow the grid instead of snapshotting it. The new `src/vt.rs` holds the event listener, the screen-reading helpers the renderers share, and the state rio-vt does not model. It is an adapter, not a shim: nothing in it is shaped like a vt100 type. Screen reads index the grid and borrow rather than calling rio-vt's `visible_rows`, which deep-copies every visible row on each call. Ratty reads the screen several times per frame and per PTY chunk, so the copy is worth avoiding. Deletes workarounds that existed only for vt100: - `normalize_hvp_sequences`, which rewrote HVP to CUP; rio-vt handles HVP - the snapshot/replay resize dance; rio-vt reflows and resets the scroll region natively - the two-row grid floor; only the column floor is still required Behavioural notes: - Cursor visibility now comes from rio-vt's resolved `CursorState`, which also hides the cursor while scrolled into history and snaps the column off a wide-cell spacer. - Cell text is the full grapheme cluster (base codepoint plus the extras table's zero-width marks), so combining marks and ZWJ sequences survive. - Kitty placeholder scanning is gated on rio-vt's per-row `kitty_virtual_placeholder` flag. - X10 mouse reporting (`CSI ? 9 h`) is supported. - Both of rio-vt's spacer kinds are left blank. `LeadingSpacer` is the pad written at end-of-line when a wide glyph wraps; it carries the active SGR style, so drawing it painted a styled block there and defeated parley_ratatui's wide-cell heuristic, which only treats a trailing space as a continuation when it has no background. vt100 had no equivalent cell, so this restores buffer parity with the old engine. - `modifyOtherKeys` and the kitty keyboard flags come from rio-vt's own accessors. Level 0 correctly reads as disabled, which the previous byte-sniffing workaround got wrong. - Replies (DA, DSR, CPR, XTVERSION, kitty mode reports) now come from the engine, which owns the protocol framing. Three of them describe rio-vt rather than ratty, so the listener patches the payloads on the way out: the DA1 capability list drops sixel (`4`) and OSC 52 (`52`), which nothing here implements, and XTVERSION and the DA2 firmware field report ratty's name and version instead of Rio's. Parsed structurally, so a rio-vt release that changes its capability list is still filtered. - The cursor reports column `width - 1` at end-of-line rather than `width`, matching alacritty-style pending-wrap semantics. - The warn-once unhandled-sequence logging is gone. rio-vt logs via `tracing` and `bevy_log` installs the global subscriber, so its diagnostics still surface, without the per-sequence deduplication. rio-vt is declared as 0.5.13. Upstream has since fixed most of what the integration had to work around: the scroll-region-bound row accessor, the missing kitty-keyboard and modifyOtherKeys accessors, the missing cell-text accessor, X10 mouse reporting, the dead `EventListener::event` method, and the abort on placing a wide glyph in a single-column grid. Adopting those removes the byte sniffer entirely and drops the grid floor back to one column. This grows the dependency tree from 658 to 695 packages, including `teletypewriter` (an unused second PTY implementation, kept out of the way -- `portable-pty` still drives the PTY) and `corcovado` with its unmaintained transitive deps. See rio-upstream-asks.md. Adds 33 tests. In `vt`: the scroll-region regression, combining marks, wide-cell spacers, cursor visibility, scrollback clamping, resize reflow, engine replies and their rewriting, mouse mode precedence, kitty keyboard flags, colour resolution, and `modifyOtherKeys` including split sequences. In `terminal`: the scroll-region and wide-character cases rendered end to end through the ratatui buffer, one layer above where the vt tests stop and where the defect actually showed. In `mouse`: selection text across rows, wide characters, and combining marks, a path that was rewritten here and previously had no coverage. Co-authored-by: Raphael Amorim <rapha850@gmail.com>
b6fd26f to
f987553
Compare
Replaces
vt100withrio-vtas Ratty's VT state machine. The integration uses Rio's native grid and protocol APIs rather than reproducing vt100's interface behind a shim.portable-ptyremains Ratty's PTY layer.This is an alternative implementation of #138. Full credit to @raphamorim for the engine work and for identifying the Rio integration gaps; the main difference here is the adapter strategy.
Why a native adapter
src/vt.rscentralizes the Rio event listener, borrowed screen/grid helpers, terminal capability replies, and state not modeled by the engine.TerminalRuntimeowns aCrosswordsandProcessor, while both renderers borrow the grid directly.This avoids three mismatches that were easy to hide behind a vt100-shaped shim:
visible_rows()to the DECSTBM scroll region;visible_rows()deep-copies rows, while Ratty reads the screen several times per frame;Square::c()is not the full grapheme cluster.Rio now exposes the APIs needed to handle these natively (
visible_line_bounds,Grid::cell_text, keyboard mode,modifyOtherKeys, and X10 mouse state).Correctness improvements
modifyOtherKeys, mouse protocol precedence, X10 mouse mode, colors, and scrollback behavior come from engine state and are covered by tests.Rio 0.5.19 update
This PR now uses
rio-vt 0.5.19withdefault-features = false. Rio added aptyfeature after the original PR was opened, so embedders such as Ratty can omit Rio's unused PTY implementation while continuing to useportable-pty.That reduces the root lockfile from the original PR's 695 packages to 676 (main has 658): +18 rather than +37. The unused
teletypewriter/corcovadochain and its old platform dependencies are gone.Rio 0.5.19 still hardcodes Rio's device capabilities/identity, so the local reply rewriting remains necessary.
Verification
Local verification on the final diff:
cargo fmt --all -- --checkcargo fmt --all --manifest-path widget/Cargo.toml -- --checkcargo test --lib --locked— 103 passed, 0 failedcargo check --all-targets --lockedcargo check --all-targets --locked --manifest-path widget/Cargo.tomlcargo clippy --all-targets --locked -- -D warningscargo clippy --all-targets --locked --manifest-path widget/Cargo.toml -- -D warningsThe workflow matrix was also corrected so all five targets run against both manifests (10 check jobs), with native ARM64 Linux on
ubuntu-24.04-arm. A Linux library-test job now runs the full 103-test suite in CI.Manual visual pass still recommended
The remaining high-value manual pass is GPU-backed behavior: vim/less/htop/tmux bottom-row alignment, resize and scrollback, truecolor/attributes, Unicode and CJK text, mouse reporting, Kitty images, and RGP inline objects in both 2D and 3D.