Skip to content

The emulator reports every sequence it could not render, and termlens installs no callbacks to hear it #266

Description

@vyncint

Today — the backend hands termlens a list of every escape sequence it could not render, and termlens throws it away. crates/termlens/src/emu/vt100.rs:71 builds the parser with the no-op callback set:

parser: ::vt100::Parser::new(rows, cols, scrollback_len),   // Callbacks for ()self.parser.process(&normalized);                            // line 99

vt100::Callbacks has twelve methods, and impl Callbacks for () ignores all of them. Four are the interesting ones — unhandled_csi, unhandled_escape, unhandled_osc, unhandled_control — and vt100 calls them for every sequence outside its dispatch table. Two more are behaviour termlens does model elsewhere and silently drops here: visual_bell (ESC g, while Screen::bells() faithfully counts audible BEL) and resize (CSI 8 ; rows ; cols t, an application asking the terminal to resize itself).

The table is smaller than it looks. For CSI with no intermediate, vt100 dispatches @ A B C D E F G H J K L M P S T X d m r t and nothing else — so CSI 4 h (insert mode), CSI b (repeat), CSI I / CSI Z (tab navigation), CSI g (clear tab stop) and every other ANSI-mode set or reset reach the callback and vanish. Measured against 0.9.0, an application using insert mode renders the wrong line and nothing anywhere says why.

Why it is worth fixing — this is the crate's central promise pointed at itself. termlens answers DECRQM truthfully or not at all, names an unanswered query inside the next timeout, reports Error::Emulator rather than letting a frozen grid run out its deadline, and refuses to claim what the emulator cannot see. The one thing it does not report is "your application asked for something this emulator does not implement, so the screen below is wrong" — which is the failure mode most likely to make a passing test meaningless, because the grid still looks plausible.

The information is free. It is already computed, already delivered, and already addressed to us; installing a callback set is the whole of the collection work. And it turns every future emulation gap from a silent wrong screen into a diagnosable one — including the gaps nobody has found yet, which is the half that cannot be fixed one issue at a time.

Fix — swap the constructor and keep a small record. vt100::Parser::new_with_callbacks(rows, cols, scrollback_len, cb) exists, vt100::Callbacks is publicly exported, and parser.callbacks() reads the state back, so nothing needs to be threaded through by hand.

Follow the shape the unanswered-query record already uses in terminal.rs, because the question is the same one and a second shape would be a second thing to learn: keep distinct sequences in first-seen order, printable-escaped the way seq_printable renders them (^[[4h), bounded with an overflow count so a stream that emits thousands cannot grow memory. Then one accessor, named for what it is rather than for the mechanism:

impl Screen {
    /// Escape sequences the emulator did not implement, so the grid below
    /// them is not what a terminal would show. Distinct shapes, first seen
    /// first, bounded.
    pub fn unsupported(&self) -> &[];
}

Two decisions to make in the PR and write into the rustdoc. First, whether visual_bell joins Screen::bells() or gets its own counter — they are different events and an application that flashes rather than beeps is doing something a test may want to assert, so a separate count is probably right, but say which and why. Second, what to do with the resize request: termlens fixes the grid size on purpose, so honouring it would be wrong, but recording that the application asked is exactly the kind of thing this accessor exists for.

Deliberately out of scope, so this stays reviewable: implementing any of the missing sequences, and surfacing the list inside timeout messages. Both are worth doing and both are their own change — say so in the PR rather than letting the diff grow.

Done when — the parser is built with a callback set that records unhandled CSI, escape, OSC and control sequences plus the visual bell and the resize request; Screen exposes them as distinct bounded shapes; a test drives printf '\033[4h' and finds ^[[4h in the list while an application using only implemented sequences reports none; and the record's bound has a test of its own.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is needed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions