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

themes: Extend snazzy #3971

Merged
merged 3 commits into from
Jan 20, 2023
Merged

Conversation

nrdxp
Copy link
Contributor

@nrdxp nrdxp commented Sep 26, 2022

In Helix, we are not restricted to just 8 colors like legacy terminal applications, and we also have a lot of different highlight categories, especially in languages with a lot of features, like Rust.

It was difficult to modify the original snazzy in a way that made the highlighting of all these various captures meaningful, so I decided to just extend the palette with colors that I felt went well with the main ones.

I've been using it already for several weeks, and just wanted to make sure I was happy with it before submitting a PR. I think I've got it to a place where it is worth sharing. If there are no major objections, we might even be able to just update snazzy.toml in place, but I thought adding a variant was less offensive to start.

Snazzy extended:
image

Snazzy:
image

@kirawi kirawi added A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer. labels Sep 26, 2022
Copy link
Contributor

@AlexanderBrevig AlexanderBrevig left a comment

Choose a reason for hiding this comment

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

cargo xtask themelint snazzy_extended with (#3983):

snazzy_extended: `ui.statusline.normal` and `ui.statusline.insert` cannot be equal
snazzy_extended: `ui.statusline.normal` and `ui.statusline.select` cannot be equal
snazzy_extended: missing `fg` or `bg` for `ui.virtual.indent-guide`
snazzy_extended: missing `fg` or `bg` for `markup.link.label`

Please add a screenshot to your description, maybe comparing it to snazzy?

@nrdxp
Copy link
Contributor Author

nrdxp commented Sep 29, 2022

@AlexanderBrevig I addressed the missing rules, but I didn't notice a difference in my build of helix, perhaps the branch is too old? Also, I added some screens.

@AlexanderBrevig
Copy link
Contributor

@AlexanderBrevig I addressed the missing rules, but I didn't notice a difference in my build of helix, perhaps the branch is too old? Also, I added some screens.

If you add this to your config :config-open then you should see both features. Color mode will change the color of NOR INS and SEL, and the indent guide will draw vertical bars at each indentation level.

[editor]
color-modes = true

[editor.indent-guides]
render = true

To see the markup.link.label open a .md file and have a link in it [example label](https://example.com)

@nrdxp
Copy link
Contributor Author

nrdxp commented Sep 29, 2022

Thanks for the info, I was able to test the changes now, and I force pushed some better choices. I think that should be everything 🤞

Only thing I'm wondering is, should we leave this an an extended theme, or just replace snazzy outright?

@the-mikedavis
Copy link
Member

should we leave this an an extended theme, or just replace snazzy outright?

Hmm good question. Since you originally added snazzy I suppose it's up to you. I like your reasoning and this looks like an improvement to me. But it does introduce some new warmer colors that make the palette less cold overall (which could be good or bad depending on how diverse of a palette you like).

@the-mikedavis the-mikedavis changed the title themes: add an extended version of snazzy themes: Extend snazzy Jan 20, 2023
@the-mikedavis the-mikedavis merged commit 68fc109 into helix-editor:master Jan 20, 2023
kirawi pushed a commit to kirawi/helix that referenced this pull request Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-theme Area: Theme and appearence related S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants