* TUI enhancement suggestion. @ 2020-06-09 7:55 Phi Debian 2020-06-09 13:04 ` Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-09 7:55 UTC (permalink / raw) To: gdb-patches [-- 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 --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-09 7:55 TUI enhancement suggestion Phi Debian @ 2020-06-09 13:04 ` Pedro Alves 2020-06-09 14:32 ` Phi Debian [not found] ` <CAJOr74jg==A7NM4qtWEq6neXqxpxxtUEVdDgsahfvRobW+Q0wA@mail.gmail.com> 0 siblings, 2 replies; 22+ messages in thread From: Pedro Alves @ 2020-06-09 13:04 UTC (permalink / raw) To: Phi Debian, gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-09 13:04 ` Pedro Alves @ 2020-06-09 14:32 ` Phi Debian 2020-06-09 15:03 ` Phi Debian [not found] ` <CAJOr74jg==A7NM4qtWEq6neXqxpxxtUEVdDgsahfvRobW+Q0wA@mail.gmail.com> 1 sibling, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-09 14:32 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-09 14:32 ` Phi Debian @ 2020-06-09 15:03 ` Phi Debian 2020-06-11 10:34 ` Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-09 15:03 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-09 15:03 ` Phi Debian @ 2020-06-11 10:34 ` Pedro Alves 2020-06-11 13:55 ` Phi Debian 0 siblings, 1 reply; 22+ messages in thread From: Pedro Alves @ 2020-06-11 10:34 UTC (permalink / raw) To: Phi Debian; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-11 10:34 ` Pedro Alves @ 2020-06-11 13:55 ` Phi Debian 2020-06-15 14:20 ` Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-11 13:55 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-11 13:55 ` Phi Debian @ 2020-06-15 14:20 ` Pedro Alves 2020-06-15 15:48 ` Hannes Domani 0 siblings, 1 reply; 22+ messages in thread From: Pedro Alves @ 2020-06-15 14:20 UTC (permalink / raw) To: Phi Debian; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 14:20 ` Pedro Alves @ 2020-06-15 15:48 ` Hannes Domani 2020-06-15 16:56 ` Phi Debian 0 siblings, 1 reply; 22+ messages in thread From: Hannes Domani @ 2020-06-15 15:48 UTC (permalink / raw) To: Phi Debian, Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 15:48 ` Hannes Domani @ 2020-06-15 16:56 ` Phi Debian 2020-06-15 19:30 ` Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-15 16:56 UTC (permalink / raw) To: Hannes Domani; +Cc: Pedro Alves, gdb-patches 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 16:56 ` Phi Debian @ 2020-06-15 19:30 ` Pedro Alves 2020-06-15 19:47 ` Phi Debian 2020-06-15 20:12 ` Tom Tromey 0 siblings, 2 replies; 22+ messages in thread From: Pedro Alves @ 2020-06-15 19:30 UTC (permalink / raw) To: Phi Debian, Hannes Domani; +Cc: gdb-patches [-- 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 19:30 ` Pedro Alves @ 2020-06-15 19:47 ` Phi Debian 2020-06-15 20:12 ` Tom Tromey 1 sibling, 0 replies; 22+ messages in thread From: Phi Debian @ 2020-06-15 19:47 UTC (permalink / raw) To: Pedro Alves; +Cc: Hannes Domani, gdb-patches > > . > > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 19:30 ` Pedro Alves 2020-06-15 19:47 ` Phi Debian @ 2020-06-15 20:12 ` Tom Tromey 2020-06-15 20:45 ` Pedro Alves 1 sibling, 1 reply; 22+ messages in thread From: Tom Tromey @ 2020-06-15 20:12 UTC (permalink / raw) To: Pedro Alves via Gdb-patches; +Cc: Phi Debian, Hannes Domani, Pedro Alves >>>>> "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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 20:12 ` Tom Tromey @ 2020-06-15 20:45 ` Pedro Alves 2020-06-16 3:38 ` Phi Debian 0 siblings, 1 reply; 22+ messages in thread From: Pedro Alves @ 2020-06-15 20:45 UTC (permalink / raw) To: Tom Tromey, Pedro Alves via Gdb-patches [-- 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-15 20:45 ` Pedro Alves @ 2020-06-16 3:38 ` Phi Debian 2020-06-16 11:02 ` [PATCH] Add "set style tui-current-position on|off" default to off Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-16 3:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, Pedro Alves via Gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Add "set style tui-current-position on|off" default to off 2020-06-16 3:38 ` Phi Debian @ 2020-06-16 11:02 ` Pedro Alves 2020-06-16 14:33 ` Eli Zaretskii 2022-11-15 10:09 ` Andrew Burgess 0 siblings, 2 replies; 22+ messages in thread From: Pedro Alves @ 2020-06-16 11:02 UTC (permalink / raw) To: Phi Debian; +Cc: Tom Tromey, Pedro Alves via Gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add "set style tui-current-position on|off" default to off 2020-06-16 11:02 ` [PATCH] Add "set style tui-current-position on|off" default to off Pedro Alves @ 2020-06-16 14:33 ` Eli Zaretskii 2022-11-15 10:09 ` Andrew Burgess 1 sibling, 0 replies; 22+ messages in thread From: Eli Zaretskii @ 2020-06-16 14:33 UTC (permalink / raw) To: Pedro Alves; +Cc: phi.debian, tom, gdb-patches > 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add "set style tui-current-position on|off" default to off 2020-06-16 11:02 ` [PATCH] Add "set style tui-current-position on|off" default to off Pedro Alves 2020-06-16 14:33 ` Eli Zaretskii @ 2022-11-15 10:09 ` Andrew Burgess 2022-11-15 12:15 ` Pedro Alves 1 sibling, 1 reply; 22+ messages in thread From: Andrew Burgess @ 2022-11-15 10:09 UTC (permalink / raw) To: gdb-patches; +Cc: Phi Debian, Pedro Alves 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 */ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add "set style tui-current-position on|off" default to off 2022-11-15 10:09 ` Andrew Burgess @ 2022-11-15 12:15 ` Pedro Alves 2022-11-16 10:41 ` Andrew Burgess 0 siblings, 1 reply; 22+ messages in thread From: Pedro Alves @ 2022-11-15 12:15 UTC (permalink / raw) To: Andrew Burgess, gdb-patches; +Cc: Phi Debian 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add "set style tui-current-position on|off" default to off 2022-11-15 12:15 ` Pedro Alves @ 2022-11-16 10:41 ` Andrew Burgess 2022-11-17 6:30 ` Phi Debian 0 siblings, 1 reply; 22+ messages in thread From: Andrew Burgess @ 2022-11-16 10:41 UTC (permalink / raw) To: Pedro Alves, gdb-patches; +Cc: Phi Debian 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add "set style tui-current-position on|off" default to off 2022-11-16 10:41 ` Andrew Burgess @ 2022-11-17 6:30 ` Phi Debian 0 siblings, 0 replies; 22+ messages in thread From: Phi Debian @ 2022-11-17 6:30 UTC (permalink / raw) To: Andrew Burgess; +Cc: Pedro Alves, gdb-patches [-- 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, ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAJOr74jg==A7NM4qtWEq6neXqxpxxtUEVdDgsahfvRobW+Q0wA@mail.gmail.com>]
[parent not found: <CAJOr74hQdi6Y4MpGy=J-3CTRA2PP08OebTO2hBFN5NyDRokb3Q@mail.gmail.com>]
[parent not found: <CAJOr74gyyw4hJm9j0fzKQdzkJKzq=yiAXyZx_c2Q=RA8GTSN7Q@mail.gmail.com>]
[parent not found: <ed9ea9c6-3d4d-6ba7-4672-bff2b2617012@redhat.com>]
* Re: TUI enhancement suggestion. [not found] ` <ed9ea9c6-3d4d-6ba7-4672-bff2b2617012@redhat.com> @ 2020-06-10 20:25 ` Phi Debian 2020-06-11 10:17 ` Pedro Alves 0 siblings, 1 reply; 22+ messages in thread From: Phi Debian @ 2020-06-10 20:25 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: TUI enhancement suggestion. 2020-06-10 20:25 ` TUI enhancement suggestion Phi Debian @ 2020-06-11 10:17 ` Pedro Alves 0 siblings, 0 replies; 22+ messages in thread From: Pedro Alves @ 2020-06-11 10:17 UTC (permalink / raw) To: Phi Debian; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-11-17 6:31 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-09 7:55 TUI enhancement suggestion Phi Debian 2020-06-09 13:04 ` Pedro Alves 2020-06-09 14:32 ` Phi Debian 2020-06-09 15:03 ` Phi Debian 2020-06-11 10:34 ` Pedro Alves 2020-06-11 13:55 ` Phi Debian 2020-06-15 14:20 ` Pedro Alves 2020-06-15 15:48 ` Hannes Domani 2020-06-15 16:56 ` Phi Debian 2020-06-15 19:30 ` Pedro Alves 2020-06-15 19:47 ` Phi Debian 2020-06-15 20:12 ` Tom Tromey 2020-06-15 20:45 ` Pedro Alves 2020-06-16 3:38 ` Phi Debian 2020-06-16 11:02 ` [PATCH] Add "set style tui-current-position on|off" default to off Pedro Alves 2020-06-16 14:33 ` Eli Zaretskii 2022-11-15 10:09 ` Andrew Burgess 2022-11-15 12:15 ` Pedro Alves 2022-11-16 10:41 ` Andrew Burgess 2022-11-17 6:30 ` Phi Debian [not found] ` <CAJOr74jg==A7NM4qtWEq6neXqxpxxtUEVdDgsahfvRobW+Q0wA@mail.gmail.com> [not found] ` <CAJOr74hQdi6Y4MpGy=J-3CTRA2PP08OebTO2hBFN5NyDRokb3Q@mail.gmail.com> [not found] ` <CAJOr74gyyw4hJm9j0fzKQdzkJKzq=yiAXyZx_c2Q=RA8GTSN7Q@mail.gmail.com> [not found] ` <ed9ea9c6-3d4d-6ba7-4672-bff2b2617012@redhat.com> 2020-06-10 20:25 ` TUI enhancement suggestion Phi Debian 2020-06-11 10:17 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).