Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable setting the underline color #308

Closed
Nogesma opened this issue Jul 13, 2023 · 6 comments · Fixed by #310
Closed

Enable setting the underline color #308

Nogesma opened this issue Jul 13, 2023 · 6 comments · Fixed by #310
Labels
Effort: Good First Issue Good for newcomers without much Rust or Ratatui experiance. Apply only if willing to help implement Type: Enhancement New feature or request

Comments

@Nogesma
Copy link
Contributor

Nogesma commented Jul 13, 2023

Problem

There is a nonstandard ansi escape code that allows setting the underline color.

This is done with the code 58. it uses the same arguments as the 38 and 48 codes do for settings their colors.
The underline color can be reset with the code 59.

See kitty documentation for more info.

This is implemented in at least kitty, wezterm, alacritty, gnome terminal and konsole.

Solution

Allow this to be possible

Style::default().underline(Color::Blue).add_modifier(Modifier::UNDERLINE),

I can make a PR if needed.
@Nogesma Nogesma added the Type: Enhancement New feature or request label Jul 13, 2023
@orhun orhun added the Effort: Good First Issue Good for newcomers without much Rust or Ratatui experiance. Apply only if willing to help implement label Jul 13, 2023
@orhun
Copy link
Member

orhun commented Jul 13, 2023

Sounds like a nice feature, feel free to submit a PR! 🐻

@Nogesma
Copy link
Contributor Author

Nogesma commented Jul 13, 2023

I've noticed only crossterm supports setting the underline color, termion and termwiz do not.
Is putting it behind the crossterm feature flag the option you prefer?

@joshka
Copy link
Member

joshka commented Jul 13, 2023

The proposed function name will conflict with #289 which adds an underline() method to Style via the Stylize trait. Perhaps underline_color? Could there likely be other extended escape codes that might be added in the future?

It would be nice to generally keep feature parity between the backends, so perhaps submitting PRs to termion / termwiz for this feature would be a good in addition to feature flagging behind the crossterm flag here.

We recently made it possible to build Ratatui with all backend feature flags enabled together in order to make it easier to develop changes to backends without having to disable / enable features, so the implementation of this might end up being a no-op on the other backends?

@Nogesma Nogesma mentioned this issue Jul 14, 2023
Merged
@Nogesma
Copy link
Contributor Author

Nogesma commented Jul 14, 2023

I've renamed it it to underline_color.

I am not really familiar with any of the non-basic escape codes, so I don't really know if some other codes would need to be added in the future.

I can make a issue on the termion/termwiz repositories, but I am not really interested in making a PR.

For now the implementation is a no-op for the other backends.

I just submitted a PR (#310).

Nogesma added a commit to Nogesma/ratatui that referenced this issue Jul 14, 2023
@Nogesma
Add underline color style for crossterm backend
af916d6 
feat(style): Enable setting the underline color (ratatui#308)

This commit adds the underline_color() function to the Style and Cell
structs. This enables setting the underline color of text on the
crossterm backend. This is a no-op for the termion and termwiz backends
as they do not support this feature.
Nogesma added a commit to Nogesma/ratatui that referenced this issue Jul 14, 2023
@Nogesma
feat(style): Enable setting the underline color for crossterm (ratatu…
2f2e457 
…i#308)

This commit adds the underline_color() function to the Style and Cell
structs. This enables setting the underline color of text on the
crossterm backend. This is a no-op for the termion and termwiz backends
as they do not support this feature.
Nogesma added a commit to Nogesma/ratatui that referenced this issue Jul 14, 2023
@Nogesma
feat(style): Enable setting the underline color for crossterm (ratatu…
6450891 
…i#308)

This commit adds the underline_color() function to the Style and Cell
structs. This enables setting the underline color of text on the
crossterm backend. This is a no-op for the termion and termwiz backends
as they do not support this feature.
@mindoodoo
Copy link
Member

@Nogesma Yes if you could open up an issue on the Termion and Wezterm backends, it would be great as I would have to agree with @joshka that feature parity accross backends is quite important.

@Nogesma Nogesma mentioned this issue Jul 14, 2023
Open
@Nogesma
Copy link
Contributor Author

Nogesma commented Jul 14, 2023

I submited an issue for termwiz, but I don't have a gitlab account on redox-os' gitlab. I would prefer to not have to create one, if someone can submit an issue there?

github-merge-queue bot pushed a commit that referenced this issue Jul 15, 2023
@Nogesma
feat(style): Enable setting the underline color for crossterm (#308) (#…
b347201 
…310)

This commit adds the underline_color() function to the Style and Cell
structs. This enables setting the underline color of text on the
crossterm backend. This is a no-op for the termion and termwiz backends
as they do not support this feature.
a-kenji pushed a commit to a-kenji/ratatui that referenced this issue Jul 16, 2023
@Nogesma @a-kenji
feat(style): Enable setting the underline color for crossterm (ratatu…
9b13cef 
…i#308) (ratatui#310)

This commit adds the underline_color() function to the Style and Cell
structs. This enables setting the underline color of text on the
crossterm backend. This is a no-op for the termion and termwiz backends
as they do not support this feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Effort: Good First Issue Good for newcomers without much Rust or Ratatui experiance. Apply only if willing to help implement Type: Enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add underline color style for crossterm backend Nogesma/ratatui
4 participants
@joshka @mindoodoo @Nogesma @orhun