Support the NO_COLOR environment variable

  I ran across this site:

    https://no-color.org/

... which lobbies for tools to recognize the NO_COLOR environment
variable and disable any terminal styling when it is seen.

This patch implements this for gdb.

Regression tested on x86-64 Fedora 38.
---
 gdb/NEWS                                       |  4 ++++
 gdb/doc/gdb.texinfo                            |  3 +++
 gdb/main.c                                     |  4 ++++
 gdb/testsuite/gdb.base/early-init-file.exp     |  3 ++-
 .../gdb.base/style-interp-exec-mi.exp          |  3 ++-
 gdb/testsuite/gdb.base/style-logging.exp       |  3 ++-
 gdb/testsuite/gdb.base/style.exp               |  9 ++++++---
 .../build-id-no-debug-warning.exp              |  3 ++-
 gdb/testsuite/gdb.debuginfod/crc_mismatch.exp  |  3 ++-
 gdb/testsuite/gdb.python/py-format-string.exp  |  3 ++-
 gdb/testsuite/gdb.python/py-source-styling.exp |  3 ++-
 gdb/testsuite/gdb.python/py-startup-opt.exp    |  3 ++-
 gdb/testsuite/gdb.rust/rust-style.exp          |  3 ++-
 gdb/testsuite/gdb.tui/tui-layout.exp           | 18 +++++++++++-------
 gdb/testsuite/lib/gdb.exp                      |  2 +-
 gdb/testsuite/lib/tuiterm.exp                  |  3 ++-
 16 files changed, 49 insertions(+), 21 deletions(-)
  

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Fri, 15 Sep 2023 18:09:08 -0600
> I ran across this site:
> 
>     https://no-color.org/
> 
> ... which lobbies for tools to recognize the NO_COLOR environment
> variable and disable any terminal styling when it is seen.
> 
> This patch implements this for gdb.

Should we also support the --color command-line argument, e.g. to
countermand NO_COLOR in the environment, and also as an alternative to
the environment variable?

> ---
>  gdb/NEWS                                       |  4 ++++
>  gdb/doc/gdb.texinfo                            |  3 +++
>  gdb/main.c                                     |  4 ++++
>  gdb/testsuite/gdb.base/early-init-file.exp     |  3 ++-
>  .../gdb.base/style-interp-exec-mi.exp          |  3 ++-
>  gdb/testsuite/gdb.base/style-logging.exp       |  3 ++-
>  gdb/testsuite/gdb.base/style.exp               |  9 ++++++---
>  .../build-id-no-debug-warning.exp              |  3 ++-
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp  |  3 ++-
>  gdb/testsuite/gdb.python/py-format-string.exp  |  3 ++-
>  gdb/testsuite/gdb.python/py-source-styling.exp |  3 ++-
>  gdb/testsuite/gdb.python/py-startup-opt.exp    |  3 ++-
>  gdb/testsuite/gdb.rust/rust-style.exp          |  3 ++-
>  gdb/testsuite/gdb.tui/tui-layout.exp           | 18 +++++++++++-------
>  gdb/testsuite/lib/gdb.exp                      |  2 +-
>  gdb/testsuite/lib/tuiterm.exp                  |  3 ++-
>  16 files changed, 49 insertions(+), 21 deletions(-)

Thanks, the documentation parts of this are approved.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

On Sat, Sep 16, 2023 at 5:35 AM Eli Zaretskii via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> > From: Tom Tromey <tom@tromey.com>
> > Cc: Tom Tromey <tom@tromey.com>
> > Date: Fri, 15 Sep 2023 18:09:08 -0600
> > I ran across this site:
> >
> >     https://no-color.org/
> >
> > ... which lobbies for tools to recognize the NO_COLOR environment
> > variable and disable any terminal styling when it is seen.
> >
> > This patch implements this for gdb.
>
> Should we also support the --color command-line argument, e.g. to
> countermand NO_COLOR in the environment, and also as an alternative to
> the environment variable?
>

