-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix indent guide styling #3324
Fix indent guide styling #3324
Conversation
Before the indent guides on top of whitespace inherited the theme from them. Now they do not.
Instead of moving let indent_guide_style = theme.try_get("ui.virtual.indent-guide").unwrap_or_else(|| theme.get("ui.virtual.whitespace)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
I love a fix that makes the code simpler :)
helix-term/src/ui/editor.rs
Outdated
indent_style, | ||
text_style.patch(indent_guide_style), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a good idea to patch ui.virtual.indent-guide
if it exists? I think it should only patch ui.virtual.whitespace
as a fallback rather than patching ui.virtual.indent-guide
if it exists. I am still undecided since users may put ui.virtual.indent-guide
without specifying a background.
Also, rather than doing this in a loop, I think it might be better if we patch it directly to indent_guide_style
rather than doing this in the inner function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I'm patching it with text_style
is that if users only specify the foreground, it should use the background from text_style
. If you look at line 457, all styles are patched from text_style
let style = spans
.iter()
.fold(text_style, |acc, span| acc.patch(theme.highlight(span.0)));
You're right about doing it outside of the loop, will fix.
* Fix incorrect indent guide styling Before the indent guides on top of whitespace inherited the theme from them. Now they do not. * Fix dark_plus theme indent_guides * Use whitespace style as fallback for indent-guide * Fix dark_plus theme indent_guides * Move indent_guide style patching out of loop
Resolves #3298.
The problem was that the indent guides would render on top of whitespace, and get their theme. The incorrect looking white indent guides were actually correct according to the theme.
The first part of the fix is to make the indent guides actually use the indent guide theme.
The second part is to make indent guides a sub theme of whitespace so that when not defined they will look like whitespace, which is the desired behavior. I updated all themes that set
ui.virtual.indent-guide
to useui.virtual.whitespace.indent-guide
, as well as updating the documentation.