CSI Sequences Not Parsed Properly

[jdugan6240]

What Operating System(s) are you seeing this problem on?

Linux X11

Which Wayland compositor or X11 Window manager(s) are you using?

Awesome WM (although this shouldn't matter)

WezTerm version

wezterm 20240226-174525-22424c32

Did you try the latest nightly build to see if the issue is better (or worse!) than your current version?

Yes, and I updated the version box above to show the version of the nightly that I tried

Describe the bug

It seems that RGB underline colors aren't parsed properly in a CSI sequence if foreground and background colors are also defined in the same CSI sequence. Interestingly, this works properly if a palette index is used instead for the underline color.

To Reproduce

Execute the following command: printf "\033[;4:3;38;2;255;0;0;48;2;0;255;0;58:2::0:0:255mtest\033[0m\n"

When this command is run, although the foreground and background of the phrase "test" will correctly be rendered as red and green, respectively, the undercurl will be the same color as the foreground. This is the case regardless of the underline style used.

Configuration

no config (this issue is config agnostic)

Expected Behavior

When the above printf statement is executed, the phrase "test" should be shown in red, with a green background, and a blue curly underline.

Logs

Debug Overlay wezterm version: 20240226-174525-22424c32 x86_64-unknown-linux-gnu Window Environment: X11 awesome Lua Version: Lua 5.4 OpenGL: AMD Radeon Vega 8 Graphics (radeonsi, raven, LLVM 16.0.6, DRM 3.57, 6.7.5-arch1-1) 4.6 (Compatibility Profile) Mesa 24.0.1-arch1.1 Enter lua statements or expressions and hit Enter. Press ESC or CTRL-D to exit 20:28:37.866 WARN window::os::x11::connection > Unable to resolve appearance using xdg-desktop-portal: get_appearance.read_setting: Reading xdg-portal org.freedesktop.appearance color-scheme: org.freedesktop.DBus.Error.ServiceUnknown: The name is not activatable

Anything else?

I'm fully aware that this is a free-time project, and you may have other priorities than fixing minor rendering issues such as this. If you'll be willing to point me in the general direction I should look (I believe I need to look at the Parser struct in termwiz::escape::parser, but beyond that I'm not sure), I can see about trying to fix this myself and submitting a PR, as if the issue is what I think it is, it should be fairly straightforward.

[jdugan6240]

Ok, after doing some more testing, I found something interesting.

If the foreground, background, and underline colors are the only attribute found in the CSI sequence, then everything works:

printf "\033[4:3m\033[48:2::0:255:0;58:2::0:0:255;38:2::255:0:0mtest\033[0m\n"

Wezterm also correctly parses many non-color CSI attributes in the same sequence:

printf "\033[4:3;3;1;9;6;53m\033[48:2::0:255:0;58:2::0:0:255;38:2::255:0:0mtest\033[0m\n"

However, if even one non-color CSI attribute lies before a set of color CSI attributes, every color attribute past the second is dropped:

printf "\033[4:3;48:2::0:255:0;58:2::0:0:255;38:2::255:0:0mtest\033[0m\n"

printf "\033[4:3;58:2::0:0:255;38:2::255:0:0;48:2::0:255:0mtest\033[0m\n"

printf "\033[4:3;38:2::255:0:0;48:2::0:255:0;58:2::0:0:255mtest\033[0m\n"

I'm not sure how the parser could be goofing up like this, but hopefully these test cases can help identify the problem.

[wez]

The ; form of some of those sequences is ambiguous and I'm not sure if it is possible to safely and sanely mix them together as in your first example. There's some commentary about this stuff in https://wezfurlong.org/wezterm/escape-sequences.html#csi-control-sequence-introducer-sequences just below the big table in that section:

There are a handful of additional SGR codes that allow setting extended colors; unlike the codes above, which are activated by a single numeric parameter out of SGR sequence, these the extended color codes require multiple parameters. The canonical representation of these sequences is to have the multiple parameters be separated by colons (:), but for compatibility reasons WezTerm also accepts an ambiguous semicolon (;) separated variation. The colon form is unambiguous and should be preferred; the semicolon form should not be used by new applications and is not documented here in the interest of avoiding accidental new implementations.

If you did want to take a crack at this, the termwiz escape sequence parser is where that logic lives. It is complex but hopefully not too difficult to follow.

Again, I'm not sure if it is possible to resolve 100%, because the ; separated form is inherently ambiguous wrt. other CSI codes that may be part of the same overall sequence.

[jdugan6240]

Sorry that I haven't had much chance to look into this - between moving and other life stuff, I've been quite busy.

I found the problem - in vtparse, MAX_PARAMS is fixed at 32, so any CSI sequence containing more parameters than that is simply cut off. This appears to be the case due to Default::default() seemingly supporting only 32-length arrays (on Linux at least).

Increasing this to 256 (what the Kitty terminal uses) and creating the array manually does fix the problem, but it also breaks the test_csi_too_many_params test, as that CSI sequence no longer has too many params. @wez is it OK to modify that test to reflect the new limit?