zellij
zellij copied to clipboard
zellij-org
feat(styling): Add styled underline support
This is my first contribution to an open-source Rust project, so please don't spare any criticism. I'd love to know how I can improve.
This PR adds the ability to display Styled underlines as supported in Kitty, Alacritty, and iTerm2. This was done by editing src/panes/terminal_character.rs to use a new enum for underlines capable of describing these nuances, and adding the underline_color property.
The documentation I used to implement this feature is here: https://sw.kovidgoyal.net/kitty/underlines/
This is my first time contributing to this repo, so please let me know if there's anything I'm missing or need to do differently.
Update: I originally had normal underlines and styled underlines in their own enums (I was thinking in terms of terminal compatibility, since most terminals support underlines, but only a few support stylized underlines, so I kept them separate), but I realized there actually wasn't a good reason for that, because I could condense it all into one with no affect on the output, with added code readability.
My branch was also apparently 33 commits behind, so I brought it up to date as well.
Hey @sploders101 - I'm happy to see this coming along, and thanks for tolerating all the merges from main! I'm going to ask you to merge again so that you can continue working (it should not have conflicts, just some updated plugin assets).
About the change itself, I haven't looked at it deeply, but for me in xterm this happens:
Same thing looks good in Alacritty, so maybe it's a support thing?
Let me know if you want help reproducing this.
@imsnif Sure thing! I'll have that out shortly. It is a support thing; styled underlines are only supported in certain terminals, but the hope is that the application won't output styled underlines unless the terminal advertised in $TERM supports them. Ideally though, even if the application does output them when the terminal doesn't support it, the terminal should ignore it. How are you getting that output in xterm? I'm happy to do some digging to try and figure it out.
Hum - if these ANSI codes mean one thing in one terminal and another thing in another, it might be a problem to add them to Zellij at this time. Zellij unfortunately doesn't distinguish well between client terminals, and copies the termios data (and all related things that come with it) of the terminal that opened it.
This is not desired and I'd very much like to change this, but it's not a small task and a lot of care needs to be placed into it.
I'm not 100% certain this is the case here though. Describing my reproduction is not trivial, since it's part of a mash of vim themes, configuration and plugins - but for yourself, if you open the same underline that worked for you in xterm, do you not reproduce this?
I'll try and see if I can reproduce it later tonight. Unfortunately, I was more concerned with how you got an application like vim to output the curly underlines when the terminal doesn't support it. If you open vim in xterm directly does it not do the same thing?
I don't have a bigger picture on the architecture at the moment, but just an idea; what if I could detect support in the client and convert the curly underlines to normal ones when it's not supported? Would you consider that an acceptable solution? I definitely don't want to break any existing installs, so if there's anything I could do to make this better, let me know.
Good question about xterm - I just made sure and it works properly without Zellij.
What you suggest about the client is a good idea in theory, but unfortunately we perform all of our conditional rendering on the server side. The client just gets the serialized output and removing the ANSI codes from there would not be a good solution.
Being able to detect different clients is a very large project, regardless of how we go about it. It touches a lot of different parts of the code-base. As we can see though - this is not the case here, since with just one client this works without Zellij and doesn't work with Zellij. My guess would be that this is not a matter of capability detection but rather of us doing something wrong with the ANSI codes.
Well, if it's a problem with ANSI codes, I should be able to fix it. It might have something to do with the fact that kitty-style solid underlines get normalized to their more widely-supported counterpart.
If it's not too much trouble, It would also be helpful to get an asciinema recording between zellij and vim so I can see exactly what vim is outputting.
I may or may not get to it today though, but definitely this weekend. If vim works properly outside zellij, it should still work with only one client without major refactoring.
I'm not sure if this helps or not, but curly underlines with colors can be enabled in tmux with:
set -as terminal-overrides ',*:Smulx=\E[4::%p1%dm'
set -as terminal-overrides ',*:Setulc=\E[58::2::%p1%{65536}%/%d::%p1%{256}%/%{255}%&%d::%p1%{255}%&%d%;m'
This works in both Alacritty and Kitty. I'm not sure if its restricted to specific terminal emulators or not but in doing research for those commands, nothing was mentioned about specific terminals. Only that the terminal supports curly underlines.
@imsnif Sorry for the late response. I haven't really had time to investigate this further until now. I finally got around to bringing Asahi Linux up to date and getting xterm installed, but I can't replicate the issue. Styled solid underlines get converted to normal ones, other forms don't display at all, and I'm able to reset it perfectly. I'm not super familiar with xterm, but mine doesn't look like that either. Is there any more information you can provide that may help me replicate this?
Hey @sploders101 - sorry for taking some time to get to this. Here's a minimal reproduction:
echo -e "\033[58;2;224;175;104mI should not be blue"
I can reproduce using this both in and outside of Zellij (with this patch ofc, otherwise we just drop the 58 SGR sequence).
This happens to me through some combination of my vim linting plugins which will be a bit tiresome to replicate :) (but I think won't help us anyway, since we'll just end up with the same ANSI sequence from above). This does not happen without Zellij in xterm.
My guess is that we're doing some over-eager evaluation of parameters somehow in this sequence (evaluating too little or too many parameters?)
sorry for taking some time to get to this.
No problem! I hope you had a happy holidays!
I was actually able to reproduce this in the Mac OS terminal, too, and I think I know why. I'm outputting semicolons, which in many cases are command delimiters. If I change the semicolons in your example to regular colons, this does not occur. My guess is that when the terminal is parsing the sequence, even though semicolons are valid argument separators, the terminal is ignoring the 58 because it doesn't know what it is, and parsing the other values as separate commands, yielding unexpected results. Colons, on the other hand, can only differentiate arguments, causing the terminal to throw out the entire sequence when it doesn't understand the 58.
I have changed the semicolons to colons, and this does not appear for me anymore. Can you verify this is fixed on your end?
Another suggestion for consistency and correctness would be to change the foreground and background color outputs to match this. This can also happen in terminals that do not support truecolor. I modified your minimal reproduction slightly to demonstrate this:
echo -e "\x1b[38;2;224;175;104mTesting"
vs
echo -e "\x1b[38:2:224:175:104mTesting"
Let me know if you would like me to make this change and I can add it in.
Hey @sploders101 - that's a really cool solution with the colons. However, I think there are still some chinks in the implementation. This is what I get in xterm with my previous now:
Not blue, but the text is black for some reason (on my black terminal background it is invisible).
I'm happy to continue troubleshooting this back and forth, but I'll admit there's a small voice in the back of my mind which tells me all these Kitty features are designed to work for Kitty and other similar setups (eg. a desktop terminal emulator) and not a lot of thought has been given in them for different setups (terminal multiplexers) - which I think is intentional on the part of the Kitty maintainer who is very hostile to the latter.
Again, happy to keep iterating if you like.
Hey @sploders101 - what's the status here? (no pressure, just want to make sure I'm not blocking this through some misunderstanding)
@imsnif Hey, sorry I didn't respond earlier. I tried to reproduce it but have time to type up a response and completely forgot about it. I wasn't able to reproduce the issue you described. Is there any chance you could give it another try on a fresh build and maybe do an asciinema recording so I can see exactly what zellij is outputting? It might help me track down what's going on.
@sploders101 - if you echo the line I mentioned above in the screenshot, do you see everything properly? Do you not get the behavior I mentioned in my screenshot?
Forgive me, asciinema (as well as rechecking this branch and getting back into context just for this reproduction) is a lot of overhead and I'm trying to move this forward. If you're not able to reproduce, I'll try to find the time for this.
I wasn't able to when I tried it before, but it's been a while so I don't remember exactly what I tried. I'll give it another shot this evening and come back with my results.
@imsnif what terminal emulator do you use? I do git clone to d65e8220b67ed7324bc4d79f265f6519c516e198 and applied this PR and in alacritty, foot and wezterm I see this like this:
I'm using xterm
Friends - I have an incredible number of things on my plate right now that need to be done before the next release. I'm sorry, but I cannot devote a lot of time to this issue. If I'm the one blocking it - I sincerely apologize, but it will have to wait.
Don't worry about it. I only checked this, because you wrote before about your lack of time, so I was curious about this.
@sploders101 I use Arch with Sway in Wayland only mode, so I couldn't check this on my main system, but I created virtual machine with Debian Bookworm and in xterm there is like imsnif said. I don't know rust so I can help with it, but if you want me to check something I can do this.
I recorded this in asciinema, but on asciinema site player doesn't show the problem like it did in xterm originally https://asciinema.org/a/T4WGMyeQzWtjWdIfFz3fEmO9m
That's strange, but this is only accurate to xterm on Debian and Ubuntu. I installed few distributions and for this moment:
- Ubuntu 22.04 LTS with xterm 372 have this error
- Ubuntu 22.10 with xterm 372 have this error
- Debian Bookworm with xterm 378 have this error
- Fedora 37 with xterm 378 doesn't have this error
- Arch Linux with xterm 379 doesn't have this error
On all distributions, qterminal highlighted this text echo -e "\033[58;2;224;175;104mI should not be blue" on blue in zellij.
Xfce4-terminal, st and gnome-terminal worked identical like wezterm, foot and alacritty on my previously picture (without error).
As xterm on Debian Bookworm has a problem but not on Fedora, although it's the same version, and Ubuntu seems to be catching Debian packages for the most part, the problem seems to be in Debian. Perhaps some terminfo value for xterm is wrong in Debian (and by extension Ubuntu)? My knowledge and possibilities end here. If there's anything I can do, let me know and I'll try to do it. I'm going to keep these virtual machines for a while.
Just to chime in, a guess would be that these terminals do not support the colon : notation? Maybe we need a different approach?
Hey @sploders101 - still interested in working on this? Just to provide transparency since a lot of people want this feature. Might be good to give someone else a chance to implement it if not.
@imsnif I'm so sorry for getting back to you so late on this. Unfortunately, I don't have a whole lot of time to work on this anymore since the issue is quite difficult for me to replicate. If anyone else wants to pick it up, I'd be happy to yield to them on it. Hopefully my branch can help serve as a guide for the implementation process. Please let me know if there is anything I should do to help facilitate this.
I would like to get this merged in, since it seems quite a few people want this functionality. I'm willing to put some more effort into it, but I'm not sure how to go about solving the problem. Unfortunately, it's a very difficult problem to solve since it involves programs outputting terminal sequences that aren't supported by the terminal they're running in. I opted to use the colon syntax because it seemed to be a logical grouping of the command to its arguments. However, it seems certain versions of xterm did not take this approach. I will be picking this back up though, because I think a good short-term solution may be to just log which syntax was used to set the style, and replicate that. I'll try it out sometime in the next few days and post back what I find.
Hi, I think it's safe to say that the status right now is: if anyone is able to put up a working solution that does not break anything else - I'd be happy to review it and merge it.
I am unfortunately not able to devote a lot of time to this issue myself as I'm concentrating on issues in our Roadmap.
Hi, I think it's safe to say that the status right now is: if anyone is able to put up a working solution that does not break anything else - I'd be happy to review it and merge it.
I am unfortunately not able to devote a lot of time to this issue myself as I'm concentrating on issues in our Roadmap.
What would it take for this feature to be included in the roadmap?
Respectfully, it's not going to be included in the roadmap. This one's up to the community.