Skip to content

Migrate test terminal emulator from vt100 to rio-vt - #198

Merged
jdx merged 2 commits into
jdx:mainfrom
raphamorim:main
Jul 28, 2026
Merged

Migrate test terminal emulator from vt100 to rio-vt#198
jdx merged 2 commits into
jdx:mainfrom
raphamorim:main

Conversation

@raphamorim

@raphamorim raphamorim commented Jul 28, 2026

Copy link
Copy Markdown

Swaps the redraw tests' terminal emulator from vt100 to rio-vt, Rio's terminal engine, behind a tiny Parser adapter in src/test.rs that keeps the same process / screen().contents() / screen().cursor_position() shape. The call sites in multiselect.rs / select.rs are unchanged apart from the type name, and the full test suite passes (the cursor-drift and frame-shrink redraw assertions hold identically).

rio-vt is a safe-Rust, dependency-light terminal core (VT state machine, grid, parser) and is actively maintained — the swap also drops the transitive net2 (RUSTSEC-2020-0016) that older terminal parsers pull in.

Summary by CodeRabbit

  • Tests
    • Improved Unix terminal test emulation by switching from the previous vt100-based parser to a shared rio-vt-backed Parser with Screen assertions.
    • Updated select and multiselect regression tests to use the shared terminal emulator for redraw and wrapped-option behavior.
    • Preserved existing redraw stability and prompt/redraw clearing coverage while updating the replay/parser wiring.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 827d3d3d-a3ff-4d3c-a23d-53b89d61e8cb

📥 Commits

Reviewing files that changed from the base of the PR and between 34b686a and e1c9441.

📒 Files selected for processing (3)
  • src/multiselect.rs
  • src/select.rs
  • src/test.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/test.rs
  • src/multiselect.rs
  • src/select.rs

📝 Walkthrough

Walkthrough

Unix test terminal emulation migrated from vt100 to a shared rio-vt-backed parser, with redraw regression tests updated to use the new adapter.

Changes

Terminal parser migration

Layer / File(s) Summary
rio-vt capture adapter
Cargo.toml, src/test.rs
Replaces the Unix test dependency on vt100 with rio-vt and adds parser, screen, content, cursor, and replay support.
Redraw regression test migration
src/multiselect.rs, src/select.rs
Updates redraw tests to construct and use the shared test Parser while preserving their existing assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit hops through bytes of light,
With rio-vt parsing screens just right.
Frames redraw, prompts stay neat,
No stacked copies underfoot or feet.
Test ears twitch: the cursor’s bright!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing vt100 with rio-vt in the test terminal emulator.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread Cargo.toml

[target.'cfg(unix)'.dev-dependencies]
portable-pty = "0.9"
rio-vt = "0.5.1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Dependency cannot be resolved

On Unix test targets, Cargo resolves this plain version requirement through crates.io, where the requested rio-vt package is unavailable, causing the test suite to fail before compilation.

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

Migrates Unix redraw-test terminal emulation from vt100 to a shared rio-vt adapter.

  • Adds an adapter exposing the existing processing, screen-content, and cursor-position test interface.
  • Updates selection and multiselection redraw tests to use the adapter.
  • Replaces the Unix development dependency on vt100 with rio-vt.

Confidence Score: 3/5

The PR is not safe to merge until the rio-vt dependency is changed to a source Cargo can resolve.

The manifest still declares rio-vt = "0.5.1" as a plain Unix development dependency without a repository override, so Unix test runs fail during dependency resolution before compilation.

Files Needing Attention: Cargo.toml

Important Files Changed

Filename Overview
Cargo.toml Replaces vt100 with the unresolved plain crates.io dependency rio-vt = "0.5.1"; the previously reported resolution failure remains.
src/test.rs Adds a Unix-only rio-vt adapter preserving the terminal-processing and screen-inspection interface used by redraw tests.
src/multiselect.rs Switches redraw regression tests from the former parser type to the shared adapter without changing their assertions.
src/select.rs Switches the frame-shrink regression test to the shared parser adapter without changing tested behavior.

Reviews (2): Last reviewed commit: "sort test imports for rustfmt" | Re-trigger Greptile

@raphamorim

Copy link
Copy Markdown
Author

Pushed a fix. For the record, the Greptile note about rio-vt being "unavailable from the configured registry" isn't accurate — the crate resolved and compiled fine on all three OSes (the log shows Checking demand … Finished before the failing step). The only red check was cargo fmt --check: the migrated test imports weren't sorted ({capture_term, replay, snapshot, Parser}{Parser, capture_term, replay, snapshot}). That's fixed now; cargo fmt --check + cargo test --lib --tests are green locally.

@jdx
jdx merged commit db3c470 into jdx:main Jul 28, 2026
8 of 9 checks passed
@jdx jdx mentioned this pull request Jul 28, 2026
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