Skip to content

Add Check for PASTEL_COLOR_MODE if COLORTERM is empty or NOT in truecolormode #140

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
Closed
wants to merge 1 commit into from
Closed

Add Check for PASTEL_COLOR_MODE if COLORTERM is empty or NOT in truecolormode #140

wants to merge 1 commit into from

Conversation

yugonline
Copy link

@yugonline yugonline commented Jun 22, 2021 •
edited
Loading

#121 @sharkdp

2021-06-21_20-52-01

Hopefully this should solve the issue. What do you think? It is a bit hacky , but... should work for all cases where COLORMODE is absent.

EDIT: attached the wrong screenshot.

@sharkdp
Copy link
Owner

sharkdp commented Jul 4, 2021

Thank you for your contribution!

I don't think that's quite the behavior we want. PASTEL_COLOR_MODE, if set, should always overwrite COLORTERM values (both can be overwritten by the --color-mode flag). Please also take a look at this section:

pastel/src/cli/main.rs

Lines 44 to 89 in 17c5ce0

match global_matches
.value_of("color-mode")
.expect("required argument")
{
"24bit" => Some(ansi::Mode::TrueColor),
"8bit" => Some(ansi::Mode::Ansi8Bit),
"off" => None,
"auto" => {
if interactive_mode {
let env_color_mode = std::env::var("PASTEL_COLOR_MODE").ok();
match env_color_mode.as_deref() {
Some("8bit") => Some(ansi::Mode::Ansi8Bit),
Some("24bit") => Some(ansi::Mode::TrueColor),
Some("off") => None,
Some(value) => {
return Err(PastelError::UnknownColorMode(value.into()));
}
None => {
let mode = ansi::get_colormode();
if mode == ansi::Mode::Ansi8Bit
&& global_matches.subcommand_name() != Some("paint")
&& global_matches.subcommand_name() != Some("colorcheck")
{
write_stderr(Color::yellow(), "pastel warning",
"Your terminal emulator does not appear to support 24-bit colors \
(this means that the COLORTERM environment variable is not set to \
'truecolor' or '24bit'). \
pastel will fall back to 8-bit colors, but you will only be able \
to see rough approximations of the real colors.\n\n\
To fix this, follow these steps:\n \
1. Run 'pastel colorcheck' to test if your terminal\n \
emulator does support 24-bit colors. If this is the\n \
case, set 'PASTEL_COLOR_MODE=24bit' to force 24-bit\n \
mode and to remove this warning. Alternatively, make\n \
sure that COLORTERM is properly set by your terminal\n \
emulator.\n \
2. If your terminal emulator does not support 24-bit\n \
colors, set 'PASTEL_COLOR_MODE=8bit' to remove this\n \
warning or try a different terminal emulator.\n\n\
\
For more information, see https://gist.github.com/XVilka/8346728\n");
}
Some(mode)
}
}

which does the right thing for the normal output (just not for STDERR, which is used in pastel pick). And let me know if you need further help.

@sharkdp
Copy link
Owner

sharkdp commented Jul 4, 2021

build error is unrelated to your branch. I will fix this in #141

@yugonline
Copy link
Author

Okay I think I got a bit more understanding of requirements now. Initially I was thinking one of them has to be set. I will take a look at the file you mentioned and try to rewire it according to that. I was a bit confused because this code snippet you linked also uses the same function (as fallback) which the STDERR uses. So I thought just adding another check for PASTEL env would be enough. I think I was totally on the wrong path with this completely. Please give me some more time to fix this and also read a bit more thoroughly the code in general.

@sharkdp sharkdp mentioned this pull request Jul 11, 2021
Closed
@sharkdp
Copy link
Owner

sharkdp commented May 26, 2022

I'm closing this due to inactivity. Please feel free to comment in case it should be re-opened.

@sharkdp sharkdp closed this May 26, 2022
@Divoolej Divoolej mentioned this pull request Jul 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yugonline @sharkdp