TUI enhancement suggestion.

TUI enhancement suggestion.

[Phi Debian]

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

Hi All,

I patched the GDB TUI to fit my needs, I post here 2 pictures to show it.
If you think it can be useful I can submit the patch.

In a nutshell the reverse video for 'point line' is not suited for my color
'theme' I prefere a tiny underline that is more readable in my case.

I you think it is worth it, I could post the patch, else I keep it for
myself (as usual) :)

Cheers,
Phi

[-- Attachment #2: gdb-1.png --]
[-- Type: image/png, Size: 13220 bytes --]

[-- Attachment #3: gdb-2.png --]
[-- Type: image/png, Size: 12478 bytes --]

Re: TUI enhancement suggestion.

[Pedro Alves]

On 6/9/20 8:55 AM, Phi Debian via Gdb-patches wrote:
> Hi All,
> 
> I patched the GDB TUI to fit my needs, I post here 2 pictures to show it.
> If you think it can be useful I can submit the patch.
> 
> In a nutshell the reverse video for 'point line' is not suited for my color
> 'theme' I prefere a tiny underline that is more readable in my case.
> 
> I you think it is worth it, I could post the patch, else I keep it for
> myself (as usual) :)

I don't object to the feature, however, I'd like to point out that I
am surprised that you're getting that yellow background color behind
"printf".  How is that happening?  It looks like a bug offhand, to me.

See the screenshots attached to this:

 https://sourceware.org/legacy-ml/gdb-patches/2019-03/msg00295.html

Particularly, the "after" ones.

Thanks,
Pedro Alves

Re: TUI enhancement suggestion.

[Phi Debian]

Well, I don't mange or setup anything regarding the reverse video.

I use mate (gnome2) on debian, and on some HW I use ubuntu HWE, I use
mainly xterm, but may end up using  terminator, then the VTE.

My terminal basic setup is a darkgreen background, on lite green
foreground, for my day to day work so basically xterm is spawned with

-bg \#003000 -fg green

Then color enter the dance, depending on soft, (gdb vs less vs git  grep
make, name it) things varies but long story short gdb only use the first 8
colors of the colormap (by name) so I use a mapping like this for xterm,
VTE.
c=(
rgb:8888/8888/8888    # black
rgb:ffff/8888/8888    # red
rgb:0000/cdcd/0000    # green
rgb:cdcd/cdcd/0000    # yellow
rgb:8888/8888/ffff    # blue
rgb:ffff/8888/ffff    # magenta
rgb:8888/ffff/ffff    # cyan
rgb:ffff/ffff/ffff    # white
)

So gdb look nice, and the syntax highlight is nice too  (BTW thanx to whom
implemented the syntax highlight, I mean source highlite)

For git or other, we can setup the color we want in the 256 color palette,
so it is easier.

I doubt this setup screw up the reverse video... but If you need me to
tweak thing to test color combinations, let me know.

I can provide a patch built on the lates 9.2 pull if you want to look the
underline on the point line, I find it aesthetical.
I implemented it in a way that extend the 'attribute' not limiting to DIM,
BOLD, and when placed only on the leading white space of the source line it
is not too intrusive.

Lemme know

Cheers,
Phi

Re: TUI enhancement suggestion.

[Phi Debian]

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

Here is the patch I made, I spotted a little typo in a comment, I leave it
in there for now, and provide this as a demonstrator.

I never submitted a patch so I don't really know the best practice, I
cutted it on top of 9.2 (latest pull) then the patch)
I used

$ git format-patch -1 HEAD

Cheers,
Phi

[-- Attachment #2: 0001-Implement-point-underline.patch --]
[-- Type: text/x-patch, Size: 5680 bytes --]

From 395ae414f395d58bd07a44077d4586512924d451 Mon Sep 17 00:00:00 2001
From: Phi <phi@kernel-tools.com>
Date: Tue, 9 Jun 2020 11:59:47 +0200
Subject: [PATCH] Implement point underline

---
 gdb/tui/tui-io.c        | 18 ++++++++++++++++++
 gdb/tui/tui-io.h        |  3 +++
 gdb/tui/tui-win.c       | 31 +++++++++++++++++++++++++++++++
 gdb/tui/tui-win.h       |  3 +++
 gdb/tui/tui-winsource.c |  4 ++--
 gdb/ui-style.h          | 15 ++++++++++++++-
 6 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e7a8ac77bc..7dcc76bab8 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -294,6 +294,7 @@ tui_apply_style (WINDOW *w, ui_file_style style)
   /* Reset.  */
   wattron (w, A_NORMAL);
   wattroff (w, A_BOLD);
+  wattroff (w, A_UNDERLINE);
   wattroff (w, A_DIM);
   wattroff (w, A_REVERSE);
   if (last_color_pair != -1)
@@ -330,6 +331,10 @@ tui_apply_style (WINDOW *w, ui_file_style style)
     case ui_file_style::NORMAL:
       break;
 
+    case ui_file_style::UNDERLINE:
+      wattron (w, A_UNDERLINE); 
+      break;
+
     case ui_file_style::BOLD:
       wattron (w, A_BOLD);
       break;
@@ -423,6 +428,19 @@ tui_set_reverse_mode (WINDOW *w, bool reverse)
   tui_apply_style (w, style);
 }
 
+
+void
+tui_set_point_mode (WINDOW *w, bool mode)
+{ ui_file_style style = last_style;
+  if(point_underline==0)
+  { tui_set_reverse_mode(w,mode);
+  }
+  if(point_underline==1)
+  { style.set_underline(mode);
+    tui_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 f28cf4e12d..8aa81b43d8 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -51,6 +51,9 @@ extern gdb::unique_xmalloc_ptr<char> tui_expand_tabs (const char *);
 /* Enter/leave reverse video mode.  */
 extern void tui_set_reverse_mode (WINDOW *w, bool reverse);
 
+/* Enter/leave point (exec) video mode.  */
+extern void tui_set_point_mode (WINDOW *w, bool mode);
+
 /* Apply STYLE to the window.  */
 extern void tui_apply_style (WINDOW *w, ui_file_style style);
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index a78837fe68..54336e1337 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -823,6 +823,29 @@ tui_show_compact_source (struct ui_file *file, int from_tty,
   printf_filtered (_("TUI source window compactness is %s.\n"), value);
 }
 
+
+bool point_underline = false;
+
+/* Callback for "set tui point-underline".  */
+
+static void
+tui_set_point_underline (const char *ignore, int from_tty,
+			struct cmd_list_element *c)
+{
+  if (TUI_SRC_WIN != nullptr)
+    TUI_SRC_WIN->refill ();
+}
+
+/* Callback for "show tui compact-source".  */
+
+static void
+tui_show_point_underline (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  printf_filtered (_("TUI source point underline is %s.\n"), value);
+}
+
+
 /* Set the tab width of the specified window.  */
 static void
 tui_set_tab_width_command (const char *arg, int from_tty)
@@ -1120,6 +1143,14 @@ the line numbers and uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);
 
+  add_setshow_boolean_cmd ("point-underline", class_tui,
+			   &point_underline, _("\
+Set whether the TUI source point line is underline."), _("\
+Show whether the TUI source point line is underline."), _("\
+This variable controls whether the TUI source point line is underlined.\n"),
+			   tui_set_point_underline, tui_show_point_underline,
+			   &tui_setlist, &tui_showlist);  
+
   tui_border_style.changed.attach (tui_rehighlight_all);
   tui_active_border_style.changed.attach (tui_rehighlight_all);
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index e379184630..d5fbfc5fea 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -57,4 +57,7 @@ struct cmd_list_element **tui_get_cmd_list (void);
 /* Whether compact source display should be used.  */
 extern bool compact_source;
 
+/* Whether underline point line should be used.  */
+extern bool point_underline;
+
 #endif /* TUI_TUI_WIN_H */
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index db0add9968..c9f493292a 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -258,12 +258,12 @@ tui_source_window_base::show_source_line (int lineno)
 
   line = &m_content[lineno - 1];
   if (line->is_exec_point)
-    tui_set_reverse_mode (handle.get (), true);
+    tui_set_point_mode (handle.get (), true);
 
   wmove (handle.get (), lineno, TUI_EXECINFO_SIZE);
   tui_puts (line->line.c_str (), handle.get ());
   if (line->is_exec_point)
-    tui_set_reverse_mode (handle.get (), false);
+    tui_set_point_mode (handle.get (), false);
 
   /* Clear to end of line but stop before the border.  */
   x = getcurx (handle.get ());
diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index 48ab52d5ea..5566f38c2f 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -135,7 +135,8 @@ struct ui_file_style
   {
     NORMAL = 0,
     BOLD,
-    DIM
+    DIM,
+    UNDERLINE
   };
 
   ui_file_style () = default;
@@ -210,6 +211,18 @@ struct ui_file_style
     m_background = c;
   }
 
