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

Default theme broken on default config of Terminal.app (OS X) #328

Closed
axelson opened this issue Oct 1, 2018 · 6 comments · Fixed by #350
Closed

Default theme broken on default config of Terminal.app (OS X) #328

axelson opened this issue Oct 1, 2018 · 6 comments · Fixed by #350
Labels

Comments

@axelson
Copy link

axelson commented Oct 1, 2018

Not sure if this is something that you're planning to fix but bat is broken by default on OS X. Not sure what the best fix would be, but my first thought is for the default theme (not sure which one it is since bat --list-themes does not tell you the default) should work reasonably with both a black and a white background (although maybe that's too difficult). As far as I can tell the default theme emits some text that is completely white, which makes the default config of bat unusable on a white background.

@sharkdp
Copy link
Owner

sharkdp commented Oct 1, 2018

Thank you for your feedback.

This is unfortunate, but there is no reasonable way to check whether the user is on a terminal with a dark or a light background color.

I don't think there's a color scheme that would work equally well on dark and light backgrounds, so I don't think there is a way to "fix" this.

As far as I can tell the default theme emits some text that is completely white, which makes the default config of bat unusable on a white background.

Yes. We want bat's color schemes to look as intended, so we need to colorize the "normal" text as well because we can't tell what the users default font color is.

What do you think? Maybe we should mention this in the README somewhere?

@sharkdp sharkdp added the question Further information is requested label Oct 1, 2018
@axelson
Copy link
Author

axelson commented Oct 1, 2018

Yeah, I think that mentioning in the README would be great. I think the key points are:

  1. By default bat's output looks good on a black background
  2. For a light background you should set the theme to GitHub (or some other recommended theme), or choose a different theme from bat --list-themes
  3. If you often switch between light and dark backgrounds, then you should set the BAT_THEME environment variable to an appropriate theme at the same time that you switch between the light and dark background
  4. Add a pointer to a collection of custom themes (or how to create your own).

I do think that it's important to recommend a specific theme in number 2 just to reduce friction

@sharkdp sharkdp added good first issue Good for newcomers documentation and removed question Further information is requested labels Oct 1, 2018
russtaylor added a commit to russtaylor/bat that referenced this issue Oct 11, 2018
@MatthieuBizien
Copy link

I run into this "bug" and thought that it was an issue with bat, then I tried in another computer and understood my mistake... 😑

I just tested with Vim, and it is able to change the color of the font. A text reset for the "white" code could be enough. The most complete fix would be to use xterm control sequence to change the the according to the background, but it may be overkill.

A quick fix would be to use a very light gray instead of white, so the user could at least understand what is the issue.

Very nice BTW 👍

@sharkdp
Copy link
Owner

sharkdp commented Nov 18, 2018

I just tested with Vim, and it is able to change the color of the font. A text reset for the "white" code could be enough.

What is "a reset for the white code"?

The most complete fix would be to use xterm control sequence to change the the according to the background

What does that mean?

@MatthieuBizien
Copy link

The reset code is '\033[0m'. It reset all the styles, including the text color. [1] The text will be in white on a black background, and in black on a white background. It should be put at the end of the styled section so it doesn't impact what is after. Ansi_term seems to do the reset at the end of paint(), so println or Style::default().paint should be enough.

For xterm control sequences I was thinking of [2], but it doesn't work on Mac terminal and others. I looked at Vim [3], and they check the name of some terminals and the env variable $COLORFGBG, which is not defined by many terminals (incl. Mac and iterm2). This combo terminal detection/$COLORFGBG is used by others [4], but it's fragile (su - doesn't propagate the env variables, non-default color on terminal without COLORFGBG...). I didn't imagine something as simple as detecting the background color could be so complicated!

[1] https://stackoverflow.com/questions/5947742/how-to-change-the-output-color-of-echo-in-linux/28938235#28938235
[2] https://stackoverflow.com/questions/2507337/is-there-a-way-to-determine-a-terminals-background-color
[3] https://github.com/vim/vim/blob/ded5f1bed7ff2d138b3ee0f9610d17290b62692d/src/option.c#L4033
[4] bhauman/rebel-readline@c351c88

@sharkdp
Copy link
Owner

sharkdp commented Nov 21, 2018

The reset code is '\033[0m'. It reset all the styles, including the text color. [1] The text will be in white on a black background, and in black on a white background.

Oh, bat does use reset sequences, of course.

The problem is that even the "normal text" is painted with a particular color (white or very bright) because that is the way the color scheme is meant to be used. I wouldn't want to change white text to "no color" because some people use green fonts on a dark background and this would spoil their bat experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants