Fix fish_config theme save

[mqudsi]

The documentation states that running fish_config theme save after fish_config theme choose [theme_name] will result in "saving" the currently chosen theme, but this does not match the actual behavior of fish_config theme save which expects a trailing argument specifying the name of the theme to select/persist.

Given that the documented way has been included in a release and that it makes more sense than calling fish_config theme save xxx when you are loading from xxx and not saving to xxx, this patch revises fish_config.fish to support the documented behavior.

When fish_config theme choose xxx is used, xxx is saved to a hidden global variable __fish_last_chosen_theme. If fish_config theme save is called without a theme name specified, then this global variable is searched for the name of the last chosen theme and that is saved/persisted as the default theme.

Closes #9088.

---

I'm requesting a review because I have no idea how fish_config is meant to be used and I'm going solely off what I read in the function -- I've never used it myself. I'm just conforming the code to the existing docs.

[faho]

This isn't the right way to go about this.

The current theme is just the set of the current color variables, so running fish_config theme save should just persist the current variables (to universal variables)

[faho]

Basically, what you need to do is just not read any file if $argv is empty, and instead only do the saving part.

So an if set -q argv[1] on line 226 .

[faho]

In essence, fish_config theme save or fish_config prompt save is the "I'm happy with this" button. It just saves the current config.

So if you do fish_config theme choose coolbeans, and you decide that it's nice, and then you do fish_config theme save it should save the coolbeans theme, but if you decide to change a color or two and then fish_config theme save it should save the theme with those alterations.

[mqudsi]

That makes sense, thanks.

[mqudsi]

I pushed an update that addresses your concerns, @faho. (I didn't force push, but I'll squash the commits into the latter if we end up going with that direction).

Note that the latest commit includes an important caveat/note:

This does not catch color variables unknown to fish! If a theme and a
prompt agree on some variable to hold some color but it's not a color variable
known to fish, it won't be persisted!

I think we have to search for all variables that match the whitelist filter we use when loading a theme from disk and persist all of those, not just the known colors?

Basically, replace for color in $known_colors with for color in $known_colors (set | string match -r '^fish_(?:pager_)?color.*$')

[faho]

Basically, replace for color in $known_colors with for color in $known_colors (set | string match -r '^fish_(?:pager_)?color.*$')

set --names, but yeah, that's probably better.

[mqudsi]

Actually, thanks for reminding me to use --names. I've made that mistake before!

I just pushed an update that does that; I think now we can comfortably use fish_config theme save to persist whatever theme was already loaded.

Separate from this pull request, I think it might be more correct for loading ("choosing") a theme to unset any pattern-matched variables that weren't overwritten in the new theme; e.g. theme A sets '$fish_color_custom' then theme B is loaded - I think loading theme B should maybe erase $fish_color_custom? Because a subsequent fish_config theme dump will include theme A's variable even after theme B has been chosen.

But I'm not sure if there are prompts that set any custom colors; we wouldn't want fish_config theme choose to erase a variable set by a prompt rather than a different theme..