ansi command should not output ANSI color codes when use_ansi_coloring is disabled in config

[NotTheDr01ds]

Describe the bug

This is the root cause of #14043.

In (unrelated to 14043) digging around in the ansi command, it comes to mind that the reason why user-land ANSI was still coloring output in #14043 is that the ansi command itself still outputs color codes, even when use_ansi_coloring is false.

This shouldn't happen.

The ansi command actually checks the use_ansi_coloring option, but only to determine if color previews should be shown with ansi --list/-l. It should also check before outputting a color (or attribute) code.

How to reproduce

Note: to reproduce correctly, each line of the following must be executed separately; otherwise the environment won't be re-read before the command is executed ...

$env.use_ansi_coloring = false
$"(ansi red)This shouldn't be Red, but it is(ansi reset)"

Expected behavior

I expect the ansi command to disregard color and attribute codes when use_ansi_coloring is false. ANSI codes related to cursor positioning, clearing the screen, and others should still be output.

Configuration

key	value
version	0.98.1
major	0
minor	98
patch	1
branch	virtual-mods
commit_hash	1dc58b8
build_os	linux-x86_64
build_target	x86_64-unknown-linux-gnu
rust_version	rustc 1.79.0 (129f3b996 2024-06-10)
rust_channel	1.79.0-x86_64-unknown-linux-gnu
cargo_version	cargo 1.79.0 (ffa9cf99a 2024-06-03)
build_time	2024-10-09 13:39:29 -04:00
build_rust_channel	release
allocator	mimalloc
features	default, sqlite, trash
installed_plugins

[weirdan]

I would expect it to work even if use_ansi_coloring is false. IMHO the command is too low-level to take into account shell preferences.

[NotTheDr01ds]

I can probably fix this:

• (More Easily) By just grouping color and attribute codes in the first x elements in the codes array. This is already how ansi -l determines which codes to preview (or not) based on use_ansi_coloring
• (More Safely) By adding a new boolean field to the AnsiCode struct indicating whether or not that code should be output when ANSI coloring is disabled.

The problem with the first method (as well as for ansi -l is that it relies on every subsequent contributor understanding where to place new items in the vector. This can be handled with comments, but it's not foolproof, of course.

[NotTheDr01ds]

@weirdan The problem is that any Nushell code that uses it (e.g., std, the default prompt, and others) doesn't honor the setting without extensive additional logic. I expect the ansi command to have that ability built-in, personally.

It's kind of ironic that it already does have logic built-in for it, but only for previews.

[NotTheDr01ds]

We could have a flag to override the use_ansi_coloring setting if needed.

[132ikl]

I also think ansi should respect the use_ansi_coloring setting. Some folks seem to care a lot about this, and I think that if you want ansi coloring off, you want it completely off, not "off except sometimes on because someone had ex. ansi red in a script".

[weirdan]

The whole reason people have ansi red in their scripts is because they lack a more high-level abstraction like show error ... (that could take use_ansi_coloring into account).

[NotTheDr01ds]

@weirdan Can you elaborate? Are you saying that someone would turn off use_ansi_coloring and then put an ansi red in their script because error make then doesn't appear in color? That seems paradoxical, so I'm guessing you mean something else.

I think:

1. It's unlikely that the same person would turn off use_ansi_coloring and then turn around and put an ansi red (etc.) in a script.
2. In the event the script author wanted colors while debugging, wouldn't they just set use_ansi_coloring: true?
3. It's far more likely that a script author has put ansi red in a script and then a user of that script/module/etc. wants to disable it via use_ansi_coloring: false.
4. In the rare event that (2) above doesn't work for a use-case, we could have a --force flag on ansi, I guess. But I'd hate for script authors to force it on other users who have disabled it.

Ultimately the most likely use case for $env.config.use_ansi_coloring: false is probably accessibility. Color-blind individuals may have a difficult time differentiating certain colors from the background, so they turn off all coloring. We need to support that even for third-party (and first party, as seen in std/banner and the prompts), I believe.

[weirdan]

Are you saying that someone would turn off use_ansi_coloring and then put an ansi red in their script because error make then doesn't appear in color?

No, I'm saying that most users (like 95% of them) shouldn't need ansi red at all. What they need is something like show message --style=error ... instead. Similar to std log error but less 'loggy'. And whatever implements that (let's call it a widget library for the sake of argument) should account for use_ansi_coloring value.

[NotTheDr01ds]

What they need is something like show message --style=error ... instead. Similar to std log error but less 'loggy'. And whatever implements that (let's call it a widget library for the sake of argument) should account for use_ansi_coloring value.

That just seems to make the issue more intractable.

The "should account for" -- Maybe they should, but I really doubt that's going to happen, and we already have several existence proofs for that. While std/log handles this correctly, the banner command didn't it was pointed out. And our standard prompt config still doesn't. Why do we think third party module authors are going to think to do it?

And if they don't (and they aren't), then their output is going to display incorrectly for some users, most importantly those who need to disable it for accessibility reasons. We shouldn't then expect that end-user to have to go to each module author to ask them to fix it.

And why even put the burden on the module author - Why make every command/prompt/whatever author have to special-case it when we can do it once at the source? Clearly the "whatever implements that" should be using ansi red as well. If we fix the problem in the internal ansi statement, then we fix it for all third-party modules/commands/etc.

[NotTheDr01ds]

Alternatively, I could see just recommending a hook.display_output where everything is ansi strip'd. Of course, then I'm not quite sure what the use of $env.config.use_ansi_coloring would really be in the first place.

[weirdan]

If we fix the problem in the internal ansi statement, then we fix it for all third-party modules/commands/etc.

Because the problem is not in the internal ansi command. It does what it says - generates an ANSI escape sequence for the specified colour. I'd say it would have a problem if it stopped doing that in some circumstances. It's bad enough with terminal colour schemes that also remap colours.

[NotTheDr01ds]

But the whole point of use_ansi_coloring: false is to tell internal Nushell commands to not generate ANSI escape sequences for colors. IMHO we have a problem that the ansi command is not respecting that. Either way, someone is going to consider it a problem, apparently ;-).