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.

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

set_color --print-colors always bold #7805

Closed
plgruener opened this issue Mar 9, 2021 · 4 comments
Closed

set_color --print-colors always bold #7805

plgruener opened this issue Mar 9, 2021 · 4 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@plgruener
Copy link

plgruener commented Mar 9, 2021

fish --version : current git build (3.2.0-44-g4218c1f1a-dirty)
OS: ArchLinux, terminals: all

Problem: set_color --print-colors outputs every named color (except 'normal') in bold, even the non-br-ones. For colorschemes that define dedicated "bright" colors, this is inaccurate. It also means that the --dim switch does not work (it gets converted to bold+dim = bold font, but in normal color).

I patched builtin_set_colors.cpp to print the escape sequences to pipes/files, this yields:

$set_color --print-colors | string escape
\e\[1m\e\[30mblack
\e\[1m\e\[34mblue
\e\[1m\e\[90mbrblack
\e\[1m\e\[94mbrblue
\e\[1m\e\[96mbrcyan
\e\[1m\e\[92mbrgreen
\e\[1m\e\[95mbrmagenta
\e\[1m\e\[91mbrred
\e\[1m\e\[97mbrwhite
\e\[1m\e\[93mbryellow
\e\[1m\e\[36mcyan
\e\[1m\e\[32mgreen
\e\[1m\e\[35mmagenta
\e\[1m\e\[31mred
\e\[1m\e\[37mwhite
\e\[1m\e\[33myellow
\e\[1m\e\[30m\e\(B\e\[mnormal

and

$ set_color --print-colors --dim | string escape
\e\[2m\e\[1m\e\[30mblack
\e\[2m\e\[1m\e\[34mblue
\e\[2m\e\[1m\e\[90mbrblack
\e\[2m\e\[1m\e\[94mbrblue
\e\[2m\e\[1m\e\[96mbrcyan
\e\[2m\e\[1m\e\[92mbrgreen
\e\[2m\e\[1m\e\[95mbrmagenta
\e\[2m\e\[1m\e\[91mbrred
\e\[2m\e\[1m\e\[97mbrwhite
\e\[2m\e\[1m\e\[93mbryellow
\e\[2m\e\[1m\e\[36mcyan
\e\[2m\e\[1m\e\[32mgreen
\e\[2m\e\[1m\e\[35mmagenta
\e\[2m\e\[1m\e\[31mred
\e\[2m\e\[1m\e\[37mwhite
\e\[2m\e\[1m\e\[33myellow
\e\[2m\e\[1m\e\[30m\e\(B\e\[mnormal

(edit: changed od -c to string escape)

This confirms it's not an issue with my terminal config, but fish sending wrong escape codes.

(Note: I had a quick glance at the code, but couldn't fix the issue myself. But I saw a comment in output.cpp that bold mode is always entered for readability reasons, that might be related?)

@faho
Copy link
Member

faho commented Mar 9, 2021

Ah, okay.

What happens here is that fish has a special case where it'll enable bold mode whenever the background is on "to make reading easier". That's pretty dubious to begin with, but removing it is probably a bigger change.

set_color runs into this because it sets a background color of "none". Fix incoming.

@faho faho added the bug Something that's not working as intended label Mar 9, 2021
@faho faho added this to the fish 3.2.1 milestone Mar 9, 2021
@faho
Copy link
Member

faho commented Mar 9, 2021

Also as a sidenote:

$ set_color --print-colors | od -c

od is super chatty and fills the entire screen with very little information here. Use string escape in its place and you'll get much more compact output. For example:

> set_color yellow | string escape
\e\[33m

@faho faho closed this as completed in 4762d52 Mar 9, 2021
@plgruener
Copy link
Author

Thanks for the quick fix.

Ah, string escape is certainly much nicer than od, thanks for the tip. Is there any way to display the escape sequences without patching fish first? (As far as I could tell they are always omitted if output is to a pipe or file.)

@faho
Copy link
Member

faho commented Mar 9, 2021

Is there any way to display the escape sequences without patching fish first?

set_color --print-colors specifically won't do it if it's not connected to a terminal.

I suppose you could use expect or similar to connect it to a fake one (e.g. like our tests do). Other than that, no.

faho added a commit that referenced this issue Mar 9, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2021
@floam floam reopened this Nov 1, 2021
@floam floam closed this as completed Nov 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

3 participants