Skip to content

highlight: Also use the fallback variable if the main is empty#8793

Merged
faho merged 3 commits into
fish-shell:masterfrom
faho:fallback-for-empty-colors
Mar 20, 2022
Merged

highlight: Also use the fallback variable if the main is empty#8793
faho merged 3 commits into
fish-shell:masterfrom
faho:fallback-for-empty-colors

Conversation

@faho
Copy link
Copy Markdown
Member

@faho faho commented Mar 15, 2022

Currently, when a variable like $fish_color_command is set but empty:

set -g fish_color_command

what happens is that highlight parses it and ends up with a "normal"
color.

Change it so instead it sees that the variable is empty and goes
on to check the fallback variable, e.g. fish_color_normal.

That makes it easier to make themes that override variables.

This means that older themes that expect an empty variable to be
"normal" need to be updated to set it to "normal".

Following from this, we could make writing .theme files easier by no
longer requiring them to list all variables with specific values.
Either the theme reader could be updated to implicitly set known color
variables to empty, or the themes could feature empty values.

See #8787.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed - I don't know how to test this easily
  • User-visible changes noted in CHANGELOG.rst

Currently, when a variable like $fish_color_command is set but empty:

    set -g fish_color_command

what happens is that highlight parses it and ends up with a "normal"
color.

Change it so instead it sees that the variable is empty and goes
on to check the fallback variable, e.g. fish_color_normal.

That makes it easier to make themes that override variables.

This means that older themes that expect an empty variable to be
"normal" need to be updated to set it to "normal".

Following from this, we could make writing .theme files easier by no
longer requiring them to list all variables with specific values.
Either the theme reader could be updated to implicitly set known color
variables to empty, or the themes could feature empty values.

See fish-shell#8787.
@faho faho added this to the fish 3.5.0 milestone Mar 15, 2022
Comment thread doc_src/interactive.rst
========================================== =====================================================================

If a variable isn't set, fish usually tries ``$fish_color_normal``, except for:
If a variable isn't set or is empty, fish usually tries ``$fish_color_normal``, except for:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds like a good idea (assuming that not too many themes rely on the empty=normal behavior)

I'm not sure how it enables themes to override variables but I didn't think very hard..

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What I mean by that is that this allows making a theme with only global variables, and it will override any potentially set universal variables.

So you can start by setting all variables to empty and then set the ones you want, you don't have to erase variables (which for uvars would be permanent).

assuming that not too many themes rely on the empty=normal behavior

Of our themes only the None theme uses any empty values, and that sets the fallback colors to normal anyway.

I don't imagine this is used much elsewhere because I don't think there is much demand for the "normal" color. Most themes, in my experience, look like dracula.fish - a fixed palette of RGB colors.

It's also very simple to fix: If you notice your theme colors being not-normal where you expected normal, simply add a "normal" where you had an empty value. This is even backwards-compatible, and it is more like what you would get with set_color.

set_color $fish_color_option $fish_color_param won't be normal only because $fish_color_option isn't set.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds good, thanks. The new behavior is more intuitive

faho added 2 commits March 18, 2022 21:35
This allows keeping it to the fallback value, making us independent
from the theme that was set before.
@faho
Copy link
Copy Markdown
Member Author

faho commented Mar 19, 2022

I just pushed commits that make fish_config (in both versions) set all variables the theme doesn't mention to empty.

That should make themes more self-contained and make it harder to accidentally inherit color from the previous theme.

Specifically it'll be nicer when we add a color variable, because then it would be set to empty and the fallback would kick in even if the theme hasn't been updated.

@faho faho merged commit edbd3ba into fish-shell:master Mar 20, 2022
@faho faho deleted the fallback-for-empty-colors branch November 22, 2022 10:34
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants