Re: [PATCH] Add "set style tui-current-position on|off" default to off

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves via Gdb-patches <gdb-patches@sourceware.org> writes:

> On 6/16/20 4:38 AM, Phi Debian via Gdb-patches wrote:
>> Hi Pedro,
>> I like the "set tui current-line-highlight reverse-mono" of your patch, it
>> remove the underline need, the current line is well readable.
>> 
>
> Great.  Here's a more complete patch, now with documentation updated.
>
> The option is now named:
>
>   "set style tui-current-position on|off"
>
> and the default is now off.
>
> From 854948d86fca90e894813ee08e349fec95c4d2ba Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 15 Jun 2020 19:35:10 +0100
> Subject: [PATCH] Add "set style tui-current-position on|off", default to off
>
> As discussed at:
>
>  https://sourceware.org/pipermail/gdb-patches/2020-June/169519.html
>
> this patch disables source code highlighting for the text highlighted
> by the TUI's current position indicator, and adds a command to enable
> it back.

I originally sent this message off-list by accident.  Resending it now
including the gdb-patches list.  Apologies to people seeing this for the
second time.

---

I'd be interested to see this patch merged.  I took a look through the
code changes, and it all looks good to me.

Attached below is a rebase of Pedro's patch.  I've gone through the docs
and comments to mention assembly highlighting, which has been added to
GDB since this patch was originally written.

I've left the author email as 'Pedro Alves <palves@redhat.com>' as this
reflects where/when the patch was written, but I can update this to
anything more suitable upon request.

Pedro (or anyone) - any objections if I merge this patch?

Thanks,
Andrew

---

commit c4c460cac282c17d9b0545ce117177da8cc56e2f
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Nov 1 16:45:30 2022 +0000

    gdb: add "set style tui-current-position on|off", default to off
    
    As discussed at:
    
     https://sourceware.org/pipermail/gdb-patches/2020-June/169519.html
    
    this patch disables source and assembly code highlighting for the
    text highlighted by the TUI's current position indicator, and adds a
    command to enable it back.

diff --git a/gdb/NEWS b/gdb/NEWS
index 0642d7637b8..3f31515297c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -73,6 +73,11 @@
   For both /r and /b GDB is now better at using whitespace in order to
   align the disassembled instruction text.
 
+* The TUI no longer styles the source and assembly code highlighted by
+  the current position indicator by default.  You can however
+  re-enable styling using the new "set style tui-current-position"
+  command.
+
 * New commands
 
 maintenance set ignore-prologue-end-flag on|off
@@ -117,6 +122,10 @@ set debug infcall on|off
 show debug infcall
   Print additional debug messages about inferior function calls.
 
+set style tui-current-position [on|off]
+  Whether to style the source and assembly code highlighted by the
+  TUI's current position indicator.  The default is off.
+
 * Changed commands
 
 document user-defined
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index abf685561fa..062347b27d0 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -296,8 +296,8 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
   return prefix_cmds;
 }
 
-static cmd_list_element *style_set_list;
-static cmd_list_element *style_show_list;
+cmd_list_element *style_set_list;
+cmd_list_element *style_show_list;
 
 /* The command list for 'set style disassembler'.  */
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ea66f4ee42d..f5f664fd168 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26602,6 +26602,15 @@
 @item show style sources
 Show the current state of source code styling.
 
+@item set style tui-current-position @samp{on|off}
+Enable or disable styling of the source and assembly code highlighted
+by the TUI's current position indicator.  The default is @samp{off}.
+@xref{TUI, ,@value{GDBN} Text User Interface}.
+
+@item show style tui-current-position
+Show whether the source and assembly code highlighted by the TUI's
+current position indicator is styled.
+
 @anchor{style_disassembler_enabled}
 @item set style disassembler enabled @samp{on|off}
 Enable or disable disassembler styling.  This affects whether
@@ -29163,8 +29172,12 @@
 when their values change.
 @end table
 
-The source and assembly windows show the current program position
-by highlighting the current line and marking it with a @samp{>} marker.
+The source and assembly windows show the current program position by
+highlighting the current line and marking it with a @samp{>} marker.
+By default, source and assembly code styling is disabled for the
+highlighted text, but you can enable it with the @code{set style
+tui-current-position on} command.  @xref{Output Styling}.
+
 Breakpoints are indicated with two markers.  The first marker
 indicates the breakpoint type:
 
diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
index a05c68e52c2..c508870a930 100644
--- a/gdb/gdbcmd.h
+++ b/gdb/gdbcmd.h
@@ -65,4 +65,8 @@ extern void print_command_line (struct command_line *, unsigned int,
 extern void print_command_lines (struct ui_out *,
 				 struct command_line *, unsigned int);
 
+/* Chains containing all defined "set/show style" subcommands.  */
+extern struct cmd_list_element *style_set_list;
+extern struct cmd_list_element *style_show_list;
+
 #endif /* !defined (GDBCMD_H) */
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index a30000ef626..5278c380fc4 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -369,6 +369,9 @@ apply_ansi_escape (WINDOW *w, const char *buf)
 
   if (reverse_mode_p)
     {
+      if (!style_tui_current_position)
+	return n_read;
+
       /* We want to reverse _only_ the default foreground/background
 	 colors.  If the foreground color is not the default (because
 	 the text was styled), we want to leave it as is.  If e.g.,
@@ -411,12 +414,18 @@ tui_set_reverse_mode (WINDOW *w, bool reverse)
   ui_file_style style = last_style;
 
   reverse_mode_p = reverse;
-  style.set_reverse (reverse);
 
   if (reverse)
     {
       reverse_save_bg = style.get_background ();
       reverse_save_fg = style.get_foreground ();
+
+      if (!style_tui_current_position)
+	{
+	  /* Switch to default style (reversed) while highlighting the
+	     current position.  */
+	  style = {};
+	}
     }
   else
     {
@@ -424,6 +433,8 @@ tui_set_reverse_mode (WINDOW *w, bool reverse)
       style.set_fg (reverse_save_fg);
     }
 
+  style.set_reverse (reverse);
+
   tui_apply_style (w, style);
 }
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 31b6606636a..e24763c0072 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -218,6 +218,30 @@ show_tui_border_kind (struct ui_file *file,
 	      value);
 }
 
+/* Implementation of the "set/show style tui-current-position" commands.  */
+
+bool style_tui_current_position = false;
+
+static void
+show_style_tui_current_position (ui_file *file,
+				 int from_tty,
+				 cmd_list_element *c,
+				 const char *value)
+{
+  gdb_printf (file, _("\
+Styling the text highlighted by the TUI's current position indicator is %s.\n"),
+		    value);
+}
+
+static void
+set_style_tui_current_position (const char *ignore, int from_tty,
+				cmd_list_element *c)
+{
+  if (TUI_SRC_WIN != nullptr)
+    TUI_SRC_WIN->refill ();
+  if (TUI_DISASM_WIN != nullptr)
+    TUI_DISASM_WIN->refill ();
+}
 
 /* Tui internal configuration variables.  These variables are updated
    by tui_update_variables to reflect the tui configuration
@@ -1195,6 +1219,19 @@ the line numbers and uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);
 
+  add_setshow_boolean_cmd ("tui-current-position", class_maintenance,
+			   &style_tui_current_position, _("\
+Set whether to style text highlighted by the TUI's current position indicator."),
+			   _("\
+Show whether to style text highlighted by the TUI's current position indicator."),
+			   _("\
+When enabled, the source and assembly code highlighted by the TUI's current\n\
+position indicator is styled."),
+			   set_style_tui_current_position,
+			   show_style_tui_current_position,
+			   &style_set_list,
+			   &style_show_list);
+
   tui_border_style.changed.attach (tui_rehighlight_all, "tui-win");
   tui_active_border_style.changed.attach (tui_rehighlight_all, "tui-win");
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index 9a92fa3a514..bdc6da034ef 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -51,4 +51,8 @@ struct cmd_list_element **tui_get_cmd_list (void);
 /* Whether compact source display should be used.  */
 extern bool compact_source;
 
+/* Whether to style the source and assembly code highlighted by the TUI's
+   current position indicator.  */
+extern bool style_tui_current_position;
+
 #endif /* TUI_TUI_WIN_H */