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

Expose "reset" color value in themes #8083

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Expose "reset" color value in themes #8083

merged 1 commit into from
Aug 29, 2023

Conversation

wildwestrom
Copy link
Contributor

@wildwestrom wildwestrom commented Aug 27, 2023

Seems like this was a pretty trivial fix, but please let me know if you spot anything obviously wrong here.

I tested it and it seemed like you could pretty easily modify reset to whatever you want and have it show up in your theme.

@wildwestrom
Copy link
Contributor Author

Since I can't put this in the title, this fixes #8075 to be clear.

@the-mikedavis the-mikedavis linked an issue Aug 28, 2023 that may be closed by this pull request
@the-mikedavis the-mikedavis changed the title Fix #8075 - Expose "reset" color value in themes Expose "reset" color value in themes Aug 28, 2023
@archseer archseer merged commit 82cd445 into helix-editor:master Aug 29, 2023
6 checks passed
@@ -70,6 +70,7 @@ over it and is merged into the default palette.

| Color Name |
| --- |
| `reset` |
Copy link
Contributor

@chtenb chtenb Aug 29, 2023

Choose a reason for hiding this comment

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

Perhaps the documentation could use a more extensive update.

The default palette uses the terminal's default 16 colors

That does no longer correspond to the list of identiefiers below it. I might send a PR at some point making some clarifications to this documentation chapter in general.

@chtenb
Copy link
Contributor

chtenb commented Aug 29, 2023

I had a look at how the Reset value is used in crossterm, and I noticed that it's used for default colors, and does not correspond to the reset code as I assumed.

https://github.com/crossterm-rs/crossterm/blob/08762b3ef4519e7f834453bf91e3fe36f4c63fe7/src/style/types/colored.rs#L59

As you see, it's translated to code 39 and 49 which correspond to the default fg and bg colors.

In this light, shouldn't we rather expose this as "default" instead of "reset"? The phrase "reset" could then be reserved for the actual reset code (0), which resets all modifiers as well as the colors (bg and fg) to their defaults.

@chtenb chtenb mentioned this pull request Aug 30, 2023
dgkf pushed a commit to dgkf/helix that referenced this pull request Jan 30, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
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.

Expose "reset" color value in themes
3 participants