fish_pager_color_*background variables don't support -b option

[plgruener]

Eg.
set fish_pager_color_background --background=red works, while
set fish_pager_color_background -b red does not, but probably should.

While you're at it, please add some clarification to the docs:

1. The relevant section "Syntax highlighting variables" states:

   The --bold or -b switches accepted by set_color are also accepted.

   This is misleading, because for set_color, -b is equivalent to --background, while --bold is shortened to -o. Additionally, all other options (bold, italic, underline, reverse, dim) also seem to be supported, both as long and short options.

2. It's a little bit confusing that the variables ending in _background have to be set with the additional --background option. I know this is technically easier to do, and I don't even know if it would be less confusing if we could omit that. Current state seems to be:
   a) every _background variable only suppports the --background option, nothing else.
   b) every other (foreground) variable supports every styling option from set_color (i.e. bold, underline, italic, reverse, dim), but not --background.

Question: how does fish internally color these parts? Are foreground and background color handled differently, or is there a call to set_color somewhere? Because I don't know why we shouldn't be able to also color the background for fish's syntax highlighting. I noticed that the foreground colors still support the reverse option, but I'm not sure if this is handled by fish or my terminal.

Tested with both version 3.2.2 and latest build 3.2.2-388-g695027234 from git on Arch Linux.

[faho]

This is misleading, because for set_color, -b is equivalent to --background, while --bold is shortened to -o.

Ah, good catch!

It's a little bit confusing that the variables ending in _background have to be set with the additional --background option. I know this is technically easier to do, and I don't even know if it would be less confusing if we could omit that.

The color variables are supposed to be usable as arguments to set_color. If we inferred the "background" because of the name, that wouldn't work anymore. (it may be possible to not have special background variables, instead using one variable that you can add both bits to)

Question: how does fish internally color these parts? Are foreground and background color handled differently, or is there a call to set_color somewhere?

Well, no. That's the issue. This is supposed to accept arguments like set_color, but it does its own, much cheesier, argument parsing. See

fish-shell/src/output.cpp

Lines 354 to 376 in 6950272

	/// Return the internal color code representing the specified color.
	/// TODO: This code should be refactored to enable sharing with builtin_set_color.
	rgb_color_t parse_color(const env_var_t &var, bool is_background) {
	bool is_bold = false;
	bool is_underline = false;
	bool is_italics = false;
	bool is_dim = false;
	bool is_reverse = false;
	std::vector<rgb_color_t> candidates;
	const wchar_t *prefix = L"--background=";
	// wcslen is not available as constexpr
	size_t prefix_len = wcslen(prefix);
	wcstring color_name;
	for (const wcstring &next : var.as_list()) {
	color_name.clear();
	if (is_background) {
	// Look for something like "--background=red".
	if (string_prefixes_string(prefix, next)) {
	color_name = wcstring(next, prefix_len);
	}

(and of course there's a TODO, but that's not trivial because it'd really require untangling the argument parsing from set_color - which is awkward because that also has "--help", and getopt needs a null-terminated array, and... it's entirely possible, just annoying work that someone needs to do)

I noticed that the foreground colors still support the reverse option, but I'm not sure if this is handled by fish or my terminal.

Both. Fish interprets the option, and prints the corresponding escape sequences. The terminal then interprets those and emits the corresponding color. With "--reverse", it prints the "enter reverse mode" escape sequence (the same as tput rev) before the color.

a) every _background variable only suppports the --background option, nothing else.

It also allows --reverse. The reason why it doesn't support the other options is because those inherently don't apply to the background - there is no way to print a yellow background in italics. Italics is a property of the text, which is a foreground thing.

"--reverse" could be "the reverse of the background I gave". Which is weird, and it will probably also apply to the text, but at least it will have an effect on the background.

b) every other (foreground) variable supports every styling option from set_color (i.e. bold, underline, italic, reverse, dim), but not --background.

#25

---

Alright, so let's first fix the issue at hand - you can't do --background red. Then, later, we can untangle the weirdness that is "parse_color" and actually do full argument parsing.

Then, after that, we can solve #25 and allow background colors for more colors, and then possibly decide that we don't need some special background variables anymore.