I'm not sure a specific --color command-line argument is necessary,
the arg --early-init-eval-command 'set style enabled' (-eiex, which I
noticed is missing from --help.) works even when NO_COLOR is set,
appropriately printing colors in the non-quiet startup message.  So
even without something specific like --color it is possible to
override the env var.  IMO the need for specific commands for things
we want to happen early like `--quiet` vs `-eiex 'set
startup-quietly'` feel to me like holdovers from before
early evaluation existed when a specific command line arg like
`--quiet` needed early evaluation to have any effect.  But perhaps
that is just an opinion.
  

> From: Matt Rice <ratmice@gmail.com>
> Date: Sat, 16 Sep 2023 08:39:34 +0000
> Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
> 
> On Sat, Sep 16, 2023 at 5:35 AM Eli Zaretskii via Gdb-patches
> <gdb-patches@sourceware.org> wrote:
> >
> > Should we also support the --color command-line argument, e.g. to
> > countermand NO_COLOR in the environment, and also as an alternative to
> > the environment variable?
> 
> I'm not sure a specific --color command-line argument is necessary,
> the arg --early-init-eval-command 'set style enabled' (-eiex, which I
> noticed is missing from --help.) works even when NO_COLOR is set,
> appropriately printing colors in the non-quiet startup message.  So
> even without something specific like --color it is possible to
> override the env var.

Since this is basically a compatibility feature, I thought that having
a related --color=WHEN command-line option would also be nice, for the
same reasons.  But if I'm the only one who thinks that, it's fine by
me not to have such an option.
  

Tom Tromey <tom@tromey.com> writes:

> I ran across this site:
>
>     https://no-color.org/
>
> ... which lobbies for tools to recognize the NO_COLOR environment
> variable and disable any terminal styling when it is seen.

I've never understood the passionate hatred some folk have for coloured
output!  But NO_COLOR sounds like a fairly reasonable request, so I'm in
favour of adding this.  I did have some thoughts on the testing...

>
> This patch implements this for gdb.
>
> Regression tested on x86-64 Fedora 38.
> ---
>  gdb/NEWS                                       |  4 ++++
>  gdb/doc/gdb.texinfo                            |  3 +++
>  gdb/main.c                                     |  4 ++++
>  gdb/testsuite/gdb.base/early-init-file.exp     |  3 ++-
>  .../gdb.base/style-interp-exec-mi.exp          |  3 ++-
>  gdb/testsuite/gdb.base/style-logging.exp       |  3 ++-
>  gdb/testsuite/gdb.base/style.exp               |  9 ++++++---
>  .../build-id-no-debug-warning.exp              |  3 ++-
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp  |  3 ++-
>  gdb/testsuite/gdb.python/py-format-string.exp  |  3 ++-
>  gdb/testsuite/gdb.python/py-source-styling.exp |  3 ++-
>  gdb/testsuite/gdb.python/py-startup-opt.exp    |  3 ++-
>  gdb/testsuite/gdb.rust/rust-style.exp          |  3 ++-
>  gdb/testsuite/gdb.tui/tui-layout.exp           | 18 +++++++++++-------
>  gdb/testsuite/lib/gdb.exp                      |  2 +-
>  gdb/testsuite/lib/tuiterm.exp                  |  3 ++-
>  16 files changed, 49 insertions(+), 21 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 98ff00d5efc..aa5fc9aa494 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,10 @@
>  
>  *** Changes since GDB 13
>  
> +* GDB now recognizes the NO_COLOR environment variable and disables
> +  styling according to the spec.  See https://no-color.org/.
> +  Styling can be re-enabled with "set style enabled on".
> +
>  * The AArch64 'org.gnu.gdb.aarch64.pauth' Pointer Authentication feature string
>    has been deprecated in favor of the 'org.gnu.gdb.aarch64.pauth_v2' feature
>    string.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9b7e06f3156..2892dd7765c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27229,6 +27229,9 @@ and styles can also be disabled entirely.
>  Enable or disable all styling.  The default is host-dependent, with
>  most hosts defaulting to @samp{on}.
>  
> +If the @env{NO_COLOR} environment variable is set to a non-empty
> +value, then @value{GDBN} will change this to @samp{off} at startup.

