Color functions should act as if NO_COLOR is set when fd 0 is not a tty

[halostatue]

Description

Colour functions should act as if NO_COLOR is set when non-interactive or input is not a TTY. In that way, running a function in a sub shell would not output ANSI escape codes, or when piping to another function.

Reproduction steps

For demonstration purposes, I modified the included src/lib/colors.sh to include cyan_bold hello at the bottom of the script. I’m also explicitly escaping the ANSI codes for visibility.

Actual behavior

$ bash src/lib/colors.sh # $- will not include i
\e[1;36mhello\e[0m
$ source src/lib/colors.sh # $- includes i and fd 0 is a tty
\e[1;36mhello\e[0m
$ cyan_bold hello | cat
\e[1;36mhello\e[0m

Expected behavior

$ bash src/lib/colors.sh # $- will not include i
hello
$ source src/lib/colors.sh # $- includes i and fd 0 is a tty
\e[1;36mhello\e[0m
$ cyan_bold hello | cat
hello

Proposed fix

Change

  if [[ -z ${NO_COLOR+x} ]]; then

to

  if [[ -z ${NO_COLOR+x} ]] && [[ -t 1 ]] && [[ $- == *i* ]]; then

[halostatue]

It may be desirable to introduce a FORCE_COLOR variable for those cases where it would be useful to have the ANSI output. It would not be something that would be enabled by default in any way, but recipes could be made for adding global --force-color variables. If this were added, the recommended code would be:

  if [[ -n "${FORCE_COLOR}" ]]; then
    printf "$color%b\e[0m\n" "$*"
  elif [[ -z ${NO_COLOR+x} ]] && [[ -t 1 ]] && [[ $- == *i* ]]; then
    printf "$color%b\e[0m\n" "$*"
  else
    printf "%b\n" "$*"
  fi

You could use if [[ -n "${FORCE_COLOR}" ]] || ([[ -z ${NO_COLOR+x} ]] && [[ -t 1 ]] && [[ $- == *I* ]]; then, but I think that's less readable.

[DannyBen]

Alright. I am not opposed to it, but isn't the whole purpose of NO_COLOR is to have one clear way to decide if you want or don't want colors?

One of the annoying things about no-color-when-no-tty is that when you run a colorful application through less or some other pager, you lose the colors without a good reason.

Thoughts on that?

[DannyBen]

I do not like the overly complex color detection settings. This is why we follow NO_COLOR.
Also - remember that this added library is provided as a convenience - people are free to modify it as they see fit.

They way I see it, adding a non tty detection, means there is no way to force colors, unless adding FORCE_COLOR..... which brings me to my first point... :)

[halostatue]

Unfortunately, NO_COLOR is one of those XKCD 927 standards: established late and not respected by a surprising number of old and new apps.

I agree that piping through less (and then remembering to have to do less -R, if I remember correctly) is annoying, but I’m far more frequently piping through pbcopy and then having to remember that I have to do something like terraform show plan | ansifilter | pbcopy because it doesn’t do any TTY detection.

chalk uses FORCE_COLOR (via supports-color); they also have strong opinions about NO_COLOR. I don’t know that I agree with them, as I think that they can work well together — the way that go-supportscolor does:

Info

…

For situations where using --color is not possible, use the environment variable FORCE_COLOR=1 (level 1 - 16 colors), FORCE_COLOR=2 (level 2 - 256 colors), or FORCE_COLOR=3 (level 3 - true color) to forcefully enable color, or FORCE_COLOR=0 to forcefully disable. The use of FORCE_COLOR overrides all other color support checks.

If NO_COLOR is specified and FORCE_COLOR is not, then colors will be disabled.

Given that FORCE_COLOR apparently has a numeric specification, I would change the full support to:

  if (( ${FORCE_COLOR:-1} > 0 )); then
    printf "$color%b\e[0m\n" "$*"
  elif (( ${FORCE_COLOR:-1} = 0 )); then
    printf "%b\n" "$*"
  elif [[ -z ${NO_COLOR+x} ]] && [[ -t 1 ]] && [[ $- == *i* ]]; then
    printf "$color%b\e[0m\n" "$*"
  else
    printf "%b\n" "$*"
  fi

[halostatue]

Note that the above fix works correctly when calling yellow_bold message, but does not work correctly when assigning to a variable (var=$(yellow_bold message)). This is unsurprising, but it means that what support that exists for adding colour to help text is broken by this.

I do not have a trivial solution to this specific issue.

[DannyBen]

Well, since the added color functions can be modified according to the users needs, I am leaning towards implementing the "full suite", but in a way that will help users disable or enable any of the features.

The full suite being (by order of priority)

• FORCE_COLOR
• NO_COLOR
• tty auto detection

I will take a look at it tomorrow, unless you plan on proposing a PR?

Also - why do we have two tty related conditions? [[ -t 1 ]] && [[ $- == *I* ]]
(I am asking since I would like to test both cases).

[DannyBen]

BTW the source script is here.

[halostatue]

I can do a PR, but probably not in the next 24–48 hours.

Also - why do we have two tty related conditions? [[ -t 1 ]] && [[ $- == *i* ]]
(I am asking since I would like to test both cases).

Only the first one is TTY-related. The second is shell-related and the presence of i in $- indicates that the shell was started interactive.

❯ bash -c 'echo $-'
hBc

❯ bash -i -c 'echo $-'
himBHc

That way, if you have a script which supports colour running under a non-interactive shell (such as under cron), you don’t have to remember to set NO_COLOR or FORCE_COLOR. Logs don't tend to handle ANSI escapes well.

[DannyBen]

Nice. I will have time for it tomorrow morning, so I will take first stab at it.

[DannyBen]

Well. There is a problem.

The color functions are intended to be used inside a string, like so:

echo "before $(red inline test) after"

when used like this, [[ -t 1 ]] is false.

So - the only way I can think of to support the auto detection, requires setting the color mode beforehand - at the script's initialization, and not the function itself.

Like so:

#!/usr/bin/env bash 

set_color_mode() {
  if (( ${FORCE_COLOR:-0} > 0 )); then
    wants_color=true
  elif (( ${FORCE_COLOR:-1} == 0 )); then
    wants_color=false
  elif [[ -n ${NO_COLOR+x} ]]; then
    wants_color=false
  elif [[ -t 1 ]] && [[ $- == *I* ]]; then
    wants_color=true
  else
    wants_color=false
  fi
}

print_in_color() {
  local color="$1"
  shift
  if $wants_color; then
    printf "$color%b\e[0m\n" "$*"
  else
    printf "%b\n" "$*"
  fi
}

red() { print_in_color "\e[31m" "$*"; }
green() { print_in_color "\e[32m" "$*"; }

set_color_mode

green "Direct test"
echo "And $(red inline test)"

This works in all cases that I found.

The bashly add * functions, do not do anything other than adding more files to your directory. Not modifying.
This kind of implementation will require placing the call to set_color_mode in the initialize() function.

[halostatue]

I think that’s a fine compromise, and sort of where I was going to probably implement this when I got around to it.

[DannyBen]

I think that’s a fine compromise

That's the thing. I cannot make that compromise. The bashly add * commands do not have control over the initialize. They just add files, mostly as an example, so you can edit them as you see fit.

[halostatue]

I think you can — and it doesn’t even effect initialize:

## Color functions [@bashly-upgrade colors]
## This file is a part of Bashly standard library
##
## Usage:
## Use any of the functions below to color or format a portion of a string.
##
##   echo "before $(red this is red) after"
##   echo "before $(green_bold this is green_bold) after"
##
## Color output will be disabled if `NO_COLOR` environment variable is set
## in compliance with https://no-color.org/
##

declare wants_color

print_in_color() {
  local color="$1"
  shift
  if $wants_color; then
    printf "$color%b\e[0m\n" "$*"
  else
    printf "%b\n" "$*"
  fi
}

red() { print_in_color "\e[31m" "$*"; }
green() { print_in_color "\e[32m" "$*"; }
yellow() { print_in_color "\e[33m" "$*"; }
blue() { print_in_color "\e[34m" "$*"; }
magenta() { print_in_color "\e[35m" "$*"; }
cyan() { print_in_color "\e[36m" "$*"; }
bold() { print_in_color "\e[1m" "$*"; }
underlined() { print_in_color "\e[4m" "$*"; }
red_bold() { print_in_color "\e[1;31m" "$*"; }
green_bold() { print_in_color "\e[1;32m" "$*"; }
yellow_bold() { print_in_color "\e[1;33m" "$*"; }
blue_bold() { print_in_color "\e[1;34m" "$*"; }
magenta_bold() { print_in_color "\e[1;35m" "$*"; }
cyan_bold() { print_in_color "\e[1;36m" "$*"; }
red_underlined() { print_in_color "\e[4;31m" "$*"; }
green_underlined() { print_in_color "\e[4;32m" "$*"; }
yellow_underlined() { print_in_color "\e[4;33m" "$*"; }
blue_underlined() { print_in_color "\e[4;34m" "$*"; }
magenta_underlined() { print_in_color "\e[4;35m" "$*"; }
cyan_underlined() { print_in_color "\e[4;36m" "$*"; }

if ((${FORCE_COLOR:-0} > 0)); then
  wants_color=true
elif ((${FORCE_COLOR:-1} == 0)); then
  wants_color=false
elif [[ -n ${NO_COLOR+x} ]]; then
  wants_color=false
elif [[ -t 1 ]] && [[ $- == *I* ]]; then
  wants_color=true
else
  wants_color=false
fi

Bashly takes whatever is present in src/lib/*.sh, removes ## … lines, and pastes it into a more or less appropriate place in the final script, which means that whatever is present bare in src/lib/colors.sh (or any other lib script) gets executed as if it were in an initializer because it runs before initialize gets called.

What's in src/lib/*.sh is a little "special" in that it doesn’t take the filename as (more or less) the function name. Otherwise, something like bashly add colors would need to add something like:

src/lib/print_in_color.sh
src/lib/red.sh
src/lib/red_bold.sh
src/lib/red_underline.sh
…

This may be something that you prefer not to do because it doesn’t fit your intentions of how Bashly should be create scripts, but nothing prevents users of Bashly from doing exactly that with the way that the src/lib/*.sh composition currently works.

The only bit of excess documentation that may be added for this (aside from describing FORCE_COLOR) would be that when bashly add colors is created, a new global variable is declared (currently wants_color, maybe bashly_wants_color to give some namespace protection) and can be overridden by someone who wants to implement --force-color or --no-color flags.

[DannyBen]

Unacceptable.

Bashly goes to great lengths to ensure the generated script is clean and neat and resembles how programs in "real" languages look like. Writing code outside functions, in the middle of the script is not something I am going to allow.

What users do with the lib directory is their own business. If they want to ruin it, they can. There is nothing I can or care to do about it.

[DannyBen]

I am going to close this as well. The color library - like any other bashly add * - is more of a utility to add commonly needed examples. People can shape it to their needs.

Thanks for raising it.