-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parser hooks #2346
Parser hooks #2346
Conversation
DCS is halfway done, still missing:
|
@Tyriar Imho there is no formal payload limitation spec'ed for OSC and DCS commands, instead they are meant to work streamlined with chunks (well more precisely on char by char, but thats a perf nightmare in JS thus we chunkify it similar to PRINT). I tried to deal with that by the new @Tyriar You might wonder what changed to before: Before:
Now:
Edit: Lowered the limit to 10 MB, with 50 MB nodejs segfaults (prolly due to the string conversion). |
Up for review. |
Done with the rework of function registration. To see the impact here are some benchmark numbers:
Across all functions its pretty equal. Still interesting that removing the intermediate string concat for "ESCAPE with collect - ESC % G" shows almost 3x boost while "CSI with collect - CSI ? p" is only slightly faster. OSC seems to benefit slightly from the OscParser while DCS perf decreased (prolly due looping into registered handlers, before it was a single handler). |
Introduced a new interface to the API for the callback registration: export interface IFunctionIdentifier {
prefix?: string;
intermediates?: string;
final: string;
} This data type is used during callback registration of CSI, DCS and ESC sequences on the parser to create numerical ids as the jump table keys. As shown above this greatly speeds up sequences with intermediates. Dispatching rules:
Again up for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any parts in particular that need me to review? Some of the internals of EscapeSequenceParser are a bit tough for me to interpret these days, I like that OscParser/DcsParser logic is moving out of that file though.
@Tyriar Well the payload limit of 10MB bugs me abit, but I think this can be increased with a string concat patch for chrome once we have a use case for a DCS/OSC command using that much payload. |
PR to get the parser actions hookable from outside. Shall fix #2332.
TODO:
Also introduces
OscParser
which simplifiesEscapeSequenceParser
and fixes error in OSC subparsing #2342. OSC handlers now can be attached with full state introspection (START, PUT, END) and UTF32 codepoints. The simplified attaching of a single function as handler is now done with a proxy classOscHandlerFactory
. Payload has a hard limit of 10MB.Experimental badge removed. Tests were already in place.
Interface and tests added.
Interface added similar to OSC with callback registration (again via a proxy class to catch the substates). Payload has a hard limit of 10MB.
Also closes #1823.