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

add penumbra+ theme #3398

Merged
merged 1 commit into from
Aug 21, 2022
Merged

add penumbra+ theme #3398

merged 1 commit into from
Aug 21, 2022

Conversation

vlmutolo
Copy link
Contributor

@vlmutolo vlmutolo commented Aug 12, 2022

From Penumbra's README:

Penumbra is a mathematically balanced colour scheme constructed in a perceptually uniform colour space with base colours inspired by the shades of colour occurring in nature due to the light of the sun and the sky. It cleanly separates the perceptual properties of colours while optimally utilizing the available colour space of typical displays.

This PR adds a new built-in theme to Helix using the Penumbra+ color scheme. The "+" is for "higher-contrast". Similar versions of this theme for the "balanced" (lower-contrast) and "++" (even higher contrast) variants of Penumbra can be created just be modifying the palette at the bottom of the theme according to the values in Penumbra's README.


This PR is currently in draft status because there are a few questions I wanted to get feedback on.

  • The biggest issue is whether punctuation should remain visually de-emphasized like comments are. This was my addition to the theme and wasn't suggested by Penumbra. Arguably a theme named "Penumbra" shouldn't make too many novel decisions on top of the original colors.
  • Should references be colored like the type or as a modifier on the type? Should they be visually distinct? In this version, they're distinct.
  • Is the yellow for text too distracting? Does it draw too much attention to text? We could try to shuffle around colors so that text is something less obtrusive.
  • I haven't put a ton of thought into languages other than Rust. I briefly considered HTML and Markdown. It would be great if someone could try out their own preferred languages and report back deficiencies.
  • Should this PR include the higher- and lower-contrast variants of Penumbra (i.e. "balanced" and "++")?

@vlmutolo vlmutolo changed the title add first draft of penumbra+ theme add penumbra+ theme Aug 12, 2022
@A-Walrus
Copy link
Contributor

As to point one I think punctuation should be more clearly visible.
One thing I dislike is how it looks with color modes, specifically the contrast between the indicator for visual or insert mode to the rest of the statusline.
I think the statusline itself should not be blue, rather only the indicator for normal mode should be blue, with the rest of the statusline some form of gray.

Other than that, looks good!

@vlmutolo
Copy link
Contributor Author

I think punctuation should be more clearly visible.

Yeah I'm enjoying this experiment with deemphasized punctuation, but for the built-in theme we should probably make the conservative choice and do text-colored punctuation.

I think the statusline itself should not be blue, rather only the indicator for normal mode should be blue, with the rest of the statusline some form of gray.

This is a good point. I like the blue status line for when color modes are disabled, but with them enabled it definitely makes the color modes harder to see. And for a built-in theme we probably want to err on the side of supporting more configurations.

@vlmutolo
Copy link
Contributor Author

vlmutolo commented Aug 12, 2022

Another issue I just noticed is that in this PR I accidentally used "white" for variables instead of "sky". I'll fix it later today.

@AceofSpades5757
Copy link
Contributor

This looks really good

@vlmutolo vlmutolo marked this pull request as ready for review August 20, 2022 20:26
@vlmutolo vlmutolo force-pushed the penumbra+ branch 2 times, most recently from 2b94abd to 93baa57 Compare August 20, 2022 20:44
@vlmutolo
Copy link
Contributor Author

vlmutolo commented Aug 20, 2022

I think this is ready for review. I reverted the punctuation to be the same color as variables (white) and made the status bar various shades of grey to avoid too much color when combined with the themed status line modes. I think this is the right choice for Helix's default version of this theme, though there's also a commented-out version for those who don't enable the colored modes.

The rest of the questions can probably go unanswered. It seems good-enough as is. If people want adjustments for other languages they can always file a PR.

@vlmutolo
Copy link
Contributor Author

Here are some random screenshots of Helix code using this theme.

2022-08-20_16-08-1661028463

2022-08-20_16-08-1661028479

2022-08-20_16-08-1661028526
.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Looks like scopes for ui.cursorline are missing. ui.cursorline.primary is for the primary selection and ui.cursorline.secondary for all secondaries. Many themes only highlight the primary one. They're not required and are disabled by default (use :set cursorline true to enable) but it's nice to add them to a shared theme like this for anyone who does want to use cursorline, similar to visible whitespace, rulers and indent-guides. Would you like to add some here?

@vlmutolo
Copy link
Contributor Author

it's nice to add them to a shared theme like this for anyone who does want to use cursorline, similar to visible whitespace, rulers and indent-guides. Would you like to add some here?

Yeah, I'd like to make this a "universal" theme as much as possible. I'll add them.

@vlmutolo
Copy link
Contributor Author

I just realized that there's no underlining for IDE warnings and errors, which I'd like to have. So I put this PR back into draft status briefly. I'll fix the underlining then mark it as ready for review.

@vlmutolo vlmutolo marked this pull request as ready for review August 20, 2022 22:36
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Looks nice, thank you! 🎨

@the-mikedavis the-mikedavis merged commit 59968d2 into helix-editor:master Aug 21, 2022
@the-mikedavis
Copy link
Member

Could you add a screenshot of this to the themes wiki? https://github.com/helix-editor/helix/wiki/Themes

@vlmutolo vlmutolo deleted the penumbra+ branch August 21, 2022 01:03
AlexanderBrevig pushed a commit to AlexanderBrevig/helix that referenced this pull request Aug 29, 2022
Co-authored-by: Vince Mutolo <vince@mutolo.org>
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
Co-authored-by: Vince Mutolo <vince@mutolo.org>
jdrst pushed a commit to jdrst/helix that referenced this pull request Sep 13, 2022
Co-authored-by: Vince Mutolo <vince@mutolo.org>
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.

None yet

4 participants