Improve/fix the TUI's current source line highlight

With styling enabled, I think the way we display the TUI's
highlighted/current line is very ugly and distracting.  The problem in
my view is that we reverse foreground/background in colored text as
well, leading to rainbow of background colors.

This patch changes that to something that I find much more sensible --
only reverse the default foreground/background colors, leave styled
text colors alone.  If the foreground color is not the default
(because the text was styled), leave the foreground color as is.  If
e.g., the terminal is fg=BLACK, and bg=WHITE, and the style wants to
print text in RED, reverse the background color (print in BLACK), but
still print the text in RED.

Note: The new ui_file_style::set_fg method isn't called set_foreground
instead, because set_foreground is a macro in /usr/lib/term.h (ncurses).

gdb/ChangeLog:
2019-03-18  Pedro Alves  <palves@redhat.com>

	* tui/tui-io.c (reverse_mode_p, reverse_save_bg, reverse_save_fg):
	New globals.
	(apply_style): New, factored out from ...
	(apply_ansi_escape): ... this.  Handle reverse video mode.
	(tui_set_reverse_mode): New function.
	* tui/tui-io.h (tui_set_reverse_mode): New declaration.
	* tui/tui-winsource.c (tui_show_source_line): Use
	tui_set_reverse_mode instead of setting A_STANDOUT.
	* ui-style.h (struct ui_file_style) <set_reverse, set_fg, set_bg>:
	New setter methods.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 75d82dbd3f25b2f4f55b45bdc5e74f718c3b7789..d23819dd2091b3c3d25211114dddbc03ee2b73ad 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2019-03-18  Pedro Alves  <palves@redhat.com>
+
+       * tui/tui-io.c (reverse_mode_p, reverse_save_bg, reverse_save_fg):
+       New globals.
+       (apply_style): New, factored out from ...
+       (apply_ansi_escape): ... this.  Handle reverse video mode.
+       (tui_set_reverse_mode): New function.
+       * tui/tui-io.h (tui_set_reverse_mode): New declaration.
+       * tui/tui-winsource.c (tui_show_source_line): Use
+       tui_set_reverse_mode instead of setting A_STANDOUT.
+       * ui-style.h (struct ui_file_style) <set_reverse, set_fg, set_bg>:
+       New setter methods.
+
 2019-03-18  Hannes Domani  <ssbssa@yahoo.de>
 
        * tui/tui-source.c (copy_source_line): Fix handling of 'column'.

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index ef1e88507aa58d6dd6322595b001463a067da41a..b9087737fe23464df5c439cd95c4713448507e65 100644 (file)
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -274,6 +274,15 @@ static int last_color_pair = -1;
 
 static ui_file_style last_style;
 
+/* If true, we're highlighting the current source line in reverse
+   video mode.  */
+static bool reverse_mode_p = false;
+
+/* The background/foreground colors before we entered reverse
+   mode.  */
+static ui_file_style::color reverse_save_bg (ui_file_style::NONE);
+static ui_file_style::color reverse_save_fg (ui_file_style::NONE);
+
 /* Given two colors, return their color pair index; making a new one
    if necessary.  */
 
@@ -298,21 +307,11 @@ get_color_pair (int fg, int bg)
   return it->second;
 }
 
-/* Apply an ANSI escape sequence from BUF to W.  BUF must start with
-   the ESC character.  If BUF does not start with an ANSI escape,
-   return 0.  Otherwise, apply the sequence if it is recognized, or
-   simply ignore it if not.  In this case, the number of bytes read
-   from BUF is returned.  */
+/* Apply STYLE to W.  */
 
-static size_t
-apply_ansi_escape (WINDOW *w, const char *buf)
+static void
+apply_style (WINDOW *w, ui_file_style style)
 {
-  ui_file_style style = last_style;
-  size_t n_read;
-
-  if (!style.parse (buf, &n_read))
-    return n_read;
-
   /* Reset.  */
   wattron (w, A_NORMAL);
   wattroff (w, A_BOLD);
@@ -368,9 +367,83 @@ apply_ansi_escape (WINDOW *w, const char *buf)
     wattron (w, A_REVERSE);
 
   last_style = style;
+}
+
+/* Apply an ANSI escape sequence from BUF to W.  BUF must start with
+   the ESC character.  If BUF does not start with an ANSI escape,
+   return 0.  Otherwise, apply the sequence if it is recognized, or
+   simply ignore it if not.  In this case, the number of bytes read
+   from BUF is returned.  */
+
+static size_t
+apply_ansi_escape (WINDOW *w, const char *buf)
+{
+  ui_file_style style = last_style;
+  size_t n_read;
+
+  if (!style.parse (buf, &n_read))
+    return n_read;
+
+  if (reverse_mode_p)
+    {
+      /* 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.,
+        the terminal is fg=BLACK, and bg=WHITE, and the style wants
+        to print text in RED, we want to reverse the background color
+        (print in BLACK), but still print the text in RED.  To do
+        that, we enable the A_REVERSE attribute, and re-reverse the
+        parsed-style's fb/bg colors.
+
+        Notes on the approach:
+
+         - there's no portable way to know which colors the default
+           fb/bg colors map to.
+
+         - this approach does the right thing even if you change the
+           terminal colors while GDB is running -- the reversed
+           colors automatically adapt.
+      */
+      if (!style.is_default ())
+       {
+         ui_file_style::color bg = style.get_background ();
+         ui_file_style::color fg = style.get_foreground ();
+         style.set_fg (bg);
+         style.set_bg (fg);
+       }
+
+      /* Enable A_REVERSE.  */
+      style.set_reverse (true);
+    }
+
+  apply_style (w, style);
   return n_read;
 }
 
+/* See tui.io.h.  */
+
+void
+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 ();
+    }
+  else
+    {
+      style.set_bg (reverse_save_bg);
+      style.set_fg (reverse_save_fg);
+    }
+
+  apply_style (w, style);
+}
+
 /* Print LENGTH characters from the buffer pointed to by BUF to the
    curses command window.  The output is buffered.  It is up to the
    caller to refresh the screen if necessary.  */

diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index 8165b5bdfc189f4f2205e83a9a788e8a6fd61ef5..34a24da292377d7df087d923527f0d46ae34cb3f 100644 (file)
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -48,6 +48,9 @@ extern void tui_redisplay_readline (void);
 /* Expand TABs into spaces.  */
 extern char *tui_expand_tabs (const char *, int);
 
+/* Enter/leave reverse video mode.  */
+extern void tui_set_reverse_mode (WINDOW *w, bool reverse);
+
 extern struct ui_out *tui_out;
 extern cli_ui_out *tui_old_uiout;
 

diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 9925b79dff11fced0f606daa2514e9dbd0626ad0..e31a0164cf267729a23434289f7acc49b0940206 100644 (file)
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -277,13 +277,13 @@ tui_show_source_line (struct tui_win_info *win_info, int lineno)
 
   line = win_info->generic.content[lineno - 1];
   if (line->which_element.source.is_exec_point)
-    wattron (win_info->generic.handle, A_STANDOUT);
+    tui_set_reverse_mode (win_info->generic.handle, true);
 
   wmove (win_info->generic.handle, lineno, 1);
   tui_puts (line->which_element.source.line,
            win_info->generic.handle);
   if (line->which_element.source.is_exec_point)
-    wattroff (win_info->generic.handle, A_STANDOUT);
+    tui_set_reverse_mode (win_info->generic.handle, false);
 
   /* Clear to end of line but stop before the border.  */
   x = getcurx (win_info->generic.handle);

diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index 04a1d448ba997c494edbc2ba400d81d9d50a8f66..2a87fbe8012d62393a420dc8a90f79ea3f6a1043 100644 (file)
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -180,18 +180,36 @@ struct ui_file_style
     return m_reverse;
   }
 
+  /* Set/clear the reverse display flag.  */
+  void set_reverse (bool reverse)
+  {
+    m_reverse = reverse;
+  }
+
   /* Return the foreground color of this style.  */
   const color &get_foreground () const
   {
     return m_foreground;
   }
 
+  /* Set the foreground color of this style.  */
+  void set_fg (color c)
+  {
+    m_foreground = c;
+  }
+
   /* Return the background color of this style.  */
   const color &get_background () const
   {
     return m_background;
   }
 
+  /* Set the background color of this style.  */
+  void set_bg (color c)
+  {
+    m_background = c;
+  }
+
   /* Return the intensity of this style.  */
   intensity get_intensity () const
   {