-
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
Use IBufferService for buffer access in InputHandler #2304
Conversation
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.
LGTM, one third less _terminal
refs 👍
Seems cleaning up InputHandler
will take some more iterations, we still have over 250 refs there (mostly .updateRange
and cols/rows access). Most of the basic flags could go into CoreService I guess? Also the mouse flags - seems we maintain like 50 separate flags here...
src/InputHandler.ts
Outdated
@@ -41,7 +41,10 @@ const GLEVEL: {[key: string]: number} = {'(': 0, ')': 1, '*': 2, '+': 3, '-': 1, | |||
class DECRQSS implements IDcsHandler { | |||
private _data: Uint32Array = new Uint32Array(0); | |||
|
|||
constructor(private _terminal: any) { } | |||
constructor( | |||
private _terminal: any, |
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.
Saw that this comes from me, wonder why I typed it as any
in the first place. 🤔
Imho DECRQSS can be fixed and (terminal removed) in a separate issue later on when we have all the needed services. Also the SGR sub command does not yet do the right thing.
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.
It was actually broken as it was using the now removed handler 😱
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.
I removed terminal usage from it completely, pretty easy
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.
Lol missing test cases, tsss. Good you spotted it 👍
updateRange should probably live in render service. I'll include cols/rows in this as well as they live on buffer service too. |
Also added a log service and added the |
This makes it easier to differentiate messages in large apps
47b004a
to
4600259
Compare
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.
Yay - InputHandler is almost "terminal free", only 157 refs to go (from over 300 or so).
@@ -87,7 +88,6 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp | |||
* The HTMLElement that the terminal is created in, set by Terminal.open. | |||
*/ | |||
private _parent: HTMLElement; | |||
private _context: Window; |
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.
Yay, window gone. Still need to get rid of the line above (and several lines below, too) 😸
Also adds the new
logLevel
option.Part of #1507
Fixes #1560
@jerch a quick once over of the non-test files would be good 😃