[PATCH] Improve/fix the TUI's current source line highlight

[PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 8087 bytes --]

[trying again, this time with a tar.gz.  sourceware rejected the image/pngs
content type...]

With styling enabled, I think the way we display the TUI's
highlighted/current line is very ugly and distracting.  At least,
I can't seem to get used to it.  The problem in my view is that we
reverse foreground/background in colored text as well, leading to
a fuzzy rainbow of 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.

I've attached screenshots of before/after patch, with both
white-on-black (actually, linux-colors / grey-ish-on-black),
and black-on-white themes in my console (konsole).  Also attached
screenshots with styling disabled, so that you can see how the
after-patch versions look more like the unstyled output.

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).

WDYT?  

Eli, could you try this on Windows, see if it behaves as intended there?

gdb/ChangeLog:
yyyy-mm-dd  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.
---
 gdb/tui/tui-io.c        | 99 ++++++++++++++++++++++++++++++++++++++++++-------
 gdb/tui/tui-io.h        |  3 ++
 gdb/tui/tui-winsource.c |  4 +-
 gdb/ui-style.h          | 18 +++++++++
 4 files changed, 109 insertions(+), 15 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index d006e41cab..2a77db9715 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -267,6 +267,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 = true;
+
+/* 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.  */
 
@@ -291,21 +300,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);
@@ -351,9 +350,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 8165b5bdfc..34a24da292 100644
--- 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 7fd460bde3..310f605f8a 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -276,13 +276,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.  */
   wclrtoeol (win_info->generic.handle);
diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index 04a1d448ba..2a87fbe801 100644
--- 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
   {
-- 
2.14.4



[-- Attachment #2: tui-screenshots.tar.gz --]
[-- Type: application/gzip, Size: 180037 bytes --]

Re: [PATCH] Improve/fix the TUI's current source line highlight

[John Baldwin]

On 3/14/19 10:47 AM, Pedro Alves wrote:
> [trying again, this time with a tar.gz.  sourceware rejected the image/pngs
> content type...]
> 
> With styling enabled, I think the way we display the TUI's
> highlighted/current line is very ugly and distracting.  At least,
> I can't seem to get used to it.  The problem in my view is that we
> reverse foreground/background in colored text as well, leading to
> a fuzzy rainbow of 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.
> 
> I've attached screenshots of before/after patch, with both
> white-on-black (actually, linux-colors / grey-ish-on-black),
> and black-on-white themes in my console (konsole).  Also attached
> screenshots with styling disabled, so that you can see how the
> after-patch versions look more like the unstyled output.
> 
> 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).
> 
> WDYT?  

I prefer your version over the current approach.  I think we've decided
to forgo the idea, but if we had light and dark "themes" it would perhaps
make sense to use the other "theme" colors for the highlight bar.  Your
version is pretty close to that with the current single "theme".

-- 
John Baldwin

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

-- 
Thanks,
Pedro Alves
On 03/14/2019 07:55 PM, John Baldwin wrote:
> On 3/14/19 10:47 AM, Pedro Alves wrote:
>> [trying again, this time with a tar.gz.  sourceware rejected the image/pngs
>> content type...]
>>
>> With styling enabled, I think the way we display the TUI's
>> highlighted/current line is very ugly and distracting.  At least,
>> I can't seem to get used to it.  The problem in my view is that we
>> reverse foreground/background in colored text as well, leading to
>> a fuzzy rainbow of 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.
>>
>> I've attached screenshots of before/after patch, with both
>> white-on-black (actually, linux-colors / grey-ish-on-black),
>> and black-on-white themes in my console (konsole).  Also attached
>> screenshots with styling disabled, so that you can see how the
>> after-patch versions look more like the unstyled output.
>>
>> 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).
>>
>> WDYT?  
> 
> I prefer your version over the current approach.  I think we've decided
> to forgo the idea, but if we had light and dark "themes" it would perhaps
> make sense to use the other "theme" colors for the highlight bar.  Your
> version is pretty close to that with the current single "theme".

One advantage of this approach over a highlight bar of some determined
color, is that this saves one color.  I.e., it reduces need to worry
about the highlight bar color being the same as the color of something
else, in which case I think that something else would become invisible
when highlighted.  The foreground/background colors are pretty much
guaranteed to be quite apart contrast-wise, so I think it should work
fine whatever the theme.  At least it looks fine to me in all the themes
konsole comes with (konsole's theme dialog changes the theme on the
spot, and you can see the effect immediately, no need to restart gdb).

Thanks,
Pedro Alves

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Hannes Domani via gdb-patches]

Am Donnerstag, 14. März 2019, 18:47:56 MEZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: 

> WDYT?

The result looks very nice.


> +/* If true, we're highlighting the current source line in reverse
> +  video mode.  */
> +static bool reverse_mode_p = true;

Shouldn't this variable be initialized to false?
Weird things happen on my end with true (sometimes the cmd window contents are reversed).


Also, I had to add the following to make it work on windows
(but beware, I'm using pdcurses, not ncurses, and I don't know if ncurses
for windows needs this as well):

--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -369,6 +386,17 @@ apply_ansi_escape (WINDOW *w, const char *buf)

   if (reverse_mode_p)
     {
+#if defined(__MINGW32__)
+      if (style.get_foreground ().is_basic ()
+         && style.get_foreground ().get_value ()
+         == (ncurses_norm_attr & 15))
+       style.set_fg (ui_file_style::NONE);
+      if (style.get_background ().is_basic ()
+         && style.get_background ().get_value ()
+         == ((ncurses_norm_attr >> 4) & 15))
+       style.set_bg (ui_file_style::NONE);
+#endif
+
       /* 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.,

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

On 03/15/2019 04:38 PM, Hannes Domani via gdb-patches wrote:
> Am Donnerstag, 14. März 2019, 18:47:56 MEZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: 
> 
>> WDYT?
> 
> The result looks very nice.
> 
> 
>> +/* If true, we're highlighting the current source line in reverse
>> +  video mode.  */
>> +static bool reverse_mode_p = true;
> 
> Shouldn't this variable be initialized to false?
> Weird things happen on my end with true (sometimes the cmd window contents are reversed).
> 

Wow, of course.  I have no idea how I missed that.

> 
> Also, I had to add the following to make it work on windows
> (but beware, I'm using pdcurses, not ncurses, and I don't know if ncurses
> for windows needs this as well):

Thanks, but can you explain it?  It is not obvious to me.

Thanks,
Pedro Alves

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> This patch changes that to something that I find much more sensible --
Pedro> only reverse the default foreground/background colors, leave styled
Pedro> text colors alone.  If the foreground color is not the default (because the
Pedro> text was styled), leave the foreground color as is.

Pedro> I've attached screenshots of before/after patch

Finally we need screenshots for gdb!  I thought the day would never
come.

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

Pedro> WDYT?  

Looks much better, please check this in.

Tom

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Eli Zaretskii]

> Date: Fri, 15 Mar 2019 16:38:37 +0000 (UTC)
> From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> 
> Also, I had to add the following to make it work on windows
> (but beware, I'm using pdcurses, not ncurses, and I don't know if ncurses
> for windows needs this as well):

Ncurses doesn't need this.

> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -369,6 +386,17 @@ apply_ansi_escape (WINDOW *w, const char *buf)
> 
>    if (reverse_mode_p)
>      {
> +#if defined(__MINGW32__)
> +      if (style.get_foreground ().is_basic ()
> +         && style.get_foreground ().get_value ()
> +         == (ncurses_norm_attr & 15))
> +       style.set_fg (ui_file_style::NONE);
> +      if (style.get_background ().is_basic ()
> +         && style.get_background ().get_value ()
> +         == ((ncurses_norm_attr >> 4) & 15))
> +       style.set_bg (ui_file_style::NONE);
> +#endif
> +

Could you describe what happens with pdcurses if you don't make this
change?  It's strange that pdcurses cannot use explicit color
specification if the color is the default one.

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Hannes Domani via gdb-patches]

Am Sonntag, 17. März 2019, 17:07:46 MEZ hat Eli Zaretskii <eliz@gnu.org> Folgendes geschrieben: 
> > Date: Fri, 15 Mar 2019 16:38:37 +0000 (UTC)> > From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>> > > > Also, I had to add the following to make it work on windows> > (but beware, I'm using pdcurses, not ncurses, and I don't know if ncurses> > for windows needs this as well):>
> Ncurses doesn't need this.>
>
> > --- a/gdb/tui/tui-io.c> > +++ b/gdb/tui/tui-io.c> > @@ -369,6 +386,17 @@ apply_ansi_escape (WINDOW *w, const char *buf)> > > >    if (reverse_mode_p)> >      {> > +#if defined(__MINGW32__)> > +      if (style.get_foreground ().is_basic ()> > +         && style.get_foreground ().get_value ()> > +         == (ncurses_norm_attr & 15))> > +       style.set_fg (ui_file_style::NONE);> > +      if (style.get_background ().is_basic ()> > +         && style.get_background ().get_value ()> > +         == ((ncurses_norm_attr >> 4) & 15))> > +       style.set_bg (ui_file_style::NONE);> > +#endif> > +>
> Could you describe what happens with pdcurses if you don't make this> change?  It's strange that pdcurses cannot use explicit color> specification if the color is the default one.
Actually, you can disregard that.
Before you added that fix for windows, I found a different workaround,it was to set the "normal" color in esc.style to gray.
I forgot that I did that, and it backfired now.So I removed that again, and everything is fine now even without that above change.
Sorry for the noise.


RegardsHannes Domani

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Eli Zaretskii]

> Date: Sun, 17 Mar 2019 16:46:37 +0000 (UTC)
> From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> 
> Actually, you can disregard that.
> Before you added that fix for windows, I found a different workaround,it was to set the "normal" color in esc.style to gray.
> I forgot that I did that, and it backfired now.So I removed that again, and everything is fine now even without that above change.

Great, thanks for unlocking this mystery.

