search

kvz / bash3boilerplate

Templates to write better Bash scripts
http://bash3boilerplate.sh
MIT License
2.12k stars 198 forks source link

using color (or not) depending on `-t 1` seems wrong #69

Closed mstreuhofer closed 7 years ago

mstreuhofer commented 7 years ago

https://github.com/kvz/bash3boilerplate/blob/9b739b4cf0c0c9f5454f7889e2417b258b1072dc/main.sh#L78

The line

if [ "${NO_COLOR}" = "true" ] || [[ "${TERM:-}" != "xterm"* ]] || [ -t 1 ]; then

seems to be wrong really in at least 2 different ways.

1) It checks STDOUT to be connected to a terminal while logging happens on STDERR only. 2) It turns OFF colors if STDOUT is actually connected to a terminal.

I'm guessing here but a quick check seems to suggest that nobody noticed until now because __b3bp_log always got called in a subshell which actually was not connected to a terminal. If the function would be just called - as suggested by shellcheck - the code would not work as expected.

Now I can fix that, but before that I wanted to ask if there is something about this code, or the decisions leading to this code, which I overlooked or I don't understand.

kvz commented 7 years ago

I think you just caught a stupid mistake by me @mstreuhofer - so kudos to you 👍