Underline styles and colors - core part#2751
Conversation
jerch
changed the title
jerch
added
area/parser
type/enhancement
Tyriar
left a comment
Tyriar
left a comment
There was a problem hiding this comment.
Looks good after the minor issues are resolved 👍, next PR would be adding rendering?
| isUnderlineColorRGB(): boolean; | ||
| isUnderlineColorPalette(): boolean; | ||
| isUnderlineColorDefault(): boolean; | ||
| getUnderlineStyle(): number; |
There was a problem hiding this comment.
Just to clarify, all underline styles will still result in the isUnderline public API being non-zero?
There was a problem hiding this comment.
The idea here for renderer side is to first test for isUnderline, if that evals to true, you'd have to request the style and the color to finally draw it. This way it is compatible to our current code, which only relies on a isUnderline test and always draws a single line.
The color methods cannot be used to determine whether underline is in place, as they would always report a color (fg color). The style method can be used for that, yes.
| public content = 0; | ||
| public fg = 0; | ||
| public bg = 0; | ||
| public extended: IExtendedAttrs = new ExtendedAttrs(); |
There was a problem hiding this comment.
So the approximate memory increase per cell will be 1 pointer for this, and then the standard ExtendedAttrs object will get shared between all cells?
There was a problem hiding this comment.
But only for cells, that actually have extended attribs set. All other (thus most cells) dont use more memory in the buffer. To get this working I used another bit in bg as indicator whether a cell holds extended attrs (HAS_EXTENDED flag).
Yepp. But thats non trivial, as you already might have guessed from the comments in #1145. Gonna fix the issues above in the next couple of days, but prolly wont make it to the renderer parts 😞 |
|
@jerch no worries, we should still merge to avoid conflicts piling up |
Adds support for colon `:` separated sub parameters in parser. Technically, after this PR, nothing should change except, now sub parameters are parsed, stored safely and we don't invalidate the whole sequence when a `:` is received within a parameter substring. In this PR: - If sub parameters are detected with a parameter, but the usage is unrecognised, we simply *skip* the parameter in `adaptDispatch`. - A separate store for sub parameters is used to avoid too many changes to the codebase. - We currently allow up to `6` sub parameters for each parameter, extra sub parameters are *ignored*. - Introduced `VTSubParameters` for easy access to underlying sub parameters. > **Info**: We don't use sub parameters for any feature yet, this is just the core implementation to support newer usecases. ## Validation Steps Performed - [x] Use of sub parameters must not have any effect on the output. - [x] Skip parameters with unexpected set of sub parameters. - [x] Skip sequences with unexpected set of sub parameters. References #4321 References #7228 References #15599 References xtermjs/xterm.js#2751 Closes #4321
2 participants
PR to support additional underline styles and colors in
AttributeData. Implements core parts of #1145.Note that this PR extends
AttributeDatain a generic way, which can be use to store other data later on (like #1134).Changes to the internal interface:
The color and the color mode accessors transparently map to the FG variants, if no independent color was set or underline is unset. Thus its important to test beforehand for
isUnderline.TODO:
AttributeDatato expose extended attributesBufferLineto hold and export extended attributes