So Pedro, I think the way is clear for you to go ahead and push.
Thanks.

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

On 03/17/2019 04:46 PM, Hannes Domani via gdb-patches wrote:
> Am Sonntag, 17. März 2019, 17:07:46 MEZ hat Eli Zaretskii <eliz@gnu.org> Folgendes geschrieben: 
>>> Date: Fri, 15 Mar 2019 16:38:37 +0000 (UTC)> > From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>> > > > Also, I had to add the following to make it work on windows> > (but beware, I'm using pdcurses, not ncurses, and I don't know if ncurses> > for windows needs this as well):>
>> Ncurses doesn't need this.>
>>
>>> --- a/gdb/tui/tui-io.c> > +++ b/gdb/tui/tui-io.c> > @@ -369,6 +386,17 @@ apply_ansi_escape (WINDOW *w, const char *buf)> > > >    if (reverse_mode_p)> >      {> > +#if defined(__MINGW32__)> > +      if (style.get_foreground ().is_basic ()> > +         && style.get_foreground ().get_value ()> > +         == (ncurses_norm_attr & 15))> > +       style.set_fg (ui_file_style::NONE);> > +      if (style.get_background ().is_basic ()> > +         && style.get_background ().get_value ()> > +         == ((ncurses_norm_attr >> 4) & 15))> > +       style.set_bg (ui_file_style::NONE);> > +#endif> > +>
>> Could you describe what happens with pdcurses if you don't make this> change?  It's strange that pdcurses cannot use explicit color> specification if the color is the default one.
> Actually, you can disregard that.
> Before you added that fix for windows, I found a different workaround,it was to set the "normal" color in esc.style to gray.
> I forgot that I did that, and it backfired now.So I removed that again, and everything is fine now even without that above change.
> Sorry for the noise.

No worries, thanks for following through and for testing.
The patch is in master and 8.3 branch now.

Thanks,
Pedro Alves

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

On 03/17/2019 03:57 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 14 Mar 2019 17:35:57 +0000
>>
>> 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.
>>
>> I've attached screenshots of before/after patch, with both
>> white-on-black (actually, linux-colors / grey-ish-on-black),
>> and black-on-white themes in my console (konsole).  Also attached
>> screenshots with styling disabled, so that you can see how the
>> after-patch versions look more like the unstyled output.
>>
>> 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).
>>
>> WDYT?  
>>
>> Eli, could you try this on Windows, see if it behaves as intended there?
> 
> Tested on MS-Windows, works fine.  I think you should push this.

Great, thanks.  I've merged this to master and branch, with the bool fixed.

Pedro Alves

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Mar 2019 17:35:57 +0000
> 
> 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.
> 
> I've attached screenshots of before/after patch, with both
> white-on-black (actually, linux-colors / grey-ish-on-black),
> and black-on-white themes in my console (konsole).  Also attached
> screenshots with styling disabled, so that you can see how the
> after-patch versions look more like the unstyled output.
> 
> 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).
> 
> WDYT?  
> 
> Eli, could you try this on Windows, see if it behaves as intended there?

Tested on MS-Windows, works fine.  I think you should push this.

Thanks.

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Pedro Alves]

-- 
Thanks,
Pedro Alves
On 03/14/2019 06:32 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Thu, 14 Mar 2019 17:35:57 +0000
>>
>> With styling enabled, I think the way we display the TUI's
>> highlighted/current line is very ugly and distracting.  At least,
>> I can't seem to get used to it.  The problem in my view is that we
>> reverse foreground/background in colored text as well, leading to
>> a fuzzy rainbow of colors.
> 
> Yes, I've seen this is as well, and it caused me to raise a brow ;-)
> 
>> WDYT?  
> 
> I like the change, thanks.
> 
>> Eli, could you try this on Windows, see if it behaves as intended there?
> 
> I will, but did your changes also handle the problems with
> highlighting lines longer than the window-width?  

It does not, that's an independent issue.

> I posted a patch for
> that in https://www.sourceware.org/ml/gdb-patches/2019-03/msg00271.html.
> If your patches don't handle that problem, we will need to merge these
> two together.
Let me go take a look at it.  I admit to being confused with all those
patches under the same subject.

Thanks,
Pedro Alves

Re: [PATCH] Improve/fix the TUI's current source line highlight

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Mar 2019 17:35:57 +0000
> 
> With styling enabled, I think the way we display the TUI's
> highlighted/current line is very ugly and distracting.  At least,
> I can't seem to get used to it.  The problem in my view is that we
> reverse foreground/background in colored text as well, leading to
> a fuzzy rainbow of colors.

Yes, I've seen this is as well, and it caused me to raise a brow ;-)

> WDYT?  

I like the change, thanks.

> Eli, could you try this on Windows, see if it behaves as intended there?

I will, but did your changes also handle the problems with
highlighting lines longer than the window-width?  I posted a patch for
that in https://www.sourceware.org/ml/gdb-patches/2019-03/msg00271.html.
If your patches don't handle that problem, we will need to merge these
two together.