Skip to content

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

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.

Sign up for GitHub

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

Merged
faho merged 3 commits into from
Mar 20, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fa69a0
highlight: Also use the fallback variable if the main is empty
faho Mar 15, 2022
843962c
fish_config CLI: Use color fallbacks
faho Mar 18, 2022
5ed2438
fish_config: Set colors that aren't set in the theme to empty
faho Mar 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc_src/interactive.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ Variable Meaning
``fish_color_search_match`` history search matches and selected pager items (background only)
========================================== =====================================================================

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

@krobelus krobelus Mar 15, 2022

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

@faho faho Mar 16, 2022

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

@krobelus krobelus Mar 19, 2022

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


- ``$fish_color_keyword``, where it tries ``$fish_color_command`` first.
- ``$fish_color_option``, where it tries ``$fish_color_param`` first.
Expand Down Expand Up @@ -178,7 +178,7 @@ Variable Meaning
``fish_pager_color_secondary_description`` description of every second unselected completion
========================================== ===========================================================

When the secondary or selected variables aren't set, the normal variables are used, except for ``$fish_pager_color_selected_background``, where the background of ``$fish_color_search_match`` is tried first.
When the secondary or selected variables aren't set or are empty, the normal variables are used, except for ``$fish_pager_color_selected_background``, where the background of ``$fish_color_search_match`` is tried first.

.. _abbreviations:

Expand Down
44 changes: 30 additions & 14 deletions share/functions/fish_config.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,27 +149,27 @@ function fish_config --description "Launch fish's web based configuration"
string replace -r '.*/([^/]*).theme$' '$1' $dir/*.theme
return
case demo
echo -ns (set_color $fish_color_command) /bright/vixens
echo -ns (set_color $fish_color_command || set_color $fish_color_normal) /bright/vixens
echo -ns (set_color normal) ' '
echo -ns (set_color $fish_color_param) jump
echo -ns (set_color $fish_color_param || set_color $fish_color_normal) jump
echo -ns (set_color normal) ' '
echo -ns (set_color $fish_color_redirection) '|'
echo -ns (set_color $fish_color_redirection || set_color $fish_color_normal) '|'
echo -ns (set_color normal) ' '
echo -ns (set_color $fish_color_quote) '"fowl"'
echo -ns (set_color $fish_color_quote || set_color $fish_color_normal) '"fowl"'
echo -ns (set_color normal) ' '
echo -ns (set_color $fish_color_redirection) '> quack'
echo -ns (set_color $fish_color_redirection || set_color $fish_color_normal) '> quack'
echo -ns (set_color normal) ' '
echo -ns (set_color $fish_color_end) '&'
echo -ns (set_color $fish_color_end || set_color $fish_color_normal) '&'
set_color normal
echo -s (set_color $fish_color_comment) ' # This is a comment'
echo -s (set_color $fish_color_comment || set_color $fish_color_normal) ' # This is a comment'
set_color normal
echo -ns (set_color $fish_color_command) echo
echo -ns (set_color $fish_color_command || set_color $fish_color_normal) echo
echo -ns (set_color normal) ' '
echo -s (set_color $fish_color_error) "'" (set_color $fish_color_quote) "Errors are the portal to discovery"
echo -s (set_color $fish_color_error || set_color $fish_color_normal) "'" (set_color $fish_color_quote || set_color $fish_color_normal) "Errors are the portal to discovery"
set_color normal
echo -ns (set_color $fish_color_command) Th
echo -ns (set_color $fish_color_command || set_color $fish_color_normal) Th
set_color normal
set_color $fish_color_autosuggestion
set_color $fish_color_autosuggestion || set_color $fish_color_normal
echo is is an autosuggestion
echo
case show
Expand Down Expand Up @@ -236,7 +236,15 @@ function fish_config --description "Launch fish's web based configuration"
return 1
end

set -l have_color 0
set -l known_colors fish_color_{normal,command,keyword,quote,redirection,\
end,error,param,option,comment,selection,operator,escape,autosuggestion,\
cwd,user,host,host_remote,cancel,search_match} \
fish_pager_color_{progress,background,prefix,completion,description,\
selected_background,selected_prefix,selected_completion,selected_description,\
secondary_background,secondary_prefix,secondary_completion,secondary_description}


set -l have_colors
while read -lat toks
# We only allow color variables.
# Not the specific list, but something named *like* a color variable.
Expand All @@ -251,11 +259,19 @@ function fish_config --description "Launch fish's web based configuration"
set -eg $toks[1]
end
set $scope $toks
set have_color 1
set -a have_colors $toks[1]
end <$file

# Set all colors that aren't mentioned to empty
for c in $known_colors
contains -- $c $have_colors
and continue

set $scope $c
end

# Return true if we changed at least one color
test $have_color -eq 1
set -q have_colors[1]
return
case dump
# Write the current theme in .theme format, to stdout.
Expand Down
49 changes: 49 additions & 0 deletions share/tools/web_config/webconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1457,13 +1457,62 @@ def do_POST(self):

if p == "/set_color/":
print("# Colorscheme: " + postvars.get("theme"))
have_colors = set()
known_colors = set(
(
"fish_color_normal",
"fish_color_command",
"fish_color_keyword",
"fish_color_quote",
"fish_color_redirection",
"fish_color_end",
"fish_color_error",
"fish_color_param",
"fish_color_option",
"fish_color_comment",
"fish_color_selection",
"fish_color_operator",
"fish_color_escape",
"fish_color_autosuggestion",
"fish_color_cwd",
"fish_color_user",
"fish_color_host",
"fish_color_host_remote",
"fish_color_cancel",
"fish_color_search_match",
"fish_pager_color_progress",
"fish_pager_color_background",
"fish_pager_color_prefix",
"fish_pager_color_completion",
"fish_pager_color_description",
"fish_pager_color_selected_background",
"fish_pager_color_selected_prefix",
"fish_pager_color_selected_completion",
"fish_pager_color_selected_description",
"fish_pager_color_secondary_background",
"fish_pager_color_secondary_prefix",
"fish_pager_color_secondary_completion",
"fish_pager_color_secondary_description",
)
)
for item in postvars.get("colors"):
what = item.get("what")
color = item.get("color")

if what:
if not what.startswith("fish_pager_color_") and not what.startswith(
"fish_color_"
):
have_colors.add("fish_color_" + what)
else:
have_colors.add(what)
output = self.do_set_color_for_variable(what, color)

# Set all known colors that weren't defined in this theme
# to empty, to avoid keeping around coloration from an earlier theme.
for what in known_colors - have_colors:
output += "\n" + self.do_set_color_for_variable(what, "")

elif p == "/get_function/":
what = postvars.get("what")
output = [self.do_get_function(what[0])]
Expand Down
4 changes: 2 additions & 2 deletions src/highlight.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,8 @@ rgb_color_t highlight_color_resolver_t::resolve_spec_uncached(const highlight_sp
highlight_role_t role = is_background ? highlight.background : highlight.foreground;

auto var = vars.get(get_highlight_var_name(role));
if (!var) var = vars.get(get_highlight_var_name(get_fallback(role)));
if (!var) var = vars.get(get_highlight_var_name(highlight_role_t::normal));
if (var.missing_or_empty()) var = vars.get(get_highlight_var_name(get_fallback(role)));
if (var.missing_or_empty()) var = vars.get(get_highlight_var_name(highlight_role_t::normal));
if (var) result = parse_color(*var, is_background);

// Handle modifiers.
Expand Down