Add support for parsing Underlined and Doubly Underlined to the parser #2916
Comments
zadjii-msft
added
Product-Conhost
ghost
added
the
Needs-Triage
|
Since you did move bold into |
DHowett-MSFT
removed
the
Needs-Triage
|
Re underline: The Kitty terminal emulator came up with the awesome idea of supporting curly and colored underlines, with the obvious intent of supporting user-friendly spell checking in terminal-based text editors. The choice of the escape sequence was coordinated between Kitty and VTE. The feature was then implemented so far at least in Kitty, VTE, Mintty, Hterm, probably a few more as well, and feature requests are filed to even more, including iTerm2, Konsole, xterm.js. Some have even added dotted and dashed underlines, too. It would be lovely if you also considered these extensions. (A bit of technical info: With truecolor support I assume you already have like 25 bits for the foreground and background color each (in order to be able to store the 256 legacy palette values as well as "default", in addition to 24 bit RGB). At least this is how we do in VTE. And we didn't want to waste another 25 bits for this rarely used feature. So we approximate truecolor underline colors to 4+5+4 bits of R, G, B, respectively. This way all the color information of a charcell fits in an int64.) |
|
Re faint: (I should really get a Windows and try out your terminal :D It's definitely on my todo list. Sorry in advance if I make assumptions about Windows Terminal that aren't correct.) "Bold" has a lot of legacy confusion whether it's actually "bold", "bright" or both. With truecolor support in many terminals nowadays, and still no unambiguous way of encoding bold, Kitty and VTE decided to push for cleaning up this legacy mess, making "bold" (SGR 1) only stand for bold and not tamper with the colors. This is Kitty's only supported operation, and we also made this mode the default recently in VTE / GNOME Terminal. xterm and urxvt also support this behavior. (Let's also note that the popular Solarized color scheme requires this mode.) "Faint" is even worse. While most terminals allow the user to configure the 16 basic colors (the 8 most basic ones, plus their With our separation of bold vs. colors, the faint property doesn't fit in this picture. Kitty doesn't implement it for this reason, and in VTE we also thought about deprecating/removing it. In fact, the behavior that would fit in this new model is to use an even thinner font than the default, without tampering with the color. This is limited by only few fonts supporting thin typeface, whereas for ones that do (like variable fonts) this kind of asks for a generic extension towards supporting even more weights. What I have in my mind is something along the lines of Please read our more detailed Thoughts about faint (SGR 2) for juicy details how it's related to the cathode ray and how it all doesn't make any sense with dark-on-bright color schemes. What I recommend you to do is: If you haven't already, please implement a mode where I also encourage you to make this mode the default, like we did in VTE, but of course it's something that you need to evaluate yourselves if you're ready for this, or if you'd prefer to stay a more backwards-compatible terminal by default. Treat "faint" with low priority, I honestly hardly ever saw any project using this attribute, and consider going with a thinner font if feasible. But don't feel bad if you don't support this property at all :-). |
## Summary of the Pull Request Adds support for Italics, Blinking, Invisible, CrossedOut text, THROUGH CONPTY. This does **NOT** add support for those styles to conhost or the terminal. We will store these "Extended Text Attributes" in a `TextAttribute`. When we go to render a line, we'll see if the state has changed from our previous state, and if so, we'll appropriately toggle that state with VT. Boldness has been moved from a `bool` to a single bit in these flags. Technically, now that these are stored in the buffer, we only need to make changes to the renderers to be able to support them. That's not being done as a part of this PR however. ## References See also #2915 and #2916, which are some follow-up tasks from this fix. I thought them too risky for 20H1. ## PR Checklist * [x] Closes #2554 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated <hr> * store text with extended attributes too * Plumb attributes through all the renderers * parse extended attrs, though we're not renderering them right * Render these states correctly * Add a very extensive test * Cleanup for PR * a block of PR feedback * add 512 test cases * Fix the build * Fix @carlos-zamora's suggestions * @miniksa's PR feedback
## Summary of the Pull Request Adds support for Italics, Blinking, Invisible, CrossedOut text, THROUGH CONPTY. This does **NOT** add support for those styles to conhost or the terminal. We will store these "Extended Text Attributes" in a `TextAttribute`. When we go to render a line, we'll see if the state has changed from our previous state, and if so, we'll appropriately toggle that state with VT. Boldness has been moved from a `bool` to a single bit in these flags. Technically, now that these are stored in the buffer, we only need to make changes to the renderers to be able to support them. That's not being done as a part of this PR however. ## References See also #2915 and #2916, which are some follow-up tasks from this fix. I thought them too risky for 20H1. ## PR Checklist * [x] Closes #2554 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated <hr> * store text with extended attributes too * Plumb attributes through all the renderers * parse extended attrs, though we're not renderering them right * Render these states correctly * Add a very extensive test * Cleanup for PR * a block of PR feedback * add 512 test cases * Fix the build * Fix @carlos-zamora's suggestions * @miniksa's PR feedback (cherry picked from commit dec5c11)
Personally I think this is a mistake, and I really hope Windows doesn't follow this approach. Interpreting SGR 1 as an increased intensity or brightness has been the expected behavior in terminals and terminal emulators going back decades now. Dropping that support is just going to cause things to break (the Kitty issue you linked to being a case in point). And if we eventually do support the concept of bold as a font weight, I hope we have the option to turn that off, because again I think that will be an undesired behavior in legacy apps that aren't expecting it. |
|
Whoops, I did not notice that WT does not support bold typeface yet. If WT intends to be a modern terminal, with a feature set comparable to let's say GNOME Terminal, Tilix, Konsole, iTerm2 etc. (as far as I understand it does intend) then adding support for bold (and while at it, italic) typeface is a must. (Without that, my suggestion would be to make SGR 1 a no-op, this is clearly not what I'm suggesting.) An option for SGR 1 doing bold only, and not altering the color, is an absolute necessity for the users of certain color schemes, including the popular Solarized. It expects the 16 colors to be absolutely independent from each other, never as a side effect of another operation switching from one to another. Also, its "bright" colors are not at all bright counterparts of the "normal" ones. This option is also desireable for those who want a saner environment, e.g. want to access a bold but not bright variant of the 8 basic colors.
This sounds pretty subjective to me. Most graphical terminal emulators do bold+bright, so I might as well argue that SGR 1 doing bold is expected, yet WT doesn't do it. The popular screen drawing library ncurses calls the relevant attribute A_BOLD, no mention of brightness in its name. That is, anyone coding against ncurses probably expects bold. Expectation is one thing, common behavior is another, the official standard is yet another. I know it's generally hard to decide what to do if they don't quite match. I'd argue that "bright" or "bold+bright" behavior is a leftover from the days with limited hardware capabilities, including limited color palette. Even with the introduction of the aixterm 16-color palette (the addition of SGR 90..97/100..107) it no longer makes sense, let alone if you extend to 256 or 16M colors. Also, if it's meant to do "bright", why does it only do so for the basic 8 colors, why not for the rest of the 16- or 256-color palette entries, or even truecolor ones? (Nitpicking: and why bright foreground only and not bright background too? There's not a word about it in the specs.)
Yes, introducing this behavior breaks things. For example, as mentioned in one of the VTE bugreports, the "make menuconfig" step of compiling the Linux kernel doesn't draw borders now. Taking a look at its source, it makes not one but two faulty assumptions that happened to work due to testing with only one particular interpretation of the specification. (If a spec says "this or that", you shouldn't write code against it expecting that it'll always be "this", correct?) We've changed VTE's behavior more than half a year ago, and roughly 50% of terminals emulator users on Linux use some VTE-based one, meaning that this change has already reached a pretty significant market share. Yet this is the most serious bug we've heard about. The generic perception of the change was pretty good, follow my comments at Alacritty 2776 and 2779 for details. And most importantly: any such new bug like the Linux kernel "make menuconfig" borders is fixable from the application's side. Contrary, the legacy behavior also has bugs, but those bugs are not fixable from the application's side. Solarized color scheme becomes pretty much unusable (or at least you need to completely avoid bold typeface). There's no access to bold+dark. With dark-on-bright default colors the two concepts of bold and bright that are enabled by the single SGR 1 actually work against each other, defeating any sane purpose SGR 1 might have. No one knows which of the two colors to make brighter in combination with the "reverse" attribute. And so on...
Compatibility is usually pretty important. In some cases, when it stands in the way of further evolution of the ecosystem, I personally believe that generally modernization and bugfixing has to triumph over compatibility. With common sense and proper care being taken, of course. Here we're "just" talking about visual representation of some data, somewhat changing it is unlikely to severely break things, it's rather just causing a less pleasant look in some cases. As for standard Linux terminal emulation, the expectation is that SGR 1 switches to (at least) bold typeface. This is the behavior of most terminal emulators, and as I said, the library that most apps use also calls it "bold". You say "legacy apps that aren't expecting it", but I seriously doubt there's any such application anywhere in the Linux world. As for Windows applications, if they request let's say a bright cyan via API, you should just translate that to SGR 95 rather than SGR 1;35 and you made it independent of this entire story: it'll always be bright but not bold. Supporting bold (and italic by the way) typeface, as well as adding an option to make SGR 1 bold only and not bright, is a feature absolutely expected from any decent modern terminal emulator. Making the "SGR 1 is bold but not bright" the default is at least as much of a political decision as a technical one, so I can only weakly encourage WT developers to make this bold (pun intended) step. The experiences from VTE's switch are pretty good, there are only a few minor visual annoyances, nothing serious. I believe there are no Windows compatibility issues to worry about, you can just map bright colors to bright colors, entirely avoiding the ambiguous SGR 1. In this new world the issues are fixable, as opposed to the old world where they were not. And for anyone not liking it or running into real issues, there'd still be this option to revert to the legacy behavior. |
|
I understand all the arguments for wanting a way to get a bold typeface without bright colors, but that could just as easily have been achieved by creating a new SGR code specifically for that bold style, with less chance of breaking backwards compatibility.
Individual expectations may be subjective, but we can make a reasonable prediction of the expectations of most users on aggregate. If the majority of terminals in use today support SGR 1 as an increased intensity (regardless of whether they also support a bold typeface), and that has been the case since at least the early 80s, then I think it's safe to say that that is what most people would expect. Or are you saying the majority of terminal emulators don't support that any more? I'll admit I'm not particularly knowledgable about the current state of terminals other than XTerm.
I think you're interpreting the word "bold" to mean what you want it to mean. When referring to a font, it may mean a thicker typeface, but when referring to a color it just means a more vivid appearance, and bright is considered a synonym. The VT100 technical manual explicitly defines bold as "increases the intensity of the display".
Except when you have something like bright white on a regular white background (which is not that uncommon a choice), the text suddenly becomes invisible.
Except if you're working in the Linux text mode console where a bold typeface is simply not an option.
I'm not saying it's going to severely break things; just that some people might prefer to turn that option off (not an uncommon preference, e.g. see here or here). And it's hard to argue that these aesthetics don't matter while also arguing that it's absolutely essential to have SGR 1 not alter the color for people using the Solarized color scheme.
Not all Windows applications would be using the API to set colors though. Legacy apps are quite likely to be using the simple escape sequences that were supported in ANSI.SYS and ANSICON. There may not be a lot of them, and there may not be any desire to support them any more, but let's not pretend that they don't exist. Anyway, as long as these things are preferences that the user can change, I don't think it matters that much. But I understand you're not even allowing SGR 1 brightness as an option in VTE any more, and I'd strongly object to us doing the same thing in Windows. |
Going for a brand new SGR code sure could have been another approach. "just as easily" – I don't think so. That approach would have needed buy-in from more key players of the game, some of which I have good reason to assume that wouldn't have been supportive, and I assume would have led to much slower adoption. Anyway, discussing how it could have been done differently in the past won't take us forward.
I'm not saying that. I'm saying that it's no longer the default in a pretty significant fraction (soon to be in the ballpark of 50%, concerning market share within Linux).
I have to admit I haven't come across VT100's definition as "bright". I'm solely using "bold" in the sense that modern graphical applications use it: increased font weight.
During the last half a year, we haven't received any report about this (except for the dialog border I've already mentioned). So it may be a bit less common than you think, at least across Unix apps.
Do people use the Linux text mode console for productive work (I mean, not firefighting or disaster recovery)? I can't really imagine why anyone would do so.
That purple screenshot made me cringe. Get a terribly looking font and then complain that it's terrible? Get a nice font (see e.g. #3226 (comment)) and it's no longer really an issue. :-) But again it's mostly a question of project vision: where does WT want to go? Is it legacy and backwards compatibility, or being a decent modern future environment that is more important? Does it want to play safe, or be brave? And why would be the terminal emulator the only piece of software where the user can disable bold fonts? (In gnome-terminal we demoted this user-visible setting to a hidden one about two years ago, I can't recall anyone asking where it went, and just removed now in the development series to see if people really care.) Anyway, I'm absolutely fine if WT adds bold typeface support along with a kill config option. Or maybe a tri-state: SGR 1 is bold, bright, or both. I most definitely do not want WT to remove the legacy compatibility behavior thereof.
This is not true. We do have that option, we just flipped the default. |
|
Thinking further about this...
I have to admit I completely missed this. I think we should conclude that for SGR 1 the expectation is:
When I'm wondering... wherever this magic happens, could it also switch the behavior of SGR 1? Either via some API-like thingy, or via a newly invented "allow/disallow bold" escape sequence? Could this be a reasonable solution to support both worlds? |
|
For what it's worth I've extended this a little, and my programs support the following subparameters:
|
|
FYI, I've been doing a bit of research into how these attributes are implemented in other terminals, and the results are rather interesting.
This isn't actually true in most cases (at least amongst my terminal collection). When bold and faint are applied at the same time, the font weight typically remains bold (if supported), and the bright palette entry for ANSI colors is still used, but that "bright" color is then algorithmically dimmed. The effect of the color changes is most obvious in a palette like Solarized, where "bright green" is actually a shade of gray. So when you have faint combined with bright green, you get a fainter shade of grey, rather than the dark green.
Yes and no. There aren't many terminals that support double-underline for real (most treat it as a synonym for single underline) so there's no obvious consensus. In VTE it's a trinary state, where the last choice (single or double) takes precedence. However in XTerm, underline and double-underline are actually separate attributes, and double-underline takes precedence when both are set, regardless of the order they were applied. I'm not sure it matters that much, but I'd probably be inclined to follow the XTerm approach. |
DHowett
changed the title
ghost
added
Resolution-Fix-Committed
…7223) This PR adds support for the ANSI _doubly underlined_ graphic rendition attribute, which is enabled by the `SGR 21` escape sequence. There was already an `ExtendedAttributes::DoublyUnderlined` flag in the `TextAttribute` class, but I needed to add `SetDoublyUnderlined` and `IsDoublyUnderlined` methods to access that flag, and update the `SetGraphicsRendition` methods of the two dispatchers to set the attribute on receipt of the `SGR 21` sequence. I also had to update the existing `SGR 24` handler to reset _DoublyUnderlined_ in addition to _Underlined_, since they share the same reset sequence. For the rendering, I've added a new grid line type, which essentially just draws an additional line with the same thickness as the regular underline, but slightly below it - I found a gap of around 0.05 "em" between the lines looked best. If there isn't enough space in the cell for that gap, the second line will be clamped to overlap the first, so you then just get a thicker line. If there isn't even enough space below for a thicker line, we move the offset _above_ the first line, but just enough to make it thicker. The only other complication was the update of the `Xterm256Engine` in the VT renderer. As mentioned above, the two underline attributes share the same reset sequence, so to forward that state over conpty we require a slightly more complicated process than with most other attributes (similar to _Bold_ and _Faint_). We first check whether either underline attribute needs to be turned off to send the reset sequence, and then check individually if each of them needs to be turned back on again. ## Validation Steps Performed For testing, I've extended the existing attribute tests in `AdapterTest`, `VTRendererTest`, and `ScreenBufferTests`, to make sure we're covering both the _Underlined_ and _DoublyUnderlined_ attributes. I've also manually tested the `SGR 21` sequence in conhost and Windows Terminal, with a variety of fonts and font sizes, to make sure the rendering was reasonably distinguishable from a single underline. Closes #2916
|
🎉This issue was addressed in #7223, which has now been successfully released as Handy links: |
…10759) ## Summary of the Pull Request This adds a new setting `intenseTextStyle`. It's a per-appearance, control setting, defaulting to `"all"`. * When set to `"all"` or `["bold", "bright"]`, then we'll render text as both **bold** and bright (1.10 behavior) * When set to `"bold"`, `["bold"]`, we'll render text formatted with `^[[1m` as **bold**, but not bright * When set to `"bright"`, `["bright"]`, we'll render text formatted with `^[[1m` as bright, but not bold. This is the pre 1.10 behavior * When set to `"none"`, we won't do anything special for it at all. ## references * I last did this in #10648. This time it's an enum, so we can add bright in the future. It's got positive wording this time. * ~We will want to add `"bright"` as a value in the future, to disable the auto intense->bright conversion.~ I just did that now. * #5682 is related ## PR Checklist * [x] Closes #10576 * [x] I seriously don't think we have an issue for "disable intense is bright", but I'm not crazy, people wanted that, right? #2916 (comment) was the closest * [x] I work here * [x] Tests added/passed * [x] MicrosoftDocs/terminal#381 ## Validation Steps Performed <!--  -->  Yea that works. Printed some bold text, toggled it on, the text was no longer bold. hooray. ### EDIT, 10 Aug ```json "intenseTextStyle": "none", "intenseTextStyle": "bold", "intenseTextStyle": "bright", "intenseTextStyle": "all", "intenseTextStyle": ["bold", "bright"], ``` all work now. Repro script: ```sh printf "\e[1m[bold]\e[m[normal]\e[34m[blue]\e[1m[bold blue]\e[m\n" ```
…10759) This adds a new setting `intenseTextStyle`. It's a per-appearance, control setting, defaulting to `"all"`. * When set to `"all"` or `["bold", "bright"]`, then we'll render text as both **bold** and bright (1.10 behavior) * When set to `"bold"`, `["bold"]`, we'll render text formatted with `^[[1m` as **bold**, but not bright * When set to `"bright"`, `["bright"]`, we'll render text formatted with `^[[1m` as bright, but not bold. This is the pre 1.10 behavior * When set to `"none"`, we won't do anything special for it at all. * I last did this in #10648. This time it's an enum, so we can add bright in the future. It's got positive wording this time. * ~We will want to add `"bright"` as a value in the future, to disable the auto intense->bright conversion.~ I just did that now. * #5682 is related * [x] Closes #10576 * [x] I seriously don't think we have an issue for "disable intense is bright", but I'm not crazy, people wanted that, right? #2916 (comment) was the closest * [x] I work here * [x] Tests added/passed * [x] MicrosoftDocs/terminal#381 <!--  -->  Yea that works. Printed some bold text, toggled it on, the text was no longer bold. hooray. ```json "intenseTextStyle": "none", "intenseTextStyle": "bold", "intenseTextStyle": "bright", "intenseTextStyle": "all", "intenseTextStyle": ["bold", "bright"], ``` all work now. Repro script: ```sh printf "\e[1m[bold]\e[m[normal]\e[34m[blue]\e[1m[bold blue]\e[m\n" ```
Discussed with @DHowett-MSFT, as a follow up of #2554. See also #2915.
In #2554 I'm not adding support for parsing these additional extended states, because their implementation seems closely tied to more complicated existing implementations.
Faint cannot be turned on at the same time as BoldDue to the timing with the upcoming 20H1 release, I felt it too risky to try and implement those as well. I've left room in the
ExtendedAttributesenum for these states, but currently, Doubly-Underlined and Faint are totally ignored.We'll need to parse and store them in the buffer correctly. It's another task entirely to support rendering these things correctly. I believe the rendering for underline state will be satisfied with #2915.
Faint might just be okay too, considering it's like the opposite of Bold.The text was updated successfully, but these errors were encountered: