public inbox for binutils-cvs@sourceware.org
help / color / mirror / Atom feed
From: Nick Clifton <nickc@sourceware.org>
To: bfd-cvs@sourceware.org
Subject: [binutils-gdb] Default to enabling colored disassembly if output is to a terminal.
Date: Tue,  9 Aug 2022 13:58:23 +0000 (GMT)	[thread overview]
Message-ID: <20220809135823.4DF643856DE8@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a88c79b77036e4778e70d62081c3cfd1044bb8e3

commit a88c79b77036e4778e70d62081c3cfd1044bb8e3
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Aug 9 14:57:48 2022 +0100

    Default to enabling colored disassembly if output is to a terminal.
    
            PR 29457
            * objdump.c (disassembler_color): Change type to an enum.
            (disassembler_extended_color): Remove.
            (usage): Update.
            (objdump_color_for_assembler_style): Update.
            (main): Update initialisation of disassembler_color.  If not
            initialised via a command line option, set based upon terminal
            output.
            * doc/binutils.texi: Update description of disassmbler-color
            option.
            * testsuite/binutils-all/arc/objdump.exp: Add
            --disassembler-color=off option when disassembling.
            * testsuite/binutils-all/arm/objdump.exp: Likewise.

Diff:
---
 binutils/ChangeLog                              | 16 ++++++
 binutils/doc/binutils.texi                      |  3 ++
 binutils/objdump.c                              | 67 ++++++++++++++++++-------
 binutils/testsuite/binutils-all/arc/objdump.exp |  2 +-
 binutils/testsuite/binutils-all/arm/objdump.exp |  2 +-
 5 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 2bf250649f2..e911f44de7c 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,19 @@
+2022-08-09  Nick Clifton  <nickc@redhat.com>
+
+	PR 29457
+	* objdump.c (disassembler_color): Change type to an enum.
+	(disassembler_extended_color): Remove.
+	(usage): Update.
+	(objdump_color_for_assembler_style): Update.
+	(main): Update initialisation of disassembler_color.  If not
+	initialised via a command line option, set based upon terminal
+	output.
+	* doc/binutils.texi: Update description of disassmbler-color
+	option.
+	* testsuite/binutils-all/arc/objdump.exp: Add
+	--disassembler-color=off option when disassembling.
+	* testsuite/binutils-all/arm/objdump.exp: Likewise.
+
 2022-08-08  Nick Clifton  <nickc@redhat.com>
 
 	* README-how-to-make-a-release: Add a link to the NEWS files in
diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index 81be746a17b..41f38f479f6 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -2828,6 +2828,9 @@ If it is necessary to disable the @option{--disassembler-color} option
 after it has previously been enabled then use
 @option{--disassembler-color=off}.
 
+If this option is not specified then the default is to enable color
+output if displaying to a terminal, but not otherwise.
+
 @item -W[lLiaprmfFsoORtUuTgAckK]
 @itemx --dwarf[=rawline,=decodedline,=info,=abbrev,=pubnames,=aranges,=macro,=frames,=frames-interp,=str,=str-offsets,=loc,=Ranges,=pubtypes,=trace_info,=trace_abbrev,=trace_aranges,=gdb_index,=addr,=cu_index,=links,=follow-links]
 @include debug.options.texi
diff --git a/binutils/objdump.c b/binutils/objdump.c
index 4076587151c..08c335476ff 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -130,8 +130,14 @@ static bool visualize_jumps = false;	/* --visualize-jumps.  */
 static bool color_output = false;	/* --visualize-jumps=color.  */
 static bool extended_color_output = false; /* --visualize-jumps=extended-color.  */
 static int process_links = false;       /* --process-links.  */
-static bool disassembler_color = false; /* --disassembler-color=color.  */
-static bool disassembler_extended_color = false; /* --disassembler-color=extended-color.  */
+
+static enum color_selection
+  {
+    on_if_terminal_output,
+    on,   				/* --disassembler-color=color.  */
+    off, 				/* --disassembler-color=off.  */
+    extended				/* --disassembler-color=extended-color.  */
+  } disassembler_color = on_if_terminal_output;
 
 static int dump_any_debugging;
 static int demangle_flags = DMGL_ANSI | DMGL_PARAMS;
@@ -409,6 +415,8 @@ usage (FILE *stream, int status)
       --disassembler-color=off   Disable disassembler color output.\n\n"));
       fprintf (stream, _("\
       --disassembler-color=color Use basic colors in disassembler output.\n\n"));
+      fprintf (stream, _("\
+      --disassembler-color=extended-color Use 8-bit colors in disassembler output.\n\n"));
 
       list_supported_targets (program_name, stream);
       list_supported_architectures (program_name, stream);
@@ -2158,44 +2166,66 @@ objdump_color_for_disassembler_style (enum disassembler_style style)
   if (style == dis_style_comment_start)
     disassembler_in_comment = true;
 
-  if (disassembler_color)
+  if (disassembler_color == on)
     {
       if (disassembler_in_comment)
 	return color;
 
       switch (style)
 	{
-	case dis_style_symbol: color = 32; break;
+	case dis_style_symbol:
+	  color = 32;
+	  break;
         case dis_style_assembler_directive:
 	case dis_style_sub_mnemonic:
-	case dis_style_mnemonic: color = 33; break;
-	case dis_style_register: color = 34; break;
+	case dis_style_mnemonic:
+	  color = 33;
+	  break;
+	case dis_style_register:
+	  color = 34;
+	  break;
 	case dis_style_address:
         case dis_style_address_offset:
-	case dis_style_immediate: color = 35; break;
+	case dis_style_immediate:
+	  color = 35;
+	  break;
 	default:
-	case dis_style_text: color = -1; break;
+	case dis_style_text:
+	  color = -1;
+	  break;
 	}
     }
-  else if (disassembler_extended_color)
+  else if (disassembler_color == extended)
     {
       if (disassembler_in_comment)
 	return 250;
 
       switch (style)
 	{
-	case dis_style_symbol: color = 40; break;
+	case dis_style_symbol:
+	  color = 40;
+	  break;
         case dis_style_assembler_directive:
 	case dis_style_sub_mnemonic:
-	case dis_style_mnemonic: color = 142; break;
-	case dis_style_register: color = 27; break;
+	case dis_style_mnemonic:
+	  color = 142;
+	  break;
+	case dis_style_register:
+	  color = 27;
+	  break;
 	case dis_style_address:
         case dis_style_address_offset:
-	case dis_style_immediate: color = 134; break;
+	case dis_style_immediate:
+	  color = 134;
+	  break;
 	default:
-	case dis_style_text: color = -1; break;
+	case dis_style_text:
+	  color = -1;
+	  break;
 	}
     }
+  else if (disassembler_color != off)
+    bfd_fatal (_("disassembly color not correctly selected"));
 
   return color;
 }
@@ -5683,11 +5713,11 @@ main (int argc, char **argv)
 	  break;
 	case OPTION_DISASSEMBLER_COLOR:
 	  if (streq (optarg, "off"))
-	    disassembler_color = false;
+	    disassembler_color = off;
 	  else if (streq (optarg, "color"))
-	    disassembler_color = true;
+	    disassembler_color = on;
 	  else if (streq (optarg, "extended-color"))
-	    disassembler_extended_color = true;
+	    disassembler_color = extended;
 	  else
 	    nonfatal (_("unrecognized argument to --disassembler-color"));
 	  break;
@@ -5900,6 +5930,9 @@ main (int argc, char **argv)
 	}
     }
 
+  if (disassembler_color == on_if_terminal_output)
+    disassembler_color = isatty (1) ? on : off;
+
   if (show_version)
     print_version ("objdump");
 
diff --git a/binutils/testsuite/binutils-all/arc/objdump.exp b/binutils/testsuite/binutils-all/arc/objdump.exp
index fe698550d30..6ade0132a7e 100644
--- a/binutils/testsuite/binutils-all/arc/objdump.exp
+++ b/binutils/testsuite/binutils-all/arc/objdump.exp
@@ -56,7 +56,7 @@ proc check_assembly { testname objfile expected { disas_flags "" } } {
 	fail $testname
 	return
     }
-    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble $disas_flags \
+    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble --disassembler-color=off $disas_flags \
 	$objfile"]
 
     if [regexp $expected $got] then {
diff --git a/binutils/testsuite/binutils-all/arm/objdump.exp b/binutils/testsuite/binutils-all/arm/objdump.exp
index 059ef1e707c..9cd057e60f1 100644
--- a/binutils/testsuite/binutils-all/arm/objdump.exp
+++ b/binutils/testsuite/binutils-all/arm/objdump.exp
@@ -41,7 +41,7 @@ if {![binutils_assemble $srcdir/$subdir/thumb2-cond.s tmpdir/thumb2-cond.o]} the
 
     # Make sure that conditional instructions are correctly decoded.
 
-    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble --start-address=6 $objfile"]
+    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble --disassembler-color=off --start-address=6 $objfile"]
 
     set want "bcc.w\[ \t\]*e12.*bx\[ \t\]*lr"


                 reply	other threads:[~2022-08-09 13:58 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20220809135823.4DF643856DE8@sourceware.org \
    --to=nickc@sourceware.org \
    --cc=bfd-cvs@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).