I wonder if we should extend this paragraph with the suggestion Matt made
(about using --early-init-eval-command 'set style enabled' to override
NO_COLOR) in order to address Eli's concern?

> +
>  @item show style enabled
>  Show the current state of styling.
>  
> diff --git a/gdb/main.c b/gdb/main.c
> index cf46f6acb20..97e04f5b5d8 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -654,6 +654,10 @@ captured_main_1 (struct captured_main_args *context)
>    int save_auto_load;
>    int ret = 1;
>  
> +  const char *no_color = getenv ("NO_COLOR");
> +  if (no_color != nullptr && *no_color != '\0')
> +    cli_styling = false;
> +
>  #ifdef HAVE_USEFUL_SBRK
>    /* Set this before constructing scoped_command_stats.  */
>    lim_at_start = (char *) sbrk (0);
> diff --git a/gdb/testsuite/gdb.base/early-init-file.exp b/gdb/testsuite/gdb.base/early-init-file.exp
> index 237439f9db7..ff0818b26ba 100644
> --- a/gdb/testsuite/gdb.base/early-init-file.exp
> +++ b/gdb/testsuite/gdb.base/early-init-file.exp
> @@ -99,9 +99,10 @@ proc check_gdb_startups_up_quietly { message } {
>      }
>  }
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""

Rather than all these changes, I'd propose the patch I've attached at
the end of this email -- this adds a new 'with_ansi_styling_terminal'
proc to lib/gdb.exp that moves all this environment setup into a single
place.

In my proposal I chose to unset NO_COLOR rather than setting it to an
empty value -- I figure that will be the state most users run in, so we
want to ensure that works.  It might be worth an extra test that
specifically checks with NO_COLOR set to an empty value ... but I
haven't done that in the patch below.

