Repository navigation
fix(emu): mouse reset, unsupported() entries, the unanswered-query note - #737
Merged
Merged
Conversation
A reset of a mouse tracking mode other than the one in force left reporting on. After `CSI ?1002h CSI ?1000l`, and after `?9h ?1000l`, `mouse_mode()` was still the mode that had been set and `click` returned Ok and typed a report. The rustdoc of `Screen::mouse_mode` and DESIGN §1 both say "disabling any of them turns reporting off", and xterm does that: `really_set_mousemode` (charproc.c) sets `MOUSE_OFF` on a reset of 9, 1000, 1002 or 1003 alike. `input_modes()` and the snapshot took vt100's value, and vt100 0.18.1 clears it only when the reset names the mode it holds (`clear_mouse_mode`). The sequence tracker now derives the protocol, as it already derives the encoding (#640): the tracking mode set last is in force, the last one named when a list sets several, and a reset naming any of the four turns reporting off. RIS turns it off and DECSTR keeps it, as before. The emulator reads the tracker for both the snapshot and the input path, and no longer reads vt100's mouse mode at all. `mouse_modes()` is unchanged and still what its rustdoc says, the modes the application enabled and has not disabled: after `?1002h ?1000l` it is {ButtonMotion} while the protocol is None. DECRQM answers from that set, as #151 decided, so `?1002$p` there still reports "set" where xterm reports "reset". Left as it is. The refusal `click`, `drag` and `scroll` share said "no CSI ?9/?1000/?1002/?1003 h was seen", which is false of an application that set one mode and reset another. It now says none is in force, and that a reset of any of the four turns reporting off. The prefix the skill's failure table names is kept. Not done: xterm's mode 1001 (highlight tracking) is not modelled, so `?1001l` does not end reporting here as it does in xterm, and `?1001h` does not replace the protocol. Evidence, on de7ee37 before the change: emu::vt100::tests::a_reset_of_another_tracking_mode_turns_reporting_off assertion `left == right` failed: ^[[?1002h^[[?1000l left: ButtonMotion right: None tests/input.rs a_reset_of_another_tracking_mode_turns_reporting_off after ?1002h ?1000l: left: ButtonMotion right: None Both pass now, with emu::seq::tests::the_mouse_protocol_is_the_last_mode_set_and_any_reset_ends_it for the tracker's rule. Two comments in each_tracking_mode_is_answered_on_its_own_evidence credited vt100 with turning reporting off; its assertions are unchanged. Closes #693 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
`unsupported()` listed queries the responder answers or names in the timeout: `CSI ? 6 n` (DECXCPR, answered `^[[?1;1R`), `CSI > q` and `CSI > 0 q` (XTVERSION), `CSI = c` (DA3), `CSI ? Ps n` (`?996n`, `?5n`) and `OSC 12 ; ?`. The rustdoc of `Screen::unsupported` says the queries termlens answers or names are not listed. The CSI filter exempted only the unprefixed `c` and `n`, and the OSC filter only `10;?` and `11;?`. The filter now exempts the prefixed forms in the shapes `seq.rs` claims them in: every `CSI ? Ps n` and `CSI = Ps n` (`seq.rs` names any `n` whose prefix is not `>`, after #692 in the next commit; `=` was named already and stays so), `CSI = c` with any parameters, `CSI > q` only with no parameter or 0, as `seq.rs` classifies it, and `OSC 12 ; ?` in its exact two-operand form, like `10;?` and `11;?`. `CSI > 1 q` and `OSC 12 ; #ff0000` stay on record. One pinned test changed: parameterless_sequences_are_recorded_without_ a_synthetic_zero used `CSI > q` as its `>`-prefixed parameterless shape and expected `^[[>q`. It now uses `CSI > m` (XTMODKEYS reset, which nobody honours) and expects `^[[>m`; what it tests is unchanged. Evidence, on 2073df5 before the change: emu::unhandled::tests::the_prefixed_queries_the_responder_answers_or_ names_are_not_reported [("DECXCPR, answered", "^[[?6n"), ("XTVERSION, named", "^[[>q"), ("XTVERSION with its 0, named", "^[[>q"), ("DA3, named", "^[[=c"), ("a private DSR, named", "^[[?996n"), ("another private DSR, named", "^[[?5n"), ("the cursor colour query, named", "^[]12;?")] Every row of the issue's table was listed. It passes now, and so do tests/unsupported.rs and tests/queries.rs. Closes #691 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Two errors in the question table, in opposite directions. `CSI > Ps n` is XTMODKEYS's "disable key modifier options" (ctlseqs, "CSI > Ps n"), a setting with no reply. The classifier's `(_, b'n')` made every prefix of `n` a DSR, so `CSI > 4 n` was named in the timeout as a query the application might be blocked on. The DSR row now takes no prefix, `?` or `=`; `CSI > Ps n` is not a question, and it stays in `unsupported()`, where it was already listed. `CSI 15 t` (the screen's size in pixels) and the `; 2` forms of the position and pixel-size reports, `CSI 13 ; 2 t` and `CSI 14 ; 2 t`, are xterm questions (charproc.c, ewGetScreenSizePixels, and the `zero_if_default(1)` case 2 of ewGetWinPosition and ewGetWinSizePixels) that termlens does not answer, and none was named: the table held 11, 13, 19, 20 and 21, one parameter each. The issue names 15 and 14;2; 13;2 is the same documented form of the report beside it, so it is named too. The set is now one function, `seq::is_window_report`, which `unhandled.rs` reads for its exemption instead of keeping a copy of the list, so the three are no longer also listed in `unsupported()` (they were). A second parameter xterm gives no meaning to (`11;2 t`, `18;0 t`) is unchanged: not named, and listed. Evidence, on 6ad1e6e before the change: emu::seq::tests::xtmodkeys_disable_is_not_a_question ^[[>4n left: [Unanswerable("^[[>4n")] right: [] emu::seq::tests::every_xterm_window_report_is_a_question ^[[15t left: [] right: [Unanswerable("^[[15t")] emu::unhandled::tests::title_stack_operations_are_reported_but_ window_reports_are_not assertion failed: shapes(b"\x1b[15t\x1b[14;2t\x1b[13;2t").is_empty() All pass now. emu::unhandled::tests::xtmodkeys_disable_is_unsupported passed before as well and pins that `CSI > Ps n` stays on record. Closes #692 Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
unsupported() kept every unhandled OSC whole, so an iTerm2 inline image (OSC 1337 ; File=... : <base64>) left its payload in the list, as far as the 64 KiB the tracker lets through since #694, once per image up to the 32 entries, in every snapshot. An entry that long names nothing a reader can use. An OSC entry is now ^[] and the first 64 bytes of its content (MAX_STRING_SHAPE), ending in "…" where it was cut. The cut falls on a character boundary of the rendering, and at most 68 bytes of the content are joined to make it, whatever the payload's size. Two OSCs that agree in those bytes are one entry. No other string sequence reaches the list: vt100 0.18 has no callback for a DCS, APC, PM or SOS it declines, and a CSI's parameters are bounded by vte. Tests: - a_long_osc_is_kept_to_a_prefix_that_names_it (unhandled.rs): a 1 MiB OSC 1337 leaves a 70-byte entry; exactly 64 bytes is kept whole with no "…", 65 shows the cut, and a two-byte character straddling the bound is left out whole. Before: the entry was 1,048,629 bytes. - an_inline_image_is_named_in_unsupported_and_not_kept (vt100.rs): three 1 MiB inline images fed in 8 KiB reads through the tracker leave three entries, each at most the bound, each naming its image, and the text after them draws. Before: "entry 1 is 65539 bytes". Pinned tests updated, both in vt100.rs, both pinned the whole 64 KiB body as the entry to show how much of it reached vt100 (#694): - an_osc_body_past_the_capture_bound_never_reaches_the_parsers and - an_osc_at_the_capture_bound_reaches_the_parsers_whole now expect the 64-byte prefix and a "…", and read how much content reached vt100 from a test-only record of the callback (last_osc_len), so they still prove the #694 bound: exactly OSC_CAPTURE_MAX bytes. Both failed against the old whole entry and pass now. Rustdoc of Screen::unsupported, docs/LIMITATIONS.md and the CHANGELOG (Changed, with the migration) say the bound. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
The record of unanswered queries keeps eight distinct shapes. A program that sent eight unanswered DSRs, printed READY, then blocked on CSI > q had the query blocking it hidden in "and 1 more", and the verdict was judged by the eight it had moved past: "probably not why this wait failed". And the overflow counted queries, not shapes, so the ninth sent five times was "and 5 more". The state now keeps the most recent unanswered query and its read, whether or not the list had room for it. query_note names it (first when it is the cause, after the rest when it is context) and it alone decides the verdict: it is the query asked after the last output, so if the application moved past it, it moved past every one. Shapes past the bound are counted by a hash each, so a repeat is not counted again; the set is capped at 4,096 hashes, past which the note says "at least N more". The named latest query is not one of the "more". Tests: - the_query_after_the_last_output_is_named_as_the_cause (terminal.rs, unit): the issue's sequence on the responder. Before, the note was the issue's text verbatim, "(^[[?15n, ... ^[[?63n, and 1 more) ... probably not why this wait failed"; now "(^[[>q) and received no answer; if it is blocked ... this is the cause", with no count. - past_the_bound_distinct_shapes_are_counted_once (terminal.rs, unit): two shapes past the bound, each sent three times. Before: "and 6 more", ^[[14t not named. Now ^[[14t is named first and "and 1 more"; after a later read, ^[[14t is named last and the verdict is context. - the_query_after_the_last_output_is_named_as_the_cause (tests/ queries.rs, PTY, ignored on Windows like its neighbours): the issue's program through the emit fixture. Before (terminal.rs stashed): failed with the issue's note. Now passes; five runs in a row passed. It holds however the reads fall: if everything shares one read, all nine are the cause and ^[[>q is still named. No existing test pinned the note past eight shapes. DESIGN.md says how the note judges, and the CHANGELOG records it under Changed with the migration. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five fixes to what the sequence tracker reports, one commit each. Four change what a test can see, so they are CHANGELOG Changed entries with migration sentences.
really_set_mousemodedoes and asScreen::mouse_mode's rustdoc and DESIGN say. AfterCSI ?1002h CSI ?1000l,mouse_mode()still saidButtonMotionandclicktyped a report, because vt100 ends reporting only on a reset of the mode it holds. The protocol is now derived in the tracker: the mode set last, and none after a reset of?9,?1000,?1002or?1003.mouse_modes()and the DECRQM replies are unchanged.unsupported()lists queries termlens answers or names in the timeout #691.unsupported()no longer lists the prefixed queries termlens answers or names in the timeout (CSI ? 6 n, otherCSI ? Ps nandCSI = Ps n,CSI = c,CSI > q,OSC 12 ; ?), asScreen::unsupported's rustdoc says.CSI > Ps nis called an unanswered query, whileCSI 15 tandCSI 14 ; 2 tare never named #692. The timeout note names xterm's window reportsCSI 15 t,CSI 13 ; 2 tandCSI 14 ; 2 t, which termlens does not answer, and no longer namesCSI > Ps n(XTMODKEYS "disable", a setting with no reply).unsupported()keeps each unhandled OSC whole, so inline images are retained in full #695.unsupported()keeps an OSC to^[]plus the first 64 bytes of its content, ending in…where it was cut (never inside a character). An iTerm2 inline image left its payload in the list, up to 64 KiB each, 32 times over. vt100 0.18 reports no other string sequence there, and vte bounds CSI parameters. The two An unterminated OSC string grows the test process's memory without bound #694 tests that pinned the whole 64 KiB entry now expect the cut one and still prove An unterminated OSC string grows the test process's memory without bound #694's bound.tests/queries.rs, which failed with the issue's note before.Each fix has a test that failed before it. main is merged in, and the merged tree passes clippy, the library tests and the queries, unsupported, input and state suites; the agent's full preflight was 40 ok, 1 skipped (zizmor without pipx), 0 failed.
Closes #690
Closes #691
Closes #692
Closes #693
Closes #695