Skip to content

Making default color scheme compatible for windows terminal displays#222

Open
robinbernon wants to merge 5 commits into
console-rs:mainfrom
robinbernon:windows_display_fix
Open

Making default color scheme compatible for windows terminal displays#222
robinbernon wants to merge 5 commits into
console-rs:mainfrom
robinbernon:windows_display_fix

Conversation

@robinbernon

Copy link
Copy Markdown
Contributor

Fixing default color scheme to be compatible with windows terminal displays.

@robinbernon

Copy link
Copy Markdown
Contributor Author

@pksunkara thoughts?

@pksunkara

Copy link
Copy Markdown
Collaborator

Looks okay to me, but I am wondering which windows terminals doesnt support these chars?

@pksunkara

Copy link
Copy Markdown
Collaborator

I am thinking that this should not be gated on OS but on term application's unicode support.

@robinbernon

Copy link
Copy Markdown
Contributor Author

These Unicode characters are just not supported on any windows terminals right now. Perhaps a request to support them could be filed somewhere but for now I think the changes will have to be made here.

@robinbernon

robinbernon commented Oct 24, 2022

Copy link
Copy Markdown
Contributor Author

I am thinking that this should not be gated on OS but on term application's unicode support.

I'm guessing this is just a windows problem. Guessing all Unix based systems should be able to support the same list of Unicode chars? (Or at lease should all include the ones here?)

@pksunkara

Copy link
Copy Markdown
Collaborator

Please use https://docs.rs/console/latest/console/struct.TermFeatures.html#method.wants_emoji instead of gating on OS. And please make sure to test it manually on your terminal.

@robinbernon

Copy link
Copy Markdown
Contributor Author

Please use https://docs.rs/console/latest/console/struct.TermFeatures.html#method.wants_emoji instead of gating on OS. And please make sure to test it manually on your terminal.

@pksunkara I've done as asked. It's decided by .wants_emoji() now :)

Comment thread src/theme.rs
#[cfg(feature = "fuzzy-select")]
fuzzy_match_highlight_style: Style::new().for_stderr().bold(),
inline_selections: true,
if Term::stdout().features().wants_emoji() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can store this in a variable and use it in emoji lines instead of this big if condition block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry do you mind clarifying what you mean here? I dont understand. Do you want a variable for the term features?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let has_emoji = Term::stderr().features().wants_emoji();

// later
ColorfulTheme {
  error_prefix: style((if has_emoji { "✘" } else { "×" }).to_string()).for_stderr().red(),
}

@Gordon01

Gordon01 commented Sep 4, 2023

Copy link
Copy Markdown
Contributor

Hey, @robinbernon can you make the changes, please? It would be valuable for Windows users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants