Move EscapeSequenceParser to core #2168
Conversation
| add(code: number, state: number, action: number | null, next: number | null): void { | ||
| this.table[state << 8 | code] = ((action | 0) << 4) | ((next === undefined) ? state : next); | ||
| add(code: number, state: number, action: number | null, next: number | undefined): void { | ||
| this.table[state << 8 | code] = ((action || 0) << 4) | ((next === undefined) ? state : next); |
There was a problem hiding this comment.
I changed action | 0 to action || 0, was this right?
There was a problem hiding this comment.
Hmm the old JS version relied on | 0 (anything cast to a number), but I think its not needed anymore (I think all add calls now have the full args list). Lemme check.
There was a problem hiding this comment.
Changed arg types to ParserAction and ParserState and removed the undefined and null types.
| @@ -279,21 +279,10 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP | |||
| } | |||
|
|
|||
| public dispose(): void { | |||
| this._printHandlerFb = null; | |||
There was a problem hiding this comment.
I think we cannot (yet) remove these as they might hold a reference from the input handler with a terminal object?
There was a problem hiding this comment.
I think in that case it will still get disposed correctly and release the reference to input handler and terminal when the consumers reference to terminal is removed
| @@ -453,7 +442,9 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP | |||
| this._printHandler(data, print, i); | |||
| print = -1; | |||
| } else if (~dcs) { | |||
| dcsHandler.put(data, dcs, i); | |||
| if (dcsHandler) { | |||
There was a problem hiding this comment.
Was there an error regarding this? In theory dcsHandler should never be null here (if so the parser has a serious logic flaw). I am abit unhappy about dcsHandler being null in between, guess this needs some refactoring (to reduce branching).
Well its good for now with this addtional check.
There was a problem hiding this comment.
dcsHandler could be null according to the type system, if we're certain and want to avoid the if we could compel the type system with !, that's unsafe though.
Part of #1507
@jerch can you check out the EscapeSequenceParser.ts diff? I needed to tweak a bunch of stuff and want to make sure it's right 🙂