>  
>      # Start GDB and confirm that the version string is styled.
>      save_vars { INTERNAL_GDBFLAGS env(HOME) env(XDG_CONFIG_HOME) } {
> diff --git a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> index 9b2ed87513f..00cccf7d300 100644
> --- a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> +++ b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> @@ -23,9 +23,10 @@
>  
>  standard_testfile
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""
>  
>      if { [prepare_for_testing "failed to prepare" \
>  	    ${testfile} ${srcfile}] } {
> diff --git a/gdb/testsuite/gdb.base/style-logging.exp b/gdb/testsuite/gdb.base/style-logging.exp
> index a156c39146f..f82e714448f 100644
> --- a/gdb/testsuite/gdb.base/style-logging.exp
> +++ b/gdb/testsuite/gdb.base/style-logging.exp
> @@ -22,9 +22,10 @@ require {!is_remote host}
>  
>  standard_testfile style.c
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""
>  
>      if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
>  	return -1
> diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
> index 60f909e2402..1afce166e93 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -69,9 +69,10 @@ proc run_style_tests { } {
>      global testfile srcfile hex binfile
>      global currently_disabled_style decimal hex
>  
> -    save_vars { env(TERM) } {
> +    save_vars { env(TERM) env(NO_COLOR) } {
>  	# We need an ANSI-capable terminal to get the output.
>  	setenv TERM ansi
> +	setenv NO_COLOR ""
>  
>  	# Restart GDB with the correct TERM variable setting, this
>  	# means that GDB will enable styling.
> @@ -319,9 +320,10 @@ proc run_style_tests { } {
>  # Python pygments module is not available, so, when we disable
>  # disassembler styling, we should always see a change in output.
>  proc test_disable_disassembler_styling { } {
> -    save_vars { env(TERM) } {
> +    save_vars { env(TERM) env(NO_COLOR) } {
>  	# We need an ANSI-capable terminal to get the output.
>  	setenv TERM ansi
> +	setenv NO_COLOR ""
>  
>  	# Restart GDB with the correct TERM variable setting, this
>  	# means that GDB will enable styling.
> @@ -427,9 +429,10 @@ proc test_disassembler_error_handling { } {
>  	return
>      }
>  
> -    save_vars { env(TERM) } {
> +    save_vars { env(TERM) env(NO_COLOR) } {
>  	# We need an ANSI-capable terminal to get the output.
>  	setenv TERM ansi
> +	setenv NO_COLOR ""
>  
>  	# Restart GDB with the correct TERM variable setting, this
>  	# means that GDB will enable styling.
> diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> index 148363a7c05..be7041a7520 100644
> --- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> +++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
> @@ -98,9 +98,10 @@ gdb_test "file ${build_id_debug_file}" \
>  # Do the same thing again, but this time check that the styling is
>  # correct.
>  with_test_prefix "check styling" {
> -    save_vars { env(TERM) } {
> +    save_vars { env(TERM) env(NO_COLOR) } {
>  	# We need an ANSI-capable terminal to get styled output.
>  	setenv TERM ansi
> +	setenv NO_COLOR ""
>  
>  	clean_restart
>  
> diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> index 074028eadfd..51d2073f4ae 100644
> --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -71,9 +71,10 @@ if {[build_executable crc_mismatch.exp crc_mismatch-2 crc_mismatch-2.c debug] !=
>      # Do the same thing again, but this time check that the styling is
>      # correct.
>      with_test_prefix "check styling" {
> -	save_vars { env(TERM) } {
> +	save_vars { env(TERM) env(NO_COLOR) } {
>  	    # We need an ANSI-capable terminal to get styled output.
>  	    setenv TERM ansi
> +	    setenv NO_COLOR ""
>  
>  	    clean_restart
>  
> diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
> index 55a37766bff..e9bc1fac097 100644
> --- a/gdb/testsuite/gdb.python/py-format-string.exp
> +++ b/gdb/testsuite/gdb.python/py-format-string.exp
> @@ -1184,11 +1184,12 @@ with_test_prefix "format_string" {
>    # Perform C Tests.
>    if { [build_inferior "${binfile}" "c"] == 0 } {
>      with_test_prefix "lang_c" {
> -      save_vars { env(TERM) } {
> +      save_vars { env(TERM) env(NO_COLOR) } {
>  	# We run all of these tests in an environment where styling
>  	# could work, but we only expect the final call to
>  	# test_styling to actually produce any styled output.
>  	setenv TERM ansi
> +	setenv NO_COLOR ""
>  	set current_lang "c"
>  	prepare_gdb "${binfile}"
>  	test_all_common
> diff --git a/gdb/testsuite/gdb.python/py-source-styling.exp b/gdb/testsuite/gdb.python/py-source-styling.exp
> index f92d6f7a706..7ecec185dbd 100644
> --- a/gdb/testsuite/gdb.python/py-source-styling.exp
> +++ b/gdb/testsuite/gdb.python/py-source-styling.exp
> @@ -20,12 +20,13 @@ load_lib gdb-python.exp
>  
>  standard_testfile
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output, additionally
>      # we need to set LC_ALL so GDB knows the terminal is UTF-8
>      # capable, otherwise we'll get a UnicodeEncodeError trying to
>      # encode the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""
>  
>      if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
>  	return -1
> diff --git a/gdb/testsuite/gdb.python/py-startup-opt.exp b/gdb/testsuite/gdb.python/py-startup-opt.exp
> index 08d1c79eee3..ed2ba2d6b64 100644
> --- a/gdb/testsuite/gdb.python/py-startup-opt.exp
> +++ b/gdb/testsuite/gdb.python/py-startup-opt.exp
> @@ -97,9 +97,10 @@ proc test_python_settings { exp_state } {
>      gdb_exit
>  }
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""
>  
>      # Check the features are off by default.
>      test_python_settings "off"
> diff --git a/gdb/testsuite/gdb.rust/rust-style.exp b/gdb/testsuite/gdb.rust/rust-style.exp
> index 0555ea4b8a4..0d0e78bbfdc 100644
> --- a/gdb/testsuite/gdb.rust/rust-style.exp
> +++ b/gdb/testsuite/gdb.rust/rust-style.exp
> @@ -19,9 +19,10 @@ load_lib rust-support.exp
>  require allow_rust_tests
>  require {can_compile rust}
>  
> -save_vars { env(TERM) } {
> +save_vars { env(TERM) env(NO_COLOR) } {
>      # We need an ANSI-capable terminal to get the output.
>      setenv TERM ansi
> +    setenv NO_COLOR ""
>  
>      standard_testfile .rs
>      if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> diff --git a/gdb/testsuite/gdb.tui/tui-layout.exp b/gdb/testsuite/gdb.tui/tui-layout.exp
> index 13823387e72..90f27c5eac1 100644
> --- a/gdb/testsuite/gdb.tui/tui-layout.exp
> +++ b/gdb/testsuite/gdb.tui/tui-layout.exp
> @@ -44,13 +44,17 @@ proc test_layout_or_focus {layout_name terminal execution} {
>  
>      set dumb_terminal [string equal $terminal "dumb"]
>  
> -    if {$dumb_terminal} {
> -	clean_restart $binfile
> -    } else {
> -	Term::clean_restart 24 80 $binfile
> -	if {![Term::prepare_for_tui]} {
> -	    unsupported "TUI not supported"
> -	    return
> +    global env
> +    save_vars { env(TERM) } {
> +	setenv TERM $terminal
> +	if {$dumb_terminal} {
> +	    clean_restart $binfile
> +	} else {
> +	    Term::clean_restart 24 80 $binfile
> +	    if {![Term::prepare_for_tui]} {
> +		unsupported "TUI not supported"
> +		return
> +	    }

I'm not sure that this change is needed.  I think I might be to blame
for adding the $terminal argument here, and, on reflection, I think
passing ansi / dumb was probably a poor choice - I should have just
passed a boolean called $tui_terminal_setup ... or something, as that
was really the intention.

I'm not sure this change really hurts -- TERM should be set to 'dumb' by
default, and once we call Term::clean_restart we're going to set TERM to
'ansi' anyway -- but I think this adds some unnecessary noise.


>  	}
>      }
>  
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b9179401c4..0a4f74d8000 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6468,7 +6468,7 @@ proc default_gdb_init { test_file_name } {
>  
>      # This disables style output, which would interfere with many
>      # tests.
> -    setenv TERM "dumb"
> +    setenv NO_COLOR sorry
>  
>      # This setting helps detect bugs in the Python code and doesn't
>      # seem to have a significant downside for the tests.
> diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
> index 598c1f8cbdf..854fde56393 100644
> --- a/gdb/testsuite/lib/tuiterm.exp
> +++ b/gdb/testsuite/lib/tuiterm.exp
> @@ -844,8 +844,9 @@ namespace eval Term {
>      # BODY.
>      proc with_tuiterm {rows cols body} {
>  	global env stty_init
> -	save_vars {env(TERM) stty_init} {
> +	save_vars {env(TERM) env(NO_COLOR) stty_init} {
>  	    setenv TERM ansi
> +	    setenv NO_COLOR ""
>  	    _setup $rows $cols
>  
>  	    uplevel $body
> -- 
> 2.41.0

Thanks,
Andrew

---

diff --git a/gdb/testsuite/gdb.base/early-init-file.exp b/gdb/testsuite/gdb.base/early-init-file.exp
index ff0818b26ba..6426da8dacf 100644
--- a/gdb/testsuite/gdb.base/early-init-file.exp
+++ b/gdb/testsuite/gdb.base/early-init-file.exp
@@ -99,10 +99,7 @@ proc check_gdb_startups_up_quietly { message } {
     }
 }
 
-save_vars { env(TERM) env(NO_COLOR) } {
-    # We need an ANSI-capable terminal to get the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
+with_ansi_styling_terminal {
 
     # Start GDB and confirm that the version string is styled.
     save_vars { INTERNAL_GDBFLAGS env(HOME) env(XDG_CONFIG_HOME) } {
diff --git a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
index 00cccf7d300..b99e767a034 100644
--- a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
+++ b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
@@ -23,11 +23,7 @@
 
 standard_testfile
 
-save_vars { env(TERM) env(NO_COLOR) } {
-    # We need an ANSI-capable terminal to get the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
-
+with_ansi_styling_terminal {
     if { [prepare_for_testing "failed to prepare" \
 	    ${testfile} ${srcfile}] } {
 	return
diff --git a/gdb/testsuite/gdb.base/style-logging.exp b/gdb/testsuite/gdb.base/style-logging.exp
index f82e714448f..c21c3321bde 100644
--- a/gdb/testsuite/gdb.base/style-logging.exp
+++ b/gdb/testsuite/gdb.base/style-logging.exp
@@ -22,11 +22,7 @@ require {!is_remote host}
 
 standard_testfile style.c
 
-save_vars { env(TERM) env(NO_COLOR) } {
-    # We need an ANSI-capable terminal to get the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
-
+with_ansi_styling_terminal {
     if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
 	return -1
     }
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 1afce166e93..7ee1b44177f 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -69,11 +69,7 @@ proc run_style_tests { } {
     global testfile srcfile hex binfile
     global currently_disabled_style decimal hex
 
-    save_vars { env(TERM) env(NO_COLOR) } {
-	# We need an ANSI-capable terminal to get the output.
-	setenv TERM ansi
-	setenv NO_COLOR ""
-
+    with_ansi_styling_terminal {
 	# Restart GDB with the correct TERM variable setting, this
 	# means that GDB will enable styling.
 	clean_restart_and_disable "restart 1" ${binfile}
diff --git a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
index be7041a7520..8ee3bb0e78c 100644
--- a/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
+++ b/gdb/testsuite/gdb.debuginfod/build-id-no-debug-warning.exp
@@ -98,11 +98,7 @@ gdb_test "file ${build_id_debug_file}" \
 # Do the same thing again, but this time check that the styling is
 # correct.
 with_test_prefix "check styling" {
-    save_vars { env(TERM) env(NO_COLOR) } {
-	# We need an ANSI-capable terminal to get styled output.
-	setenv TERM ansi
-	setenv NO_COLOR ""
-
+    with_ansi_styling_terminal {
 	clean_restart
 
 	gdb_test_no_output "set debug-file-directory ${debug_file_directory}" \
diff --git a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
index 51d2073f4ae..e88c4b3c653 100644
--- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
+++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
@@ -71,11 +71,7 @@ if {[build_executable crc_mismatch.exp crc_mismatch-2 crc_mismatch-2.c debug] !=
     # Do the same thing again, but this time check that the styling is
     # correct.
     with_test_prefix "check styling" {
-	save_vars { env(TERM) env(NO_COLOR) } {
-	    # We need an ANSI-capable terminal to get styled output.
-	    setenv TERM ansi
-	    setenv NO_COLOR ""
-
+	with_ansi_styling_terminal {
 	    clean_restart
 
 	    set debug_file_re [style "${escapedobjdirsubdir}/crc_mismatch-2\\.debug" file]
diff --git a/gdb/testsuite/gdb.python/py-format-string.exp b/gdb/testsuite/gdb.python/py-format-string.exp
index e9bc1fac097..04d19e483d9 100644
--- a/gdb/testsuite/gdb.python/py-format-string.exp
+++ b/gdb/testsuite/gdb.python/py-format-string.exp
@@ -1184,12 +1184,10 @@ with_test_prefix "format_string" {
   # Perform C Tests.
   if { [build_inferior "${binfile}" "c"] == 0 } {
     with_test_prefix "lang_c" {
-      save_vars { env(TERM) env(NO_COLOR) } {
+      with_ansi_styling_terminal {
 	# We run all of these tests in an environment where styling
 	# could work, but we only expect the final call to
 	# test_styling to actually produce any styled output.
-	setenv TERM ansi
-	setenv NO_COLOR ""
 	set current_lang "c"
 	prepare_gdb "${binfile}"
 	test_all_common
diff --git a/gdb/testsuite/gdb.python/py-source-styling.exp b/gdb/testsuite/gdb.python/py-source-styling.exp
index 7ecec185dbd..d3ae339661c 100644
--- a/gdb/testsuite/gdb.python/py-source-styling.exp
+++ b/gdb/testsuite/gdb.python/py-source-styling.exp
@@ -20,14 +20,11 @@ load_lib gdb-python.exp
 
 standard_testfile
 
-save_vars { env(TERM) env(NO_COLOR) } {
+with_ansi_styling_terminal {
     # We need an ANSI-capable terminal to get the output, additionally
     # we need to set LC_ALL so GDB knows the terminal is UTF-8
     # capable, otherwise we'll get a UnicodeEncodeError trying to
     # encode the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
-
     if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
 	return -1
     }
diff --git a/gdb/testsuite/gdb.python/py-startup-opt.exp b/gdb/testsuite/gdb.python/py-startup-opt.exp
index ed2ba2d6b64..729b1c13cb9 100644
--- a/gdb/testsuite/gdb.python/py-startup-opt.exp
+++ b/gdb/testsuite/gdb.python/py-startup-opt.exp
@@ -97,11 +97,7 @@ proc test_python_settings { exp_state } {
     gdb_exit
 }
 
-save_vars { env(TERM) env(NO_COLOR) } {
-    # We need an ANSI-capable terminal to get the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
-
+with_ansi_styling_terminal {
     # Check the features are off by default.
     test_python_settings "off"
 
diff --git a/gdb/testsuite/gdb.rust/rust-style.exp b/gdb/testsuite/gdb.rust/rust-style.exp
index 0d0e78bbfdc..b4ea05ca8ff 100644
--- a/gdb/testsuite/gdb.rust/rust-style.exp
+++ b/gdb/testsuite/gdb.rust/rust-style.exp
@@ -19,11 +19,7 @@ load_lib rust-support.exp
 require allow_rust_tests
 require {can_compile rust}
 
-save_vars { env(TERM) env(NO_COLOR) } {
-    # We need an ANSI-capable terminal to get the output.
-    setenv TERM ansi
-    setenv NO_COLOR ""
-
+with_ansi_styling_terminal {
     standard_testfile .rs
     if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
 	     {debug rust}]} {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0a4f74d8000..de22da8d8a8 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9238,6 +9238,25 @@ proc with_override { name override body } {
     return $result
 }
 
+# Run BODY after setting the TERM environment variable to 'ansi', and
+# unsetting the NO_COLOR environment variable.
+proc with_ansi_styling_terminal { body } {
+    save_vars { ::env(TERM) ::env(NO_COLOR) } {
+	# Set environment variables to allow styling.
+	setenv TERM ansi
+	unset -nocomplain ::env(NO_COLOR)
+
+	set code [catch {uplevel 1 $body} result]
+    }
+
+    if {$code == 1} {
+	global errorInfo errorCode
+	return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+    } else {
+	return -code $code $result
+    }
+}
+
 # Setup tuiterm.exp environment.  To be used in test-cases instead of
 # "load_lib tuiterm.exp".  Calls initialization function and schedules
 # finalization function.
  

>>>>> "Eli" == Eli Zaretskii via Gdb-patches <gdb-patches@sourceware.org> writes:

Eli> Should we also support the --color command-line argument, e.g. to
Eli> countermand NO_COLOR in the environment, and also as an alternative to
Eli> the environment variable?

I tend to think it's not needed.  There are already several ways to
change it, including the simple 'NO_COLOR= gdb' -- an empty value is
explicitly ignored.

If someone wants to add it, though, I won't object.

Tom
  

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Tom Tromey <tom@tromey.com> writes:
>> I ran across this site:
>> 
>> https://no-color.org/
>> 
>> ... which lobbies for tools to recognize the NO_COLOR environment
>> variable and disable any terminal styling when it is seen.

Andrew> I've never understood the passionate hatred some folk have for coloured
Andrew> output!  But NO_COLOR sounds like a fairly reasonable request, so I'm in
Andrew> favour of adding this.  I did have some thoughts on the testing...

Me neither but it seems pretty simple to accommodate them.

>> -    if {$dumb_terminal} {
>> -	clean_restart $binfile
>> -    } else {
>> -	Term::clean_restart 24 80 $binfile
>> -	if {![Term::prepare_for_tui]} {
>> -	    unsupported "TUI not supported"
>> -	    return
>> +    global env
>> +    save_vars { env(TERM) } {
>> +	setenv TERM $terminal
>> +	if {$dumb_terminal} {
>> +	    clean_restart $binfile
>> +	} else {
>> +	    Term::clean_restart 24 80 $binfile
>> +	    if {![Term::prepare_for_tui]} {
>> +		unsupported "TUI not supported"
>> +		return
>> +	    }

Andrew> I'm not sure that this change is needed.

Something is needed here, because without this there are regressions.

Tom
  

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Tom Tromey <tom@tromey.com> writes:
>>> I ran across this site:
>>> 
>>> https://no-color.org/
>>> 
>>> ... which lobbies for tools to recognize the NO_COLOR environment
>>> variable and disable any terminal styling when it is seen.
>
> Andrew> I've never understood the passionate hatred some folk have for coloured
> Andrew> output!  But NO_COLOR sounds like a fairly reasonable request, so I'm in
> Andrew> favour of adding this.  I did have some thoughts on the testing...
>
> Me neither but it seems pretty simple to accommodate them.
>
>>> -    if {$dumb_terminal} {
>>> -	clean_restart $binfile
>>> -    } else {
>>> -	Term::clean_restart 24 80 $binfile
>>> -	if {![Term::prepare_for_tui]} {
>>> -	    unsupported "TUI not supported"
>>> -	    return
>>> +    global env
>>> +    save_vars { env(TERM) } {
>>> +	setenv TERM $terminal
>>> +	if {$dumb_terminal} {
>>> +	    clean_restart $binfile
>>> +	} else {
>>> +	    Term::clean_restart 24 80 $binfile
>>> +	    if {![Term::prepare_for_tui]} {
>>> +		unsupported "TUI not supported"
>>> +		return
>>> +	    }
>
> Andrew> I'm not sure that this change is needed.
>
> Something is needed here, because without this there are regressions.

OK, yeah, I see what I missed now.

Thanks,
Andrew
  

On Tue, 19 Sep 2023, Tom Tromey wrote:

> Andrew> I've never understood the passionate hatred some folk have for coloured
> Andrew> output!  But NO_COLOR sounds like a fairly reasonable request, so I'm in
> Andrew> favour of adding this.  I did have some thoughts on the testing...
> 
> Me neither but it seems pretty simple to accommodate them.

 I came across this message late, but I find it worth commenting anyway.

 I can't speak for other people, however in my case multicoloured output
distracts me and hurts my eyes.  As simple as that, and there is no hatred 
involved.  All the people are not the same and I appreciate that for some 
people multicoloured text may be easier to follow instead.

 Thank you for adding this feature.

  Maciej