Repository navigation
core: add stdin tokenizer/router - #770
Merged
Merged
Conversation
@opentui/core
@opentui/react
@opentui/solid
@opentui/core-darwin-arm64
@opentui/core-darwin-x64
@opentui/core-linux-arm64
@opentui/core-linux-x64
@opentui/core-win32-arm64
@opentui/core-win32-x64
commit: |
simonklee
marked this pull request as ready for review
March 3, 2026 06:16
simonklee
marked this pull request as draft
March 5, 2026 19:39
simonklee
force-pushed
the
stdin-unified
branch
3 times, most recently
from
March 7, 2026 08:00
22ad217 to
e14b29e
Compare
simonklee
marked this pull request as ready for review
March 8, 2026 08:50
simonklee
commented
Mar 8, 2026
simonklee
commented
Mar 8, 2026
The renderer stdin path does chunk-level either/or routing: mouse-first early return or StdinBuffer processing. Mixed chunks can drop trailing input (for example mouse+key in one read), which causes silent data loss in interactive sessions. Add a flag-gated Zig stdin tokenizer/classifier pipeline and TS router while keeping legacy behavior as the default.
Parse raw terminal input into typed StdinEvent objects (key, mouse, paste, response) via a byte-oriented state machine. The parser owns key and mouse decoding directly — the renderer routes pre-parsed events without payload decoding or reparsing. Stream bracketed paste through a collector that keeps only a small tail for end-marker detection, so arbitrary paste sizes don't grow the parser buffer.
Only recover [<...M/m as a delayed escape sequence immediately after a timed-out lone ESC, so literal [< input is not misclassified.
Keep ESC b and ESC f as literal Meta chords instead of rewriting them globally, and reset parser mouse button state when mouse mode toggles or the renderer resizes. Also recover delayed X10 mouse continuations after a timed-out ESC so chunk boundaries do not leak mouse bytes as keypresses.
This was referenced Mar 9, 2026
This was referenced Mar 9, 2026
simonklee
pushed a commit
that referenced
this pull request
Mar 23, 2026
…ation (#858) ## Problem `resume()` calls `stdin.resume()` then defers listener registration into a `setImmediate`: ```ts this.stdin.resume() this.addExitListeners() setImmediate(() => { while (this.stdin.read() !== null) {} this.stdin.on("data", this.stdinListener) }) ``` The stream starts flowing immediately, but `stdinListener` isn't attached until the next macrotask. Any keystrokes arriving in that gap are silently lost — which is exactly the window where a user types after `fg`. `setupInput()` has a milder variant of the same bug: `resume()` before `on("data")`. The drain loop was introduced in #331 to discard stale input from child process suspension. The placement made it ineffective: `read()` returns null in flowing mode because data goes through events, not pull. Stale bytes were only "discarded" by accident — emitted as `data` events with no listener present. **Repro:** in the test suite, `mockInput.pressKey()` right after `renderer.resume()` is silently dropped. The existing tests masked this by yielding a macrotask (`await new Promise(r => setImmediate(r))`) before asserting. ## Fix Reorder `resume()` to: drain → register listener → resume stream: ```ts while (this.stdin.read() !== null) {} this.stdin.on("data", this.stdinListener) this.stdin.resume() ``` The ordering is load-bearing: 1. **Drain first** — `read()` pulls from the internal buffer in paused mode, discarding stale input (#331's scenario). 2. **Register listener second** — must come after the drain. Adding a `data` listener can auto-resume a paused Readable, flushing stale bytes through the listener before the drain runs. 3. **Resume last** — starts flowing mode. New data hits the registered listener with zero gap. `setupInput()` just swaps the two lines (no drain needed at init). ## Tests **Modified:** removed three `setImmediate` yield workarounds (2 in `renderer.control`, 1 in `renderer.input`). The tests now assert synchronous delivery — they'd fail if the fix were wrong. **New (4):** - "keystrokes received immediately after resume() without yielding" — core invariant. - "keystrokes survive multiple rapid suspend/resume cycles" — 5 cycles, no accumulation. - "input buffered during suspension is drained on resume" — `push()` bytes into stdin during suspension, verifies they're discarded. Directly tests the #331 scenario. - "suspend/resume does not leak stdin listeners" — 10 cycles, `listenerCount("data")` stays constant. ## Background I hit this in 0.1.88 while building a TUI on OpenTUI and patched the bundled JS. When 0.1.90 shipped, `setupInput()` had its `setImmediate` removed but the ordering was still wrong, and `resume()` still defers via `setImmediate`. I've been running a patched build since. The `setImmediate` + drain pattern was introduced in #770 (building on #331). The intent was sound — this fix keeps the drain but moves it to where `read()` actually works.
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.
Fixes #748 + related #705, #682, #727, #686, OpenCode 15760
We currently have a chunk-level either/or stdin split in
CliRenderer:That design is unsafe for mixed chunks and can silently drop input (e.g. mouse + key in same read).
Examples:
\x1b[<64;10;5Mx-> trailingxcan be droppedx\x1b[<64;10;5M-> mouse can be swallowed by non-mouse pathI want to have one canonical stdin pipeline, but i tried to add this a merge-safe way, which is why this PR is a bit larger and more of a refactor than just a straight parser swap.
mainuses a split stdin pipeline:KeyHandlerreparses strings the parser stack already understandsThis makes input harder to reason about and can lose details for mixed protocol chunks, X10 mouse payload bytes, delayed escape continuations, and legacy high-byte/meta cases. For instance, issue 661, i now realize is most likely a case of utf8 decoding happening on X10 mouse bytes
The boundary is:
parseKeypress()andMouseParserinternally and creates fully typedStdinEventobjects (key, mouse, paste, response).ParsedKeyintoKeyEvent. It is not a parser.event.typeand forwarding to the right handler.This removes:
StdinBufferstring tokenizer and its even emittersetEncoding("utf8")on stdin (decoded strings before framing, wrong order for a binary protocol)MouseParserinstance in renderer that consumed mouse bytes before they ever reached the bufferparseKeypress()call insideKeyHandler.processInput()Also:
stdinReadStream directly AFAIK, which is why the setEncoding issues have gone unnoticed, i think. I haven't addressed this yet. - Added many tests to the parser :)In one sentence: main decodes first, parses later. In the PR we parse bytes first, decode only when semantics are known.