- Notifications
You must be signed in to change notification settings - Fork1.8k
add class for blinking text control code (\x1b[5m) to DOM renderer#5119
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Conversation
Use main document to create viewport element
Update node-pty, use conpty.dll
| UNDERLINE_CLASS='xterm-underline', | ||
| OVERLINE_CLASS='xterm-overline', | ||
| STRIKETHROUGH_CLASS='xterm-strikethrough', | ||
| BLINK_CLASS='xterm-blink', |
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.
We'd also want the associated CSS class for this in xterm.css if we went with a class
| UNDERLINE_CLASS='xterm-underline', | ||
| OVERLINE_CLASS='xterm-overline', | ||
| STRIKETHROUGH_CLASS='xterm-strikethrough', | ||
| BLINK_CLASS='xterm-blink', |
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.
We should also add an implementation on the webgl side for this so we're not merging in a partially complete feature. Probably the easiest way to implement this in such a way that would share the implementation across multiple renderers is to leverage the decorations service and have an interval to set the transparency of the decoration on and off periodically:
xterm.js/src/common/services/Services.ts
Lines 363 to 368 incb3bd46
| exportinterfaceIDecorationServiceextendsIDisposable{ | |
| serviceBrand:undefined; | |
| readonlydecorations:IterableIterator<IInternalDecoration>; | |
| readonlyonDecorationRegistered:Event<IInternalDecoration>; | |
| readonlyonDecorationRemoved:Event<IInternalDecoration>; | |
| registerDecoration(decorationOptions:IDecorationOptions):IDecoration|undefined; |
| UNDERLINE_CLASS='xterm-underline', | ||
| OVERLINE_CLASS='xterm-overline', | ||
| STRIKETHROUGH_CLASS='xterm-strikethrough', | ||
| BLINK_CLASS='xterm-blink', |
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'm not sure what the right default is for this, but regardless for accessibility reasons we will want to allow the embedder to disable blinking text. So I think we need some new option to drive this.
| constisWindows=process.platform==='win32'; | ||
| constterm=pty.spawn(isWindows ?'pwsh.exe' :'bash',[],{ | ||
| name:'xterm-256color', | ||
| cols:cols??80, | ||
| rows:rows??24, | ||
| cwd:process.platform==='win32' ?undefined :env.PWD, | ||
| cwd:isWindows ?undefined :env.PWD, | ||
| env, | ||
| encoding:USE_BINARY ?null :'utf8' | ||
| encoding:USE_BINARY ?null :'utf8', | ||
| useConpty:isWindows, | ||
| useConptyDll:isWindows, |
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.
Good change, you can make a separate PR this node-pty stuff if you want to get it merged quickly.
My company happened to need the blinking text escape code, so I added it to the DOM renderer.