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

lint: penumbra+ #3581

Closed

Conversation

AlexanderBrevig
Copy link
Contributor

passes linter from #3234
image

@AlexanderBrevig
Copy link
Contributor Author

@vlmutolo please sanity check my choices :)

@vlmutolo
Copy link
Contributor

vlmutolo commented Aug 28, 2022

Hi @AlexanderBrevig. What's the goal of this commit? Knowing that would help me to comment on the screenshot.

At first glance, judging only the screenshot you included, it seems like the command explanation box is pretty bright for a dark theme. It's also kind of low-contrast.

@AlexanderBrevig
Copy link
Contributor Author

This is the current look on master when using a bright theme in my terminal.

image

The goal is to set what's needed to make sure that the user experience is in line with what I presume you intended :)

@AlexanderBrevig
Copy link
Contributor Author

Moved to bulk PR #3587

@vlmutolo
Copy link
Contributor

Seems like the main issue here is that the background is the wrong color. I wonder if this is an issue with how Helix is rendering to the terminal.

@AlexanderBrevig
Copy link
Contributor Author

Seems like the main issue here is that the background is the wrong color. I wonder if this is an issue with how Helix is rendering to the terminal.

ui.background and ui.help were not defined correctly, the fix is shown in the diff for this PR (and included in the PR linked above)
When a theme does not set values, helix will default to the terminal for some things.
Which looks weird with some configurations. In this case, a bright theme in the terminal, since bg were not set and thus defaulting to terminal settings.
:)

@AlexanderBrevig
Copy link
Contributor Author

Sorry to be chatting in two places. I forwarded the use of sky, it should be shade. :) I'll fix my bug later today.

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.

2 participants