+  /* Set the intensity of this style to bold  */
+  void set_bold (bool mode) 
+  { 
+    m_intensity=mode?BOLD:NORMAL;
+  }
+
+  /* Set the intensity of this style to bold  */
+  void set_underline (bool mode) 
+  { 
+    m_intensity=mode?UNDERLINE:NORMAL;
+  }
+
   /* Return the intensity of this style.  */
   intensity get_intensity () const
   {
-- 
2.25.1

Re: TUI enhancement suggestion.

[Phi Debian]

On Wednesday, June 10, 2020, Pedro Alves <palves@redhat.com> wrote:

> Hi Phi,
>
> It would be much better to keep the discussion on the
> mailing list, so that others could interact as well.
>
> Thanks,
> Pedro Alves
>
>
Ok. Point line in src win along with the nice syntax highlight produce
inconsistent background color as Pedro notified in the pics provided. In
the example there is a strange yellow background on function names, it
seems that the syntax highlight use a BOLD attribute. On basic xterm and
vte, this BOLD goes unnoticed. When one use xterm resource colorBD see
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html resource also
implemented in vte, resource doing the  mapping a color to the BOLD
attribute (in the example yellow) it produces this background  color change.

Dunno if that expected.

I'd like to be able to got more control (config) on how point line is
displayed, in my case using underline attribute for point line would be
lighter then reverse video line and would preserve (simplify) the syntax
highlight.

If someones as idea would be interesting to share.

Cheers,
Phi

Re: TUI enhancement suggestion.

[Pedro Alves]

On 6/10/20 9:25 PM, Phi Debian wrote:
> 
> 
> On Wednesday, June 10, 2020, Pedro Alves <palves@redhat.com <mailto:palves@redhat.com>> wrote:
> 
>     Hi Phi,
> 
>     It would be much better to keep the discussion on the
>     mailing list, so that others could interact as well.
> 
>     Thanks,
>     Pedro Alves
> 
> 
> Ok. Point line in src win along with the nice syntax highlight produce inconsistent background color as Pedro notified in the pics provided. In the example there is a strange yellow background on function names, it seems that the syntax highlight use a BOLD attribute. On basic xterm and vte, this BOLD goes unnoticed. When one use xterm resource colorBD see https://invisible-island.net/xterm/ctlseqs/ctlseqs.html resource also implemented in vte, resource doing the  mapping a color to the BOLD attribute (in the example yellow) it produces this background  color change.
> 
> Dunno if that expected.

I don't know much about these color mappings, but to me it looks like a bug offhand that
requesting "bold" affects the background color.  It would think that "bold" makes the
foreground text... bold.

> 
> I'd like to be able to got more control (config) on how point line is displayed, in my case using underline attribute for point line would be lighter then reverse video line and would preserve (simplify) the syntax highlight. 

If you like it better that way, then maybe others do too.  I don't mind the
new option.

I also wouldn't mind an option to specify a different background color
for the current source line highlight.

> 
> If someones as idea would be interesting to share.
> 
> Cheers,
> Phi 

Thanks,
Pedro Alves

Re: TUI enhancement suggestion.

[Pedro Alves]

On 6/9/20 4:03 PM, Phi Debian via Gdb-patches wrote:
> Here is the patch I made, I spotted a little typo in a comment, I leave it
> in there for now, and provide this as a demonstrator.
> 

Cool, thanks.  Patches should be sent against current master, because
new features are not going to be merged to the gdb 9 branch, only bugfixes.

> I never submitted a patch so I don't really know the best practice, I
> cutted it on top of 9.2 (latest pull) then the patch)
> I used
> 
> $ git format-patch -1 HEAD
> 

That's good.  The best way to send a patch is to then use 
  git send-email 0001-whatever.patch
but attaching it as you've done is also OK.

Take a look here:

  https://sourceware.org/gdb/wiki/ContributionChecklist

We will need to clear the copyright assignment issue:
  https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
in order to be able to accept your contribution.  Let me know and
I'll get your started.

As for the patch itself, how about, instead of:

  set tui point-underline on|off

we make it an enum command like:

  set tui current-line-highlight none|reverse|underline

An enum like this leaves it open for other styles in the future.
"none" could be to just display the ">" on the left side
and nothing else, like currently still done on the assembly
window.  

Also, I'm suggesting

  set tui current-line-highlight

and not

  set tui current-source-line-highlight

because it seems like a bug to me that the assembly window
doesn't highlight the current instruction in the same fashion
as the current source window highlight the current source line.

In your screenshot, the underline only goes up until the "printf",
instead of underlining the actual source code text in the line as well.
That seems like a bug.  What happens if you disable styling, with
"set style enabled off"?  Does that cause the whole text line to
be underlined?

Thanks,
Pedro Alves

Re: TUI enhancement suggestion.

[Phi Debian]

Ok I got the big picture, the little patch I made was more a demonstrator
than a final patch, I don't know the review process.

As soon as I can I redo one, with our remarks.

Regarding the underline going up to printf I made it on purpose as I find
it less intrusive, underlinie space is simple, while underlining text
(source) can render less readable again (like colors), so it is something I
can spot quickly but not too intrusive. I use very tall xterm, spotting the
underline in the space part is very easy easier thanjust the little >
before the line number.. That was the reasoning... Yet I admit, when there
is no styling it should works too so I will look again :)

I can't work too much on this unfortunately so it may be a while since I
come black to this.

Cheers

Re: TUI enhancement suggestion.

[Pedro Alves]

On 6/11/20 2:55 PM, Phi Debian via Gdb-patches wrote:
> Ok I got the big picture, the little patch I made was more a demonstrator
> than a final patch, I don't know the review process.
> 
> As soon as I can I redo one, with our remarks.
> 
> Regarding the underline going up to printf I made it on purpose as I find
> it less intrusive, underlinie space is simple, while underlining text
> (source) can render less readable again (like colors), so it is something I
> can spot quickly but not too intrusive. I use very tall xterm, spotting the
> underline in the space part is very easy easier thanjust the little >
> before the line number.. That was the reasoning... Yet I admit, when there
> is no styling it should works too so I will look again :)

Note that people are working on adding support for range stepping to gdb:

 https://sourceware.org/pipermail/gdb-patches/2020-May/168673.html

I can see that evolving such that the TUI would highlight the part of the
line that corresponds to the current statement, instead of the whole line.

Like:

      printf ("foo); ++i;
      ^^^^^^^^^^^^^

And after a statement-step:

      printf ("foo); ++i;
                     ^^^^

So I think that it is better if both the reverse-video and the underline
highlight methods highlight the exact same characters.

Then, we could have a separate setting to pick between highlighting
the whole line including the whitespace on the left, as we do
currently:

      printf ("foo); ++i;
^^^^^^^^^^^^^^^^^^^^^^^^^

and highlighting the current line's text only, no highlight on the left
empty space.  This is similar to what e.g., Visual Studio highlights
(https://www.atlascode.com/wp-content/uploads/2017/04/stepintospecific-original.gif),
for example:

      printf ("foo); ++i;
      ^^^^^^^^^^^^^^^^^^^

and highlighting just the left of the current line:

      printf ("foo); ++i;
^^^^^^

like you are suggesting.  

But that would be orthogonal to reverse vs underline.  I.e., we would
have a setting for "how do highlight" vs a setting for "what to highlight".

> I can't work too much on this unfortunately so it may be a while since I
> come black to this.
> 
Thanks,
Pedro Alves

Re: TUI enhancement suggestion.

[Hannes Domani]

 Am Montag, 15. Juni 2020, 16:21:11 MESZ hat Pedro Alves via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> On 6/11/20 2:55 PM, Phi Debian via Gdb-patches wrote:
> > Ok I got the big picture, the little patch I made was more a demonstrator
> > than a final patch, I don't know the review process.
> >
> > As soon as I can I redo one, with our remarks.
> >
> > Regarding the underline going up to printf I made it on purpose as I find
> > it less intrusive, underlinie space is simple, while underlining text
> > (source) can render less readable again (like colors), so it is something I
> > can spot quickly but not too intrusive. I use very tall xterm, spotting the
> > underline in the space part is very easy easier thanjust the little >
> > before the line number.. That was the reasoning... Yet I admit, when there
> > is no styling it should works too so I will look again :)
>
> Note that people are working on adding support for range stepping to gdb:
>
> https://sourceware.org/pipermail/gdb-patches/2020-May/168673.html
>
> I can see that evolving such that the TUI would highlight the part of the
> line that corresponds to the current statement, instead of the whole line.
>
> Like:
>
>       printf ("foo); ++i;
>       ^^^^^^^^^^^^^
>
> And after a statement-step:
>
>       printf ("foo); ++i;
>                     ^^^^
>
> So I think that it is better if both the reverse-video and the underline
> highlight methods highlight the exact same characters.
>
> Then, we could have a separate setting to pick between highlighting
> the whole line including the whitespace on the left, as we do
> currently:
>
>       printf ("foo); ++i;
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> and highlighting the current line's text only, no highlight on the left
> empty space.  This is similar to what e.g., Visual Studio highlights
> (https://www.atlascode.com/wp-content/uploads/2017/04/stepintospecific-original.gif),
> for example:
>
>       printf ("foo); ++i;
>       ^^^^^^^^^^^^^^^^^^^
>
> and highlighting just the left of the current line:
>
>       printf ("foo); ++i;
> ^^^^^^
>
> like you are suggesting.
>
> But that would be orthogonal to reverse vs underline.  I.e., we would
> have a setting for "how do highlight" vs a setting for "what to highlight".
>
>
> > I can't work too much on this unfortunately so it may be a while since I
> > come black to this.

In my range stepping patchset I "highlight" the current column position in the
TUI as well, but since no end-column information is available, it's only
1 character wide.

And the highlighting is done by simply disabling reverse-video for that
1 character, it was the simplest I could think of.


Hannes

Re: TUI enhancement suggestion.

[Phi Debian]

Hi Pedro and all,

I am really sorry but I lost my bandwith, I am starting a new job tomorow
leaving little room for investigation etc.

Regarding the point line, used to be inverse video on monochrome, i thought
it was kind of bizare with highlighted text because we don't have a uniform
background color (before patch) then an attempt was made to have a uniform
background  color but then it monopolized one color that can't be used with
foreground. The underline suffer the same problem as the inverse video, I.e
its color change as we highlight text. That's why the little demo only
underlined the front part of the line, that could be be reverse video too
but just the whitespace on the front of the line are by definition
monochrome.  I reckon my demo is bugged as you noticed, when disabling
syntax highlight then my underline goes all the way through the point line.

May be an idea could be set the whole point line with un-highlited,
monochrome,  then with the new range thing use reverse or underline for the
sub part only

My little underline on leading whitespace was just a way to find the >
quicker as I got long win on 4k display with 100++ Line so spotting the
point line quickly is a plus.

I'll try my best to review try a monochrome highlight to see the visual
aspect

Anyway keep going the TUI it is real good.

Re: TUI enhancement suggestion.

[Pedro Alves]

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

On 6/15/20 5:56 PM, Phi Debian via Gdb-patches wrote:
> Hi Pedro and all,
> 
> I am really sorry but I lost my bandwith, I am starting a new job tomorow
> leaving little room for investigation etc.
> 
> Regarding the point line, used to be inverse video on monochrome, i thought
> it was kind of bizare with highlighted text because we don't have a uniform
> background color (before patch) then an attempt was made to have a uniform
> background  color but then it monopolized one color that can't be used with
> foreground. The underline suffer the same problem as the inverse video, I.e
> its color change as we highlight text. That's why the little demo only
> underlined the front part of the line, that could be be reverse video too
> but just the whitespace on the front of the line are by definition
> monochrome.  I reckon my demo is bugged as you noticed, when disabling
> syntax highlight then my underline goes all the way through the point line.
> 
> May be an idea could be set the whole point line with un-highlited,
> monochrome,  then with the new range thing use reverse or underline for the
> sub part only

Yeah.  I actually saw some screenshots of IDEs disabling syntax highlighting
in the current highlighted source line the other day, and thought that looks
nice too, and probably a better solution that the underline idea.

E.g., looking around I found this:

 https://forum.unity.com/proxy.php?image=http%3A%2F%2Fpuu.sh%2FpL42e%2F11645e7230.gif&hash=fedb45091f23bfa99b53af562f56ff0c

Try the attached patch, and do "set tui current-line-highlight reverse-mono".

"set tui current-line-highlight reverse-styled" is the same as current GDB:

  https://i.imgur.com/UwX6bfz.png

"set tui current-line-highlight reverse-mono" gives you this:

  https://i.imgur.com/T1DeRum.png

Thanks,
Pedro Alves

[-- Attachment #2: 0001-set-tui-current-line-highlight-mode-reverse-styled-r.patch --]
[-- Type: text/x-patch, Size: 5155 bytes --]

From 5524ac5733cc95c652c5db1aff6dde362ae08b1b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 15 Jun 2020 19:35:10 +0100
Subject: [PATCH] set tui current-line-highlight-mode
 reverse-styled|reverse-mono

---
 gdb/tui/tui-io.c  | 25 +++++++++++++++++--------
 gdb/tui/tui-win.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tui/tui-win.h |  8 ++++++++
 3 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e7a8ac77bce..4a012ca0eea 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -365,6 +365,10 @@ apply_ansi_escape (WINDOW *w, const char *buf)
 
   if (reverse_mode_p)
     {
+      current_line_highlight_mode mode = get_current_line_highlight_mode ();
+      if (mode == current_line_highlight_mode::reverse_mono)
+	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.,
@@ -407,19 +411,24 @@ 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);
+      current_line_highlight_mode mode = get_current_line_highlight_mode ();
+      switch (mode)
+	{
+	case current_line_highlight_mode::reverse_styled:
+	  reverse_save_bg = style.get_background ();
+	  reverse_save_fg = style.get_foreground ();
+	  break;
+	case current_line_highlight_mode::reverse_mono:
+	  style = {};
+	  break;
+	}
     }
 
+  style.set_reverse (reverse);
+
   tui_apply_style (w, style);
 }
 
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index a78837fe689..81eeb090888 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -108,6 +108,16 @@ static const char *const tui_border_mode_enums[] = {
   NULL
 };
 
+static const char *highlight_mode_reverse_styled = "reverse-styled";
+static const char *highlight_mode_reverse_mono = "reverse-mono";
+
+/* Possible values for tui-current-line-highlight variable.  */
+static const char *const tui_current_line_highlight_mode_enums[] = {
+  highlight_mode_reverse_styled,
+  highlight_mode_reverse_mono,
+  NULL
+};
+
 struct tui_translate
 {
   const char *name;
@@ -218,6 +228,39 @@ show_tui_border_kind (struct ui_file *file,
 		    value);
 }
 
+/* Tui configuration variables controlled with set/show command.  */
+static const char *tui_current_line_highlight_mode
+  = highlight_mode_reverse_styled;
+
+current_line_highlight_mode
+get_current_line_highlight_mode ()
+{
+  for (int i = 0; i < ARRAY_SIZE (tui_current_line_highlight_mode_enums); i++)
+    if (tui_current_line_highlight_mode
+	== tui_current_line_highlight_mode_enums[i])
+      return (current_line_highlight_mode) i;
+
+  gdb_assert_not_reached ("unknown mode");
+}
+
+static void
+show_tui_current_line_highlight_mode (struct ui_file *file,
+				      int from_tty,
+				      struct cmd_list_element *c,
+				      const char *value)
+{
+  fprintf_filtered (file, _("\
+The mode to use for the current source highlight is \"%s\".\n"),
+		    value);
+}
+
+static void
+set_tui_current_line_highlight_mode (const char *ignore, int from_tty,
+				     struct cmd_list_element *c)
+{
+  if (TUI_SRC_WIN != nullptr)
+    TUI_SRC_WIN->refill ();
+}
 
 /* Tui internal configuration variables.  These variables are updated
    by tui_update_variables to reflect the tui configuration
@@ -1120,6 +1163,18 @@ the line numbers and uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);
 
+  add_setshow_enum_cmd ("current-line-highlight-mode", no_class,
+			tui_current_line_highlight_mode_enums,
+			&tui_current_line_highlight_mode, _("\
+Set the attribute mode to use for the active TUI window border."), _("\
+Show the attribute mode to use for the active TUI window border."), _("\
+This variable controls the attributes to use for the active window border:\n\
+   reverse-styled  use reverse video for the background, and style the text\n\
+   reverse-mono    use reverse video for the background, don't style the text"),
+			set_tui_current_line_highlight_mode,
+			show_tui_current_line_highlight_mode,
+			&tui_setlist, &tui_showlist);
+
   tui_border_style.changed.attach (tui_rehighlight_all);
   tui_active_border_style.changed.attach (tui_rehighlight_all);
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index e3791846307..f6a39bbf437 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -57,4 +57,12 @@ struct cmd_list_element **tui_get_cmd_list (void);
 /* Whether compact source display should be used.  */
 extern bool compact_source;
 
+enum current_line_highlight_mode
+{
+  reverse_styled,
+  reverse_mono,
+};
+
+current_line_highlight_mode get_current_line_highlight_mode ();
+
 #endif /* TUI_TUI_WIN_H */

base-commit: 669203174311c5be76744a879563c697cd479853
-- 
2.14.5

Re: TUI enhancement suggestion.

[Phi Debian]

>
> .
>
> E.g., looking around I found this:
>
>  https://forum.unity.com/proxy.php?image=http%3A%2F%2Fpuu.sh%
> 2FpL42e%2F11645e7230.gif&hash=fedb45091f23bfa99b53af562f56ff0c
>
> Try the attached patch, and do "set tui current-line-highlight
> reverse-mono".
>
> "set tui current-line-highlight reverse-styled" is the same as current GDB:
>
>   https://i.imgur.com/UwX6bfz.png
>
> "set tui current-line-highlight reverse-mono" gives you this:
>
>   https://i.imgur.com/T1DeRum.png
>
> Thanks,
> Pedro Alves
>

Looks promising I like that .png will trim the pathe tomorrow.
Cheers,
Phi

Re: TUI enhancement suggestion.

[Tom Tromey]

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

Pedro> Try the attached patch, and do "set tui current-line-highlight reverse-mono".

Maybe the new mode ought to be the default.

Pedro> +static void
Pedro> +set_tui_current_line_highlight_mode (const char *ignore, int from_tty,
Pedro> +				     struct cmd_list_element *c)
Pedro> +{
Pedro> +  if (TUI_SRC_WIN != nullptr)
Pedro> +    TUI_SRC_WIN->refill ();
Pedro> +}

Possibly this should update the assembly window as well.

Tom

Re: TUI enhancement suggestion.

[Pedro Alves]

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

On 6/15/20 9:12 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Pedro> Try the attached patch, and do "set tui current-line-highlight reverse-mono".
> 
> Maybe the new mode ought to be the default.

Yeah.  I'm not sure what is the mode that people prefer the most, but that
would be fine with me.

> 
> Pedro> +static void
> Pedro> +set_tui_current_line_highlight_mode (const char *ignore, int from_tty,
> Pedro> +				     struct cmd_list_element *c)
> Pedro> +{
> Pedro> +  if (TUI_SRC_WIN != nullptr)
> Pedro> +    TUI_SRC_WIN->refill ();
> Pedro> +}
> 
> Possibly this should update the assembly window as well.

Indeed.

Here's an updated patch.

I wonder whether some option under "set style" would be more appropriate here.

Typing "set tui current-line-highlight reverse-mono" is a bit of a "mouthful".

Like,

 (gdb) set style tui-current-line on|off

I see that we already have "set style tui-active-border" and
"set style tui-border" in there.

Thanks,
Pedro Alves

[-- Attachment #2: 0001-set-tui-current-line-highlight-mode-reverse-styled-r.patch --]
[-- Type: text/x-patch, Size: 5330 bytes --]

From 162a6ff7256b88faae70988fd490f22371f3298e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 15 Jun 2020 19:35:10 +0100
Subject: [PATCH] set tui current-line-highlight-mode
 reverse-styled|reverse-mono

---
 gdb/tui/tui-io.c  | 11 +++++++++-
 gdb/tui/tui-win.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/tui/tui-win.h | 14 +++++++++++++
 3 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e7a8ac77bce..36b6ae81c45 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -365,6 +365,10 @@ apply_ansi_escape (WINDOW *w, const char *buf)
 
   if (reverse_mode_p)
     {
+      current_line_highlight_mode mode = get_current_line_highlight_mode ();
+      if (mode == current_line_highlight_mode::reverse_mono)
+	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.,
@@ -407,12 +411,15 @@ 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 ();
+
+      current_line_highlight_mode mode = get_current_line_highlight_mode ();
+      if (mode == current_line_highlight_mode::reverse_mono)
+	style = {};
     }
   else
     {
@@ -420,6 +427,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 a78837fe689..426a59194a1 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -108,6 +108,16 @@ static const char *const tui_border_mode_enums[] = {
   NULL
 };
 
+static const char *highlight_mode_reverse_styled = "reverse-styled";
+static const char *highlight_mode_reverse_mono = "reverse-mono";
+
+/* Possible values for tui-current-line-highlight variable.  */
+static const char *const tui_current_line_highlight_mode_enums[] = {
+  highlight_mode_reverse_styled,
+  highlight_mode_reverse_mono,
+  NULL
+};
+
 struct tui_translate
 {
   const char *name;
@@ -218,6 +228,44 @@ show_tui_border_kind (struct ui_file *file,
 		    value);
 }
 
+/* Implementation of the "set/show tui current-line-highlight" commands.  */
+
+static const char *tui_current_line_highlight_mode
+  = highlight_mode_reverse_styled;
+
+static void
+show_tui_current_line_highlight_mode (struct ui_file *file,
+				      int from_tty,
+				      struct cmd_list_element *c,
+				      const char *value)
+{
+  fprintf_filtered (file, _("\
+The mode to use for the current source highlight is \"%s\".\n"),
+		    value);
+}
+
+static void
+set_tui_current_line_highlight_mode (const char *ignore, int from_tty,
+				     struct cmd_list_element *c)
+{
+  if (TUI_SRC_WIN != nullptr)
+    TUI_SRC_WIN->refill ();
+  if (TUI_DISASM_WIN != nullptr)
+    TUI_DISASM_WIN->refill ();
+}
+
+/* See tui-win.h.  */
+
+current_line_highlight_mode
+get_current_line_highlight_mode ()
+{
+  for (int i = 0; i < ARRAY_SIZE (tui_current_line_highlight_mode_enums); i++)
+    if (tui_current_line_highlight_mode
+	== tui_current_line_highlight_mode_enums[i])
+      return (current_line_highlight_mode) i;
+
+  gdb_assert_not_reached ("unknown mode");
+}
 
 /* Tui internal configuration variables.  These variables are updated
    by tui_update_variables to reflect the tui configuration
@@ -1120,6 +1168,18 @@ the line numbers and uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);
 
+  add_setshow_enum_cmd ("current-line-highlight-mode", no_class,
+			tui_current_line_highlight_mode_enums,
+			&tui_current_line_highlight_mode, _("\
+Set the mode to use for the current line highlight."), _("\
+Show the mode to use for the current line highlight."), _("\
+This variable controls the mode to use for the current line indicator:\n\
+   reverse-styled  use reverse video for the background, and style the text\n\
+   reverse-mono    use reverse video for the background, don't style the text"),
+			set_tui_current_line_highlight_mode,
+			show_tui_current_line_highlight_mode,
+			&tui_setlist, &tui_showlist);
+
   tui_border_style.changed.attach (tui_rehighlight_all);
   tui_active_border_style.changed.attach (tui_rehighlight_all);
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index e3791846307..ae67b42273b 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -57,4 +57,18 @@ struct cmd_list_element **tui_get_cmd_list (void);
 /* Whether compact source display should be used.  */
 extern bool compact_source;
 
+enum current_line_highlight_mode
+{
+  /* Reverse video, and if source styling is enabled, display the
+     foreground text with style enabled.  */
+  reverse_styled,
+
+  /* Reverse video, and display foreground text in background color,
+     with source styling disabled.  */
+  reverse_mono,
+};
+
+/* Get the current line highlight mode.  */
+current_line_highlight_mode get_current_line_highlight_mode ();
+
 #endif /* TUI_TUI_WIN_H */

base-commit: 669203174311c5be76744a879563c697cd479853
-- 
2.14.5

Re: TUI enhancement suggestion.

[Phi Debian]

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.

Cheers,
Phi

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

[Pedro Alves]

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.

gdb/ChangeLog:

	* NEWS: Document that the TUI no longer styles the source code
	highlighted by the current position indicator by default.
	Mention "set style tui-current-position".

	* cli/cli-style.c (style_set_list, style_show_list): Make extern.
	* gdbcmd.h (style_set_list, style_show_list): Declare.
	* tui/tui-io.c (apply_ansi_escape): Return early if styling the
	current position is disabled.
	(tui_set_reverse_mode): If reversing, and source styling for the
	current position is disabled, switch to default style, reversed.
	* tui/tui-win.c (style_tui_current_position)
	(show_style_tui_current_position, set_style_tui_current_position):
	New.
	(_initialize_tui_win): Install the "set/show style
	tui-current-position" commands.
	* tui/tui-win.h (style_tui_current_position): Declare.

gdb/doc/ChangeLog:

	* gdb.texinfo (Output Styling): Document "set/show style
	tui-current-position".
	(TUI Overview): Document source code highlighting for the current
	position indicator and mention "set style tui-current-position
	off".
---
 gdb/doc/gdb.texinfo | 17 +++++++++++++++--
 gdb/NEWS            |  8 ++++++++
 gdb/cli/cli-style.c |  4 ++--
 gdb/gdbcmd.h        |  4 ++++
 gdb/tui/tui-io.c    | 13 ++++++++++++-
 gdb/tui/tui-win.c   | 37 +++++++++++++++++++++++++++++++++++++
 gdb/tui/tui-win.h   |  4 ++++
 7 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 59e3e75d18a..48d0ffd8524 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25521,6 +25521,15 @@ default is @samp{on}.
 
 @item show style sources
 Show the current state of source code styling.
+
+@item set style tui-current-position @samp{on|off}
+Enable or disable source code styling of the source 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 code highlighted by the TUI's current position
+indicator is styled.
 @end table
 
 Subcommands of @code{set style} control specific forms of styling.
@@ -27721,8 +27730,12 @@ This window shows the processor registers.  Registers are highlighted
 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 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/NEWS b/gdb/NEWS
index cebfd18f0c6..0c72b687cfe 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -54,6 +54,10 @@
 
 * TUI windows can now be arranged horizontally.
 
+* The TUI no longer styles the source code text highlighted by the
+  current position indicator by default.  You can however re-enable
+  styling using the new "set style tui-current-position" command.
+
 * The command history filename can now be set to the empty string
   either using 'set history filename' or by setting 'GDBHISTFILE=' in
   the environment.  The effect of setting this filename to the empty
@@ -79,6 +83,10 @@ tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
 
+set style tui-current-position [on|off]
+  Whether to style the source code text highlighted by the TUI's
+  current position indicator.  The default is off.
+
 * New targets
 
 GNU/Linux/RISC-V (gdbserver)	riscv*-*-linux*
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index a0c3cc51801..acbb58ea7bf 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -253,8 +253,8 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 			  &m_set_list, &m_show_list, (void *) this);
 }
 
-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;
 
 static void
 set_style_enabled  (const char *args, int from_tty, struct cmd_list_element *c)
diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
index 4406094ea59..eed87d53847 100644
--- a/gdb/gdbcmd.h
+++ b/gdb/gdbcmd.h
@@ -128,6 +128,10 @@ extern struct cmd_list_element *setchecklist;
 
 extern struct cmd_list_element *showchecklist;
 
+/* Chains containing all defined "set/show style" subcommands.  */
+extern struct cmd_list_element *style_set_list;
+extern struct cmd_list_element *style_show_list;
+
 /* Chain containing all defined "save" subcommands.  */
 
 extern struct cmd_list_element *save_cmdlist;
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index e7a8ac77bce..0f1d90b66dc 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -365,6 +365,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.,
@@ -407,12 +410,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
     {
@@ -420,6 +429,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 a78837fe689..e9570a7924a 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)
+{
+  fprintf_filtered (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
@@ -1120,6 +1144,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 code text highlighted by the TUI's current position\n\
+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_active_border_style.changed.attach (tui_rehighlight_all);
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index e3791846307..4def8dd670b 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -57,4 +57,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 code text highlighted by the TUI's
+   current position indicator.  */
+extern bool style_tui_current_position;
+
 #endif /* TUI_TUI_WIN_H */

base-commit: 669203174311c5be76744a879563c697cd479853
-- 
2.14.5

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

[Eli Zaretskii]

> Date: Tue, 16 Jun 2020 12:02:31 +0100
> From: Pedro Alves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tom@tromey.com>,
>  Pedro Alves via Gdb-patches <gdb-patches@sourceware.org>
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Document that the TUI no longer styles the source code
> 	highlighted by the current position indicator by default.
> 	Mention "set style tui-current-position".
> 
> 	* cli/cli-style.c (style_set_list, style_show_list): Make extern.
> 	* gdbcmd.h (style_set_list, style_show_list): Declare.
> 	* tui/tui-io.c (apply_ansi_escape): Return early if styling the
> 	current position is disabled.
> 	(tui_set_reverse_mode): If reversing, and source styling for the
> 	current position is disabled, switch to default style, reversed.
> 	* tui/tui-win.c (style_tui_current_position)
> 	(show_style_tui_current_position, set_style_tui_current_position):
> 	New.
> 	(_initialize_tui_win): Install the "set/show style
> 	tui-current-position" commands.
> 	* tui/tui-win.h (style_tui_current_position): Declare.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Output Styling): Document "set/show style
> 	tui-current-position".
> 	(TUI Overview): Document source code highlighting for the current
> 	position indicator and mention "set style tui-current-position
> 	off".

OK for the documentation parts.

Thanks.

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

[Andrew Burgess]

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 */

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

[Pedro Alves]

On 2022-11-15 10:09 a.m., Andrew Burgess wrote:

> 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?

LGTM.  Thanks for pushing this forward.

Pedro Alves

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

[Andrew Burgess]

Pedro Alves <pedro@palves.net> writes:

> On 2022-11-15 10:09 a.m., Andrew Burgess wrote:
>
>> 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?
>
> LGTM.  Thanks for pushing this forward.

Thanks.  I've pushed the patch.

Andrew

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

[Phi Debian]

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

@Andrew, @Pedro, thanx for the TUI patch, looks much nicer with my terminal
color palette and theme, I can read the current line now, before I got to
cut/paste it into my scratch buffer to read....

Cheers,