Surface every OSC sequence, including the ones this terminal ignores - #19
Merged
Merged
Conversation
An OSC code with no case in HandleOsc reaches Debug.WriteLine and is gone. The parser already raises EscapeSequenceParser.Osc with the raw payload, but Terminal exposes neither the parser nor the InputHandler, so nothing downstream can compensate -- an embedder wanting OSC 133 shell integration, or OSC 9;9 for a cwd on Windows, has no way to see the bytes arrive. Terminal.OscReceived carries the identifier, the numeric code, the data after the first ';', and the whole payload verbatim. Recognized says whether the sequence reached a handler here, so a listener can implement only what this terminal ignores today and stop doing so by itself once a code lands in HandleOsc, instead of racing it. Note that it is about dispatch, not completeness: OSC 4 is recognized and its handler is still a TODO. Additive by construction. It fires after built-in handling rather than before, so a listener reads terminal state as settled, and nothing it does changes what the terminal already did -- there is deliberately no way to suppress or override handling from here. Tests hold the ordering and the not-disturbing. Splitting on the first ';' only is also contractual rather than incidental: OSC 9;4;1;50 and OSC 133;D;<exit> carry their own sub-parameters, and a listener cannot reconstruct them if Data has already been carved up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 24, 2026
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is additive, preserves existing behavior, and is backed by targeted tests covering the key contract points.
Pull request overview
This PR adds a public, observation-only Terminal.OscReceived event that surfaces every completed OSC sequence (including unhandled/unknown ones) after built-in handling, enabling downstream consumers to implement OSC features not yet supported by the terminal.
Changes:
- Added
Terminal.OscReceivedand internal plumbing (RaiseOscReceived) to publish OSC payload details and whether the terminal recognized/dispatched the sequence. - Updated
InputHandler.HandleOscto computeRecognizedand raise the new event after built-in OSC handling. - Added a focused test suite validating payload shape, ordering, terminators (BEL/ST), and non-interference with built-in behavior.
File summaries
| File | Description |
|---|---|
| src/XTerm.NET/Terminal.cs | Exposes the new OscReceived event, adds an internal raiser, and clears it on Dispose(). |
| src/XTerm.NET/InputHandler.cs | Raises OscReceived after OSC handling and reports whether the OSC was recognized/dispatched. |
| src/XTerm.NET/Events/TerminalEvents.cs | Introduces OscReceivedEventArgs carrying identifier/code/data/raw/recognized metadata. |
| src/XTerm.NET.Tests/OscPassthroughTests.cs | Adds test coverage for event firing semantics, payload parsing, ordering, and terminators. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+144
to
+145
| /// Observation only: this fires AFTER any built-in handling, and setting nothing on it changes | ||
| /// what the terminal did. Use <see cref="Recognized"/> to implement only what the library |
tomlm
approved these changes
Aug 24, 2026
JohnCampionJr
added a commit
to JohnCampionJr/XTerm.NET
that referenced
this pull request
Aug 24, 2026
BUILD WAS BROKEN. The merge of main into this branch dropped the closing brace of NotificationEventArgs and the opening line of the next doc comment, so every class after it nested inside it and the file would not compile. tomlm#19 and tomlm#20 both inserted at the same anchor in TerminalEvents.cs, which is what the merge had to reconcile. COMMENT DESCRIBED BEHAVIOUR THE CODE DOES NOT HAVE. It said an unrecognised sub-parameter "must not be treated as a notification", and the code does exactly that on purpose -- the notification form is OSC 9 ; text, with no sub-parameter at all, so it can only be the fallback. The real requirement is about ORDER: claimed sub-commands have to be matched first, or OSC 9;4;1;50 pops a toast reading "4;1;50" on every progress tick. Reworded to say that, and to say that an unclaimed sub-parameter being a notification is the intended reading of a permissive extension space. A CLAIMED SUB-COMMAND WITH NO PAYLOAD is now ignored rather than shown. "OSC 9;9" carries a sub-command and nothing else; the fallback turned it into a notification whose entire body was "9". A TEST FROM tomlm#19 WENT STALE, CORRECTLY. It asserted OSC 133 was unrecognised, which was true when the passthrough event was added and is not any more -- this branch is what implements it. That is the Recognized contract working rather than a test rotting: a listener filling the gap stops doing so once a code lands in HandleOsc. Moved to OSC 1337, which really is unimplemented, and added the opposite assertion for 133 so both halves are pinned. 698 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
The problem
An OSC code with no
caseinHandleOscreachesDebug.WriteLineand is gone:EscapeSequenceParseralready raises anOscevent with the raw payload, butTerminalexposes neither the parser nor theInputHandler, so nothing downstream can compensate. An embedder who wants OSC 133 shell integration, or OSC 9;9 for a working directory on Windows, currently has no way to even see the bytes arrive.The change
carrying
Identifier,Code(-1 when non-numeric),Data(after the first;),Raw, andRecognized.Recognizedreports whether the sequence reached a handler here, so a listener can implement only what this terminal ignores today and stop doing so on its own once a code lands inHandleOsc, rather than racing it. It describes dispatch, not completeness — OSC 4 is recognized and its handler is still aTODO.Additive by construction
Splitting on the first
;only is contractual rather than incidental:OSC 9;4;1;50andOSC 133;D;<exit>carry their own sub-parameters, and a listener cannot reconstruct them ifDatahas already been carved up.Testing
9 new tests in
OscPassthroughTests.cscovering unknown and known codes, ordering, non-interference, non-numeric identifiers, empty payloads, embedded semicolons, multiple sequences, andSTas well asBELtermination.Full suite: 616 passed, 0 failed.
Context
This is one of two independent PRs; neither is stacked on the other. The companion adds first-class handling for OSC 9 and OSC 133. This one stands on its own regardless of that: it is the general escape hatch, and it means the next unimplemented OSC code does not need a library round-trip.
🤖 Generated with Claude Code