Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
To: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Cc: Tom Tromey <tom@tromey.com>
Subject: Re: [PATCH v2] Support the NO_COLOR environment variable
Date: Tue, 26 Sep 2023 14:50:33 +0100	[thread overview]
Message-ID: <8734z1xcuu.fsf@redhat.com> (raw)
In-Reply-To: <20230925195751.1120801-1-tom@tromey.com>

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.
>
> This patch implements this for gdb.
>
> Regression tested on x86-64 Fedora 38.
>
> Co-Authored-By: Andrew Burgess <aburgess@redhat.com>

LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew



> ---
>  gdb/NEWS                                      |  4 ++++
>  gdb/doc/gdb.texinfo                           |  3 +++
>  gdb/main.c                                    |  4 ++++
>  gdb/testsuite/gdb.base/early-init-file.exp    |  4 +---
>  gdb/testsuite/gdb.base/readline-ask.exp       |  2 ++
>  .../gdb.base/style-interp-exec-mi.exp         |  5 +----
>  gdb/testsuite/gdb.base/style-logging.exp      |  5 +----
>  gdb/testsuite/gdb.base/style.exp              | 11 +++++-----
>  .../build-id-no-debug-warning.exp             |  5 +----
>  gdb/testsuite/gdb.debuginfod/crc_mismatch.exp |  5 +----
>  gdb/testsuite/gdb.python/py-format-string.exp |  3 +--
>  .../gdb.python/py-source-styling.exp          |  4 +---
>  gdb/testsuite/gdb.python/py-startup-opt.exp   |  5 +----
>  gdb/testsuite/gdb.rust/rust-style.exp         |  5 +----
>  gdb/testsuite/gdb.tui/tui-layout.exp          | 18 +++++++++-------
>  gdb/testsuite/lib/gdb.exp                     | 21 ++++++++++++++++++-
>  gdb/testsuite/lib/tuiterm.exp                 |  3 ++-
>  17 files changed, 60 insertions(+), 47 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 957b5a938b0..bd01b1d21e4 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 f9c2f955ef6..c962222bac0 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27272,6 +27272,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.
> +
>  @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..6426da8dacf 100644
> --- a/gdb/testsuite/gdb.base/early-init-file.exp
> +++ b/gdb/testsuite/gdb.base/early-init-file.exp
> @@ -99,9 +99,7 @@ proc check_gdb_startups_up_quietly { message } {
>      }
>  }
>  
> -save_vars { env(TERM) } {
> -    # We need an ANSI-capable terminal to get the output.
> -    setenv TERM ansi
> +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/readline-ask.exp b/gdb/testsuite/gdb.base/readline-ask.exp
> index 51bf6424a02..ff33c83c422 100644
> --- a/gdb/testsuite/gdb.base/readline-ask.exp
> +++ b/gdb/testsuite/gdb.base/readline-ask.exp
> @@ -21,6 +21,8 @@ if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object {debug}]
>      return -1
>  }
>  
> +setenv TERM dumb
> +
>  # INPUTRC gets reset for the next testfile.
>  setenv INPUTRC $inputrc
>  clean_restart ${binfile}
> diff --git a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> index 9b2ed87513f..b99e767a034 100644
> --- a/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> +++ b/gdb/testsuite/gdb.base/style-interp-exec-mi.exp
> @@ -23,10 +23,7 @@
>  
>  standard_testfile
>  
> -save_vars { env(TERM) } {
> -    # We need an ANSI-capable terminal to get the output.
> -    setenv TERM ansi
> -
> +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 a156c39146f..c21c3321bde 100644
> --- a/gdb/testsuite/gdb.base/style-logging.exp
> +++ b/gdb/testsuite/gdb.base/style-logging.exp
> @@ -22,10 +22,7 @@ require {!is_remote host}
>  
>  standard_testfile style.c
>  
> -save_vars { env(TERM) } {
> -    # We need an ANSI-capable terminal to get the output.
> -    setenv TERM ansi
> -
> +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 60f909e2402..7ee1b44177f 100644
> --- a/gdb/testsuite/gdb.base/style.exp
> +++ b/gdb/testsuite/gdb.base/style.exp
> @@ -69,10 +69,7 @@ proc run_style_tests { } {
>      global testfile srcfile hex binfile
>      global currently_disabled_style decimal hex
>  
> -    save_vars { env(TERM) } {
> -	# We need an ANSI-capable terminal to get the output.
> -	setenv TERM ansi
> -
> +    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}
> @@ -319,9 +316,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 +425,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..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,10 +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) } {
> -	# We need an ANSI-capable terminal to get styled output.
> -	setenv TERM ansi
> -
> +    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 074028eadfd..e88c4b3c653 100644
> --- a/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> +++ b/gdb/testsuite/gdb.debuginfod/crc_mismatch.exp
> @@ -71,10 +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) } {
> -	    # We need an ANSI-capable terminal to get styled output.
> -	    setenv TERM ansi
> -
> +	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 55a37766bff..04d19e483d9 100644
> --- a/gdb/testsuite/gdb.python/py-format-string.exp
> +++ b/gdb/testsuite/gdb.python/py-format-string.exp
> @@ -1184,11 +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) } {
> +      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
>  	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..d3ae339661c 100644
> --- a/gdb/testsuite/gdb.python/py-source-styling.exp
> +++ b/gdb/testsuite/gdb.python/py-source-styling.exp
> @@ -20,13 +20,11 @@ load_lib gdb-python.exp
>  
>  standard_testfile
>  
> -save_vars { env(TERM) } {
> +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
> -
>      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..729b1c13cb9 100644
> --- a/gdb/testsuite/gdb.python/py-startup-opt.exp
> +++ b/gdb/testsuite/gdb.python/py-startup-opt.exp
> @@ -97,10 +97,7 @@ proc test_python_settings { exp_state } {
>      gdb_exit
>  }
>  
> -save_vars { env(TERM) } {
> -    # We need an ANSI-capable terminal to get the output.
> -    setenv TERM ansi
> -
> +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 0555ea4b8a4..b4ea05ca8ff 100644
> --- a/gdb/testsuite/gdb.rust/rust-style.exp
> +++ b/gdb/testsuite/gdb.rust/rust-style.exp
> @@ -19,10 +19,7 @@ load_lib rust-support.exp
>  require allow_rust_tests
>  require {can_compile rust}
>  
> -save_vars { env(TERM) } {
> -    # We need an ANSI-capable terminal to get the output.
> -    setenv TERM ansi
> -
> +with_ansi_styling_terminal {
>      standard_testfile .rs
>      if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
>  	     {debug rust}]} {
> 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
> +	    }
>  	}
>      }
>  
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 1b9179401c4..de22da8d8a8 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.
> @@ -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.
> 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


      parent reply	other threads:[~2023-09-26 13:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-25 19:57 Tom Tromey
2023-09-26  0:19 ` Kevin Buettner via Gdb-patches
2023-09-26  2:29 ` Eli Zaretskii via Gdb-patches
2023-09-26 13:50 ` Andrew Burgess via Gdb-patches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8734z1xcuu.fsf@redhat.com \
    --to=gdb-patches@sourceware.org \
    --cc=aburgess@redhat.com \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox