-
Notifications
You must be signed in to change notification settings - Fork 102
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Question/feature request: better support for ignoring unrecognized SGR instructions #178
Comments
Thanks for the detailed bug report @jfly! I'm fine with adding logic for skipping SGR codes in the way you described. Please do send a PR (and include a few tests 😉 )! |
Now we do the math as we find each digit, rather than at the end after we have collected each digit. It's possible this is faster than extending a str and doing an `int(...)` conversion at the end (I haven't profiled it), but that's not why I made this change. I made this change to support an upcoming change to support SGR parameter "context", which is when a SGR CSI parameter is actually a list of integers: <selectel#178>. It's nicer to be able to collect the integers as we go, rather than having to process them all at the end.
This fixes selectel#178. Before, we assumed that CSI parameters are only integers. However, that caused us to barf in weird ways when presented with CSI parameters that contain `:`-delimited subparameters. This update to the parsing code causes us to parse those subparameters, and then immediately discard them. That may seem kind of weird, but I'm laying the groundwork for a followup change to the SGR handling code to actually be aware of subparameters (selectel#179). I just didn't want to muddy this diff with that change as well.
Thanks, @superbobry! I've put together a PR here: #180. If you're ok with the approach there, I think it would be pretty straightforward to do another PR that reworks pyte to actually do something useful with these |
First off, apologies if I'm confusing any terminologies here. I've only recently started digging into the messy historical universe of TTYs. It's entirely possible this is an issue with how I'm using pyte, and not anything that needs to change in pyte itself.
I'm using termtosvg to convert a asciinema recording to svg.
termtosvg
usespyte
under the hood, which iswhy I'm looking at pyte.In the recording, you can see that I'm trying to set the underline color with SGR code 58:
echo -e "(\x1b[4;58:2:255:165:0mdon't panic\x1b[m)"
. I'm not surprised pyte doesn't support this, Wikipedia says this about code 58: "Not in standard; implemented in Kitty, VTE, mintty, and iTerm2.". However, when I try to render this recording with termtosvg, I see more characters than I'd expect. These characters are coming from pyte.Eliminating asciinema and termtosvg from the equation, I wrote up a simple pyte script to test this out:
My pyte test script (demo.py)
This reproduces the issue. Note how the 58 is ignored, but parameters after the 58 get dumped to the screen:
For me, ideally pyte would ignore instruction 58 and its (colon-separated*) parameters. Is this something y'all would be open to? I've read through the relevant parsing code in streams.py, and I don't think it would be a huge lift to change the behavior. There's already some logic to call a
debug
function for unrecognized csi codes, it just doesn't absorb the whole code including its parameters.Or is this the wrong question to ask because I shouldn't expect to be able to record a session on any old terminal emulators (in my case, Alacritty) and be able to feed it into pyte? Like, if I was using pyte correctly, whatever software I'm running under pyte that wants to print colorful underlines should first do some sort of feature detection to see if the terminal can even do colorful underlines. (I have no idea if such feature detection is even possible).
*For total noobs like me, I spent a long time confused about the meanings of
:
vs;
here, until I found some very helpful discussions online:IIUC, it sounds like
:
is preferred for parameters because it allows more graceful degradation: if a terminal doesn't support/understand a particular SGR code, it can skip until the next;
. Otherwise, it would need to know how many parameters the SGR code takes as context in order to be able to skip past them (which it couldn't know because it's dealing with an unsupported SGR code!)The text was updated successfully, but these errors were encountered: