ConPTY swallows SGR 3 and 23, so terminals like Alacritty are prevented from supporting italics #2554
Comments
ghost
added
Needs-Triage
zadjii-msft
added
Issue-Bug
DHowett-MSFT
removed
the
Needs-Triage
|
Alright, so we've got serious progress here. Here are some before, after screenshots:
(imagine the "Blinking" on the right blinking). I need to add some tests, but not we'll render italics, blinking, invisible, crossed-out all through conpty. Unfortunately, double-underline and faint are a bit more complicated, so they aren't going to make the 20H1 cut, but I'll circle back with follow-up issues for them. |
|
Thanks, sounds good! I presume these things all combine properly with bold, which isn’t depicted, as well. Is it possible to pass unimplemented things through, even if they’re not rendered by Command Prompt or Windows Terminal? Not that I actually have any use for double-underline and faint. |
|
Unfortunately double-underline and faint have some implementations that would run closer to some other areas. I might be able to pass them through, but implementing them (in the buffer) is more complicated than the others in this set. Bold does work fortunately: |
|
Great, thanks! I’m looking forward to receiving this. 🙂 |
## 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
ghost
added
Resolution-Fix-Committed
## 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)
|
🎉This issue was addressed in #2917, which has now been successfully released as Handy links: |
|
Hey! The fix for this went out in Windows in the Insider channel's Fast Ring with build 19013. |
Windows 10 version 1903, build 10.0.18362.295. Alacritty master.
Using Alacritty as the terminal (since it supports bold and italic text rendering), italic doesn’t work, because it seems to gets swallowed by ConPTY. alacritty/alacritty#2745 has some discussion.
Steps to reproduce
Run the following in Bash or similar under Alacritty, with the
enable_experimental_conpty_backendsetting enabled:(If you don’t have that setting enabled, it’ll use WinPTY which also doesn’t pass italics through, but that’s irrelevant here.)
Expected behavior
Output like this:
Actual behavior
Italics don’t occur:
Further, when I insert logging into Alacritty’s
csi_dispatchmethod, to observe what SGR codes it’s actually hearing about and applying, I see that the 3 (italic) and 23 (stop italic) have been swallowed, presumably by ConPTY, and Alacritty never saw them.(ConPTY also modifies the
CSI [ 1 mbold code to add another SGR code to change to the high-intensity colour. I still haven’t decided how I feel about that!)I don’t know much about how these things work, so I don’t know if this is normal, but I was caught by surprise that ConPTY was apparently rendering things before they ever got to Alacritty, so that terminals are limited in what they can implement by what ConPTY implements. I expected it to pass everything through untouched, or at least to pass unknown things through rather than swallowing them.
The text was updated successfully, but these errors were encountered: