[PATCH] Style more output of "disassemble" command

[PATCH] Style more output of "disassemble" command

[Tom Tromey]

I noticed a couple of spots where the "disassemble" could style its
output, but currently does not.  This patch adds styling to the
function name at the start of the disassembly, and any addresses
printed there.

gdb/ChangeLog
2020-08-04  Tom Tromey  <tom@tromey.com>

	* cli/cli-cmds.c (print_disassembly): Style function name and
	addresses.

gdb/testsuite/ChangeLog
2020-08-04  Tom Tromey  <tom@tromey.com>

	* gdb.base/style.exp: Check that "main"'s name is styled.
---
 gdb/ChangeLog                    |  5 +++++
 gdb/cli/cli-cmds.c               | 18 ++++++++++++------
 gdb/testsuite/ChangeLog          |  4 ++++
 gdb/testsuite/gdb.base/style.exp |  5 ++++-
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index e3965fea076..b70da4a0145 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1397,12 +1397,16 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
     {
       printf_filtered ("Dump of assembler code ");
       if (name != NULL)
-	printf_filtered ("for function %s:\n", name);
+	printf_filtered ("for function %ps:\n",
+			 styled_string (function_name_style.style (), name));
       if (block == nullptr || BLOCK_CONTIGUOUS_P (block))
         {
 	  if (name == NULL)
-	    printf_filtered ("from %s to %s:\n",
-			     paddress (gdbarch, low), paddress (gdbarch, high));
+	    printf_filtered ("from %ps to %ps:\n",
+			     styled_string (address_style.style (),
+					    paddress (gdbarch, low)),
+			     styled_string (address_style.style (),
+					    paddress (gdbarch, high)));
 
 	  /* Dump the specified range.  */
 	  gdb_disassembly (gdbarch, current_uiout, flags, -1, low, high);
@@ -1413,9 +1417,11 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
 	    {
 	      CORE_ADDR range_low = BLOCK_RANGE_START (block, i);
 	      CORE_ADDR range_high = BLOCK_RANGE_END (block, i);
-	      printf_filtered (_("Address range %s to %s:\n"),
-			       paddress (gdbarch, range_low),
-			       paddress (gdbarch, range_high));
+	      printf_filtered (_("Address range %ps to %ps:\n"),
+			       styled_string (address_style.style (),
+					      paddress (gdbarch, range_low)),
+			       styled_string (address_style.style (),
+					      paddress (gdbarch, range_high)));
 	      gdb_disassembly (gdbarch, current_uiout, flags, -1,
 			       range_low, range_high);
 	    }
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index bfd26144fa4..ef9f7c80a5a 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -91,9 +91,12 @@ save_vars { env(TERM) } {
 	    "Defined at $base_file_expr:$macro_line\r\n#define SOME_MACRO 23"
     }
 
+    set main [style main function]
     set func [style some_called_function function]
     # Somewhere should see the call to the function.
-    gdb_test "disassemble main" "[style $hex address].*$func.*"
+    gdb_test "disassemble main" \
+	[concat "Dump of assembler code for function $main:.*" \
+	     "[style $hex address].*$func.*"]
 
     set ifield [style int_field variable]
     set sfield [style string_field variable]
-- 
2.17.2

Re: [PATCH] Style more output of "disassemble" command

[Gary Benson]

Tom Tromey wrote:
> I noticed a couple of spots where the "disassemble" could style its
> output, but currently does not.  This patch adds styling to the
> function name at the start of the disassembly, and any addresses
> printed there.
...
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index e3965fea076..b70da4a0145 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1397,12 +1397,16 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
>      {
>        printf_filtered ("Dump of assembler code ");
>        if (name != NULL)
> -	printf_filtered ("for function %s:\n", name);
> +	printf_filtered ("for function %ps:\n",
> +			 styled_string (function_name_style.style (), name));
>        if (block == nullptr || BLOCK_CONTIGUOUS_P (block))
>          {
>  	  if (name == NULL)
> -	    printf_filtered ("from %s to %s:\n",
> -			     paddress (gdbarch, low), paddress (gdbarch, high));
> +	    printf_filtered ("from %ps to %ps:\n",
> +			     styled_string (address_style.style (),
> +					    paddress (gdbarch, low)),
> +			     styled_string (address_style.style (),
> +					    paddress (gdbarch, high)));
>  
>  	  /* Dump the specified range.  */
>  	  gdb_disassembly (gdbarch, current_uiout, flags, -1, low, high);
> @@ -1413,9 +1417,11 @@ print_disassembly (struct gdbarch *gdbarch, const char *name,
>  	    {
>  	      CORE_ADDR range_low = BLOCK_RANGE_START (block, i);
>  	      CORE_ADDR range_high = BLOCK_RANGE_END (block, i);
> -	      printf_filtered (_("Address range %s to %s:\n"),
> -			       paddress (gdbarch, range_low),
> -			       paddress (gdbarch, range_high));
> +	      printf_filtered (_("Address range %ps to %ps:\n"),
> +			       styled_string (address_style.style (),
> +					      paddress (gdbarch, range_low)),
> +			       styled_string (address_style.style (),
> +					      paddress (gdbarch, range_high)));
>  	      gdb_disassembly (gdbarch, current_uiout, flags, -1,
>  			       range_low, range_high);
>  	    }

Your patch looks good, but, as a consistency thing, does GDB have a
position w.r.t. gettext macros?  Both these hunks are in the same
function, but only the second hunk's printf_filtered calls have _().

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

Re: [PATCH] Style more output of "disassemble" command

[Tom Tromey]

>>>>> "Gary" == Gary Benson via Gdb-patches <gdb-patches@sourceware.org> writes:

>> printf_filtered ("Dump of assembler code ");
>> if (name != NULL)
>> -	printf_filtered ("for function %s:\n", name);
>> +	printf_filtered ("for function %ps:\n",
>> +			 styled_string (function_name_style.style (), name));
[...]

Gary> Your patch looks good, but, as a consistency thing, does GDB have a
Gary> position w.r.t. gettext macros?  Both these hunks are in the same
Gary> function, but only the second hunk's printf_filtered calls have _().

Thanks for noticing that.  Yeah, those should have _() wrappers.
Probably the function really needs to be restructured a bit to be
i18n-friendly -- breaking up sentences like this isn't good.
I'm just going to add the _() though.

I'll check this in shortly.

Tom