add class for blinking text control code (\x1b[5m) to DOM renderer #5119
Conversation
Use main document to create viewport element
Update node-pty, use conpty.dll
| @@ -24,6 +24,7 @@ export const enum RowCss { | |||
| UNDERLINE_CLASS = 'xterm-underline', | |||
| OVERLINE_CLASS = 'xterm-overline', | |||
| STRIKETHROUGH_CLASS = 'xterm-strikethrough', | |||
| BLINK_CLASS = 'xterm-blink', | |||
There was a problem hiding this comment.
We'd also want the associated CSS class for this in xterm.css if we went with a class
| @@ -24,6 +24,7 @@ export const enum RowCss { | |||
| UNDERLINE_CLASS = 'xterm-underline', | |||
| OVERLINE_CLASS = 'xterm-overline', | |||
| STRIKETHROUGH_CLASS = 'xterm-strikethrough', | |||
| BLINK_CLASS = 'xterm-blink', | |||
There was a problem hiding this comment.
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 in cb3bd46
| @@ -24,6 +24,7 @@ export const enum RowCss { | |||
| UNDERLINE_CLASS = 'xterm-underline', | |||
| OVERLINE_CLASS = 'xterm-overline', | |||
| STRIKETHROUGH_CLASS = 'xterm-strikethrough', | |||
| BLINK_CLASS = 'xterm-blink', | |||
There was a problem hiding this comment.
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.
| const isWindows = process.platform === 'win32'; | ||
| const term = 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.
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.