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

use better colour defaults #2257

Merged
merged 1 commit into from
Nov 12, 2022
Merged

use better colour defaults #2257

merged 1 commit into from
Nov 12, 2022

Conversation

jesseduffield
Copy link
Owner

@jesseduffield jesseduffield commented Nov 10, 2022

  • PR Description

We have no way of knowing whether a terminal is in a dark or light theme. HOWEVER we do have one trick up our sleeves: the FgDefault colour is going to be black in a light theme and white in a dark theme. So we need to make use of that colour when rendering text. Previously we allowed the user to configure a 'lightTheme' boolean value that would determine whether to use black or white. We're now removing that config value and just using FgDefault.

fixes #2255

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
    • no need
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@@ -24,9 +24,6 @@ var (
// DefaultTextColor is the default text color
DefaultTextColor = style.FgWhite

// DefaultHiTextColor is the default highlighted text color
DefaultHiTextColor = style.FgLightWhite
Copy link
Owner Author

Choose a reason for hiding this comment

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

This wasn't being used

@jesseduffield
Copy link
Owner Author

We would need to add a new config value for theme.defaultFgColor. Could you raise an issue for that @simon-abbott ?

@simon-abbott
Copy link

I realized this wasn't the best place for that comment so I migrated it to a discussion. I can definitely open a new issue though!

@simon-abbott
Copy link

Issue submitted: #2279.

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.

Use foreground instead of white color
2 participants