Prompt element colours will not use customised colours

[kopischke]

Because the colours of the prompt elements like the success and error prompt symbols (pure_color_prompt_on_symbols and pure_color_prompt_on_error) are assigned and exported in a conf.d snippet, which is sourced before the user configuration file (config.fish), overriding colour variables (pure_color_X) in config.fish per the README will not actually affect the colours of elements assigned this colour in interactive login shell sessions.

To reproduce

on macOS 10.14, using Terminal.app and fish set as default shell via chsh

1. install pure

2. add the following line to your config.fish:

   set --export pure_color_success (set_color green)

3. start a new fish session by opening a new Terminal window

Expected result: the success prompt symbol is green

Actual result: the success prompt symbol is magenta

Notes: oddly enough, this works as expected in a subshell (“oddly enough” because the fish help clearly states all configuration files, including snippets, are read on every shell start, but this behaviour only makes sense if fish actually skips reading snippets in a non-login shell).

[edouard-lopez]

You will need to erase the variable before overriding them as discussed in #138

set --erase pure_color_success
set --global pure_color_success (set_color green)

[andreiborisov]

@edouard-lopez apart from the fact that we need to elaborate this in README, as anything we can do to simplify this?

[andreiborisov]

So, just to be clear, the problem lies in the fact that config.fish sourced the last one after all other files in conf.d (see https://fishshell.com/docs/current/index.html#initialization), but variables evaluate they value on set instead of storing reference.

Here is a couple of solutions:

1. We can store feature variables as text effectively storing reference instead of the value

set $pure_color_prompt_on_error \$pure_color_danger

and evaluate their values on invocation

eval echo $pure_color_prompt_on_error

Obviously this is quite ugly and hacky (maybe even a bit slower).

2. We can provide function pure_set which would do appropriate erasing for the user. Somewhat acceptable, but the need to use special function to set variables is unintuitive.

3. We can recommend users to set pure color variables as universal so it won't be rewritten by _pure_set_default. This is probably the most idiomatic solution.

4. We can move pure.fish from conf.d and source it manually from config.fish (like we used to), so it will be executed after config.fish and _pure_set_default would work as intended.

If @jorgebucaran like to chime in and talk wisdom to us, that would be very helpful too☺️

[jorgebucaran]

Don't tell people to use pure_color_success, pure_color_mute, etc. Use them internally to set the actual variables and tell people to use pure_color_prompt_on_success, etc.

set -x pure_color_prompt_on_success (set_color green)

[andreiborisov]

I would like to salvage ability to set base colors for users though, seems useful to me

[jorgebucaran]

@schrodincat Then something like this.

function _pure_prompt_symbol
    set -l prompt_color $pure_color_prompt_on_success
    if set -q pure_color_success
        set prompt_color $pure_color_success
    end

    # ...

    echo "$prompt_color$prompt_symbol"
end

[andreiborisov]

@jorgebucaran although it implicitly assumes that $pure_color_prompt_on_success should inherit from $pure_color_success (which is not always the case, for example, when a user sets base $pure_color_success color and tweaks specific feature$pure_color_prompt_on_success color), this seems like a nice middle ground.

For now, though, I've tried to fix this making base color variables universal.

[kopischke]

@schrodincat I very much appreciate you trying to tackle the issue, but I am not sure how making colour variables universal will achieve this? As you stated quite correctly

the problem lies in the fact that config.fish sourced the last one after all other files in conf.d (see https://fishshell.com/docs/current/index.html#initialization), but variables evaluate they value on set instead of storing reference.

to my best understanding, this means that after the changes

1. conf.d/pure.fish will still unconditionally set the colour defaults , as neither it nor _pure_set_default.fish check if the variable is set, [EDIT: _pure_set_default.fish actually does, this was an error of mine (or, for the Americans: “my bad”)] and fish’s set command unconditionally sets a variable value. Also, before somebody waves --erase like a magic wand in front of my nose again: unless I completely misunderstand both fish’s docs and its actual behaviour in my terminal, that flag is useful to prevent scope shadowing issues; it is not needed to re-assign a variable;
2. the colour assignment to $pure_color_prompt_on_success in conf.d/pure.fish will still be static;
3. changes to base colours in config.fish thus will still not propagate to derived properties.

Am I missing something? To me, it appears that the universal variable solution would only work if

1. either conf.d/pure.fish or _pure_set_default.fish bail when a variable is already set;
2. you instruct users to re-start their fish session after setting universal colour variables for them to take effect, which is counterintuitive as the whole point of universal vars is that they need not be set for every session.

Assuming my analysis is correct, making base colours universal is the right first step, but you will need a dynamic evaluation of the assignment of base colours to derived prompt properties, be it through an eval hack or through an intermediary function, as the second. If you want a user friendly way of configuring base colours, that is, which I happen to think is the correct approach.

[andreiborisov]

@kopischke

conf.d/pure.fish will still unconditionally set the colour defaults, as neither it nor _pure_set_default.fish check if the variable is set

_pure_set_default always checked for the existence of a variable before setting it, that's basically the reason it exists.

you instruct users to re-start their fish session after setting universal color variables for them to take effect, which is counterintuitive as the whole point of universal vars is that they need not be set for every session

Universal variables are meant to be set mainly by hand in a terminal, it's kind of a replacement for config.fish in that sense: instead of setting a variable on every shell load in config.fish similar to what you'd do in bash, you only set the variable once and it persists across sessions (stored in fish_variables file). This file loads before files in conf.d directory. So if you'll do set -U pure_color_success (set_color green) for example, the new value propagates to feature colors on the next load. You still can add the same command in the config.fish, however there will be a minor inconvenience indeed, the new value won't propagate to feature colors on the first load. Only on the second one. But since universal variables are not really meant to be set this way, I see it as an acceptable trade-off.

In my opinion, all other proposed solutions are making the code either slower or too convoluted because they are essentially trying to fight how fish actually works.

[kopischke]

@schrodincat

_pure_set_default always checked for the existence of a variable before setting it, that's basically the reason it exists.

So it does, you are right and I was wrong; apologies for a hasty misreading.

Universal variables are meant to be set mainly by hand in a terminal, it's kind of a replacement for config.fish in that sense …

You are missing one essential behaviour of universal variables, the one making them far more than a way to set variables persistently without touching config.fish: they immediately affect all running fish sessions (that is where the “universal” comes from) – no need to restart the session. As the fish docs state succinctly:

Universal variables are variables that are shared between all the users' fish sessions on the computer. Fish stores many of its configuration options as universal variables. This means that in order to change fish settings, all you have to do is change the variable value once, and it will be automatically updated for all sessions, and preserved across computer reboots and login/logout.

(emphasis mine). I have made a GIF illustrating exactly that:

This behaviour makes uvars ideal for shell wide persistent user settings; in fact, as the docs mention, one of their functions out of the box is to set fish’s own colours persistently and across all sessions 😉. Switching pure’s colour variables to universal vars thus is a good step; the problem is these universal variables will currently still only be assigned to prompt elements on shell start-up, no matter how the uvars were set (the right way, interactively, or the pointless way, in config.fish). That in turn means you still need to launch a new session for the assignment to the variable to take effect after a change, which is exactly what using universal variables is meant to avoid.

[andreiborisov]

@kopischke I think I misunderstood jorgebucaran solution the first time. I’ll be able to solve this issue without enforcing universal scope on to variables, like you want me too, in more idiomatic way. PR is coming, however, it won’t be available till the end of the next week.

[kopischke]

@schrodincat very cool! Thanks for that, and no rush.