* [PATCH] [gdb/tui] Factor out border-mode help text @ 2023-05-21 19:59 Tom de Vries 2023-05-22 9:09 ` Tom de Vries 0 siblings, 1 reply; 6+ messages in thread From: Tom de Vries @ 2023-05-21 19:59 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey I noticed that the help texts for tui border-mode and tui active-border-mode are similar. Factor out the common part into macro HELP_ATTRIBUTE_MODE. Tested on x86_64-linux. --- gdb/tui/tui-win.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c index 6710b3e17e5..9d89658ef20 100644 --- a/gdb/tui/tui-win.c +++ b/gdb/tui/tui-win.c @@ -1219,18 +1219,22 @@ This variable controls the border of TUI windows:\n\ show_tui_border_kind, &tui_setlist, &tui_showlist); - add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, - &tui_border_mode, _("\ -Set the attribute mode to use for the TUI window borders."), _("\ -Show the attribute mode to use for the TUI window borders."), _("\ -This variable controls the attributes to use for the window borders:\n\ +#define HELP_ATTRIBUTE_MODE "\ normal normal display\n\ standout use highlight mode of terminal\n\ reverse use reverse video mode\n\ half use half bright\n\ half-standout use half bright and standout mode\n\ bold use extra bright or bold\n\ - bold-standout use extra bright or bold with standout mode"), + bold-standout use extra bright or bold with standout mode" + + add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, + &tui_border_mode, _("\ +Set the attribute mode to use for the TUI window borders."), _("\ +Show the attribute mode to use for the TUI window borders."), + _("\ +This variable controls the attributes to use for the window borders:\n" + HELP_ATTRIBUTE_MODE), tui_set_var_cmd, show_tui_border_mode, &tui_setlist, &tui_showlist); @@ -1238,19 +1242,16 @@ This variable controls the attributes to use for the window borders:\n\ add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums, &tui_active_border_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\ - normal normal display\n\ - standout use highlight mode of terminal\n\ - reverse use reverse video mode\n\ - half use half bright\n\ - half-standout use half bright and standout mode\n\ - bold use extra bright or bold\n\ - bold-standout use extra bright or bold with standout mode"), +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" + HELP_ATTRIBUTE_MODE), tui_set_var_cmd, show_tui_active_border_mode, &tui_setlist, &tui_showlist); +#undef HELP_ATTRIBUTE_MODE + add_setshow_zuinteger_cmd ("tab-width", no_class, &internal_tab_width, _("\ Set the tab width, in characters, for the TUI."), _("\ base-commit: f1cc4f02cb558d513cb4575211dbbb690391618f -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Factor out border-mode help text 2023-05-21 19:59 [PATCH] [gdb/tui] Factor out border-mode help text Tom de Vries @ 2023-05-22 9:09 ` Tom de Vries 2023-05-24 9:56 ` Bruno Larsen ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Tom de Vries @ 2023-05-22 9:09 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey [-- Attachment #1: Type: text/plain, Size: 520 bytes --] On 5/21/23 21:59, Tom de Vries via Gdb-patches wrote: > I noticed that the help texts for tui border-mode and tui active-border-mode > are similar. > > Factor out the common part into macro HELP_ATTRIBUTE_MODE. > This is a v2, which uses c++ std::string instead of a macro. OTOH, it changes the translation boundaries, and I'm not sure if the new parts still classify as "entire sentence". Then again, I was not able to find any files containing translations for gdb, so perhaps it doesn't matter. Thanks, - Tom [-- Attachment #2: v2-0001-gdb-tui-Factor-out-border-mode-help-text.patch --] [-- Type: text/x-patch, Size: 3223 bytes --] From 9d9af360f0732cc92de54c4d86a86816444f1996 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Sat, 20 May 2023 13:07:40 +0200 Subject: [PATCH v2] [gdb/tui] Factor out border-mode help text I noticed that the help texts for tui border-mode and tui active-border-mode are similar. Factor out the common part into std::string help_attribute_mode. Tested on x86_64-linux. --- gdb/tui/tui-win.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c index 6710b3e17e5..bc699ce0080 100644 --- a/gdb/tui/tui-win.c +++ b/gdb/tui/tui-win.c @@ -1219,34 +1219,39 @@ This variable controls the border of TUI windows:\n\ show_tui_border_kind, &tui_setlist, &tui_showlist); - add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, - &tui_border_mode, _("\ -Set the attribute mode to use for the TUI window borders."), _("\ -Show the attribute mode to use for the TUI window borders."), _("\ -This variable controls the attributes to use for the window borders:\n\ + const std::string help_attribute_mode (_("\ normal normal display\n\ standout use highlight mode of terminal\n\ reverse use reverse video mode\n\ half use half bright\n\ half-standout use half bright and standout mode\n\ bold use extra bright or bold\n\ - bold-standout use extra bright or bold with standout mode"), + bold-standout use extra bright or bold with standout mode")); + + const std::string help_tui_border_mode + = (std::string ("\ +This variable controls the attributes to use for the window borders:\n") + + help_attribute_mode); + + add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, + &tui_border_mode, _("\ +Set the attribute mode to use for the TUI window borders."), _("\ +Show the attribute mode to use for the TUI window borders."), + help_tui_border_mode.c_str (), tui_set_var_cmd, show_tui_border_mode, &tui_setlist, &tui_showlist); + const std::string help_tui_active_border_mode + = (std::string ("\ +This variable controls the attributes to use for the active window borders:\n") + + help_attribute_mode); + add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums, &tui_active_border_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\ - normal normal display\n\ - standout use highlight mode of terminal\n\ - reverse use reverse video mode\n\ - half use half bright\n\ - half-standout use half bright and standout mode\n\ - bold use extra bright or bold\n\ - bold-standout use extra bright or bold with standout mode"), +Show the attribute mode to use for the active TUI window border."), + help_tui_active_border_mode.c_str (), tui_set_var_cmd, show_tui_active_border_mode, &tui_setlist, &tui_showlist); base-commit: 92172a19f67026b3eb0f9be6762b7a5821abef84 -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Factor out border-mode help text 2023-05-22 9:09 ` Tom de Vries @ 2023-05-24 9:56 ` Bruno Larsen 2023-06-07 13:12 ` Tom de Vries 2023-06-09 13:39 ` Tom Tromey 2 siblings, 0 replies; 6+ messages in thread From: Bruno Larsen @ 2023-05-24 9:56 UTC (permalink / raw) To: Tom de Vries, gdb-patches; +Cc: Tom Tromey On 22/05/2023 11:09, Tom de Vries via Gdb-patches wrote: > On 5/21/23 21:59, Tom de Vries via Gdb-patches wrote: >> I noticed that the help texts for tui border-mode and tui >> active-border-mode >> are similar. >> >> Factor out the common part into macro HELP_ATTRIBUTE_MODE. >> > > This is a v2, which uses c++ std::string instead of a macro. > > OTOH, it changes the translation boundaries, and I'm not sure if the > new parts still classify as "entire sentence". > > Then again, I was not able to find any files containing translations > for gdb, so perhaps it doesn't matter. > > + const std::string help_tui_border_mode > + = (std::string ("\ > +This variable controls the attributes to use for the window borders:\n") > + + help_attribute_mode); I think there is no need to explicitly call the std::string constructor here (at least compiling with gcc you don't). You could have it as: + const std::string help_tui_border_mode + = ("This variable controls the attributes to use for the window borders:\n" + + help_attribute_mode); Which makes it a bit clearer in my opinion. Other than that (or if my suggestion isn't really feasible on all compilers/systems), looks ok to me: Reviewed-By: Bruno Larsen <blarsen@redhat.com> -- Cheers, Bruno > Thanks, > - Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Factor out border-mode help text 2023-05-22 9:09 ` Tom de Vries 2023-05-24 9:56 ` Bruno Larsen @ 2023-06-07 13:12 ` Tom de Vries 2023-06-09 13:39 ` Tom Tromey 2 siblings, 0 replies; 6+ messages in thread From: Tom de Vries @ 2023-06-07 13:12 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey On 5/22/23 11:09, Tom de Vries via Gdb-patches wrote: > On 5/21/23 21:59, Tom de Vries via Gdb-patches wrote: >> I noticed that the help texts for tui border-mode and tui >> active-border-mode >> are similar. >> >> Factor out the common part into macro HELP_ATTRIBUTE_MODE. >> > > This is a v2, which uses c++ std::string instead of a macro. > > OTOH, it changes the translation boundaries, and I'm not sure if the new > parts still classify as "entire sentence". > > Then again, I was not able to find any files containing translations for > gdb, so perhaps it doesn't matter. As there are no comments on these issues sofar, I've committed v1. Thanks, - Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Factor out border-mode help text 2023-05-22 9:09 ` Tom de Vries 2023-05-24 9:56 ` Bruno Larsen 2023-06-07 13:12 ` Tom de Vries @ 2023-06-09 13:39 ` Tom Tromey 2023-06-09 15:12 ` Tom de Vries 2 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2023-06-09 13:39 UTC (permalink / raw) To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries, Tom Tromey >>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: Tom> OTOH, it changes the translation boundaries, and I'm not sure if the Tom> new parts still classify as "entire sentence". Tom> Then again, I was not able to find any files containing translations Tom> for gdb, so perhaps it doesn't matter. Yeah, gdb has a bunch of calls to gettext but nobody has ever gotten translations done or installed them in the tree. I think some of the build machinery is missing too... hard to recall, it's been ages since I looked at this. Tom> + const std::string help_attribute_mode (_("\ Tom> normal normal display\n\ ... Tom> + const std::string help_tui_border_mode Tom> + = (std::string ("\ Tom> +This variable controls the attributes to use for the window borders:\n") Tom> + + help_attribute_mode); ... The text here isn't passed through gettext. Just wrapping the constant string in _() is enough. Tom> + add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, Tom> + &tui_border_mode, _("\ Tom> +Set the attribute mode to use for the TUI window borders."), _("\ Tom> +Show the attribute mode to use for the TUI window borders."), Tom> + help_tui_border_mode.c_str (), Does add_setshow_enum_cmd copy the string that's passed in? I wouldn't have thought so. If not, then this can lead to a use-after-free. Instead help_tui_border_mode would have to be 'static'. Tom> + const std::string help_tui_active_border_mode Tom> + = (std::string ("\ Tom> +This variable controls the attributes to use for the active window borders:\n") Tom> + + help_attribute_mode); Here too. And similarly with 'static' I think. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [gdb/tui] Factor out border-mode help text 2023-06-09 13:39 ` Tom Tromey @ 2023-06-09 15:12 ` Tom de Vries 0 siblings, 0 replies; 6+ messages in thread From: Tom de Vries @ 2023-06-09 15:12 UTC (permalink / raw) To: Tom Tromey, Tom de Vries via Gdb-patches [-- Attachment #1: Type: text/plain, Size: 2384 bytes --] On 6/9/23 15:39, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: > > Tom> OTOH, it changes the translation boundaries, and I'm not sure if the > Tom> new parts still classify as "entire sentence". > > Tom> Then again, I was not able to find any files containing translations > Tom> for gdb, so perhaps it doesn't matter. > > Yeah, gdb has a bunch of calls to gettext but nobody has ever gotten > translations done or installed them in the tree. I think some of the > build machinery is missing too... hard to recall, it's been ages since I > looked at this. > > Tom> + const std::string help_attribute_mode (_("\ > Tom> normal normal display\n\ > ... > > Tom> + const std::string help_tui_border_mode > Tom> + = (std::string ("\ > Tom> +This variable controls the attributes to use for the window borders:\n") > Tom> + + help_attribute_mode); > ... > > The text here isn't passed through gettext. > Just wrapping the constant string in _() is enough. > Fixed. > Tom> + add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, > Tom> + &tui_border_mode, _("\ > Tom> +Set the attribute mode to use for the TUI window borders."), _("\ > Tom> +Show the attribute mode to use for the TUI window borders."), > Tom> + help_tui_border_mode.c_str (), > > Does add_setshow_enum_cmd copy the string that's passed in? I wouldn't > have thought so. If not, then this can lead to a use-after-free. It does actually, see add_setshow_cmd_full_erased in cli-decode.c, which does: ... if (help_doc != NULL) { full_set_doc = xstrprintf ("%s\n%s", set_doc, help_doc); full_show_doc = xstrprintf ("%s\n%s", show_doc, help_doc); } else { full_set_doc = make_unique_xstrdup (set_doc); full_show_doc = make_unique_xstrdup (show_doc); } ... > Instead help_tui_border_mode would have to be 'static'. > > Tom> + const std::string help_tui_active_border_mode > Tom> + = (std::string ("\ > Tom> +This variable controls the attributes to use for the active window borders:\n") > Tom> + + help_attribute_mode); > > Here too. And similarly with 'static' I think. This version, rebased on trunk and also addressing the comment from Bruno about not needing explicit std::string constructors is what I'm planning to commit. Thanks, - Tom [-- Attachment #2: 0001-gdb-tui-Replace-macro-HELP_ATTRIBUTE_MODE-with-std-s.patch --] [-- Type: text/x-patch, Size: 2813 bytes --] From f7b7da4c2a7fa1624c21696dcedd96274ad4aba3 Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Fri, 9 Jun 2023 16:48:22 +0200 Subject: [PATCH] [gdb/tui] Replace macro HELP_ATTRIBUTE_MODE with std::string Replace macro HELP_ATTRIBUTE_MODE with a std::string. Tested on x86_64-linux. Reviewed-By: Bruno Larsen <blarsen@redhat.com> Reviewed-By: Tom Tromey <tom@tromey.com> --- gdb/tui/tui-win.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c index 71f961e04d2..7bceebb4525 100644 --- a/gdb/tui/tui-win.c +++ b/gdb/tui/tui-win.c @@ -1220,39 +1220,43 @@ This variable controls the border of TUI windows:\n\ show_tui_border_kind, &tui_setlist, &tui_showlist); -#define HELP_ATTRIBUTE_MODE "\ + const std::string help_attribute_mode (_("\ normal normal display\n\ standout use highlight mode of terminal\n\ reverse use reverse video mode\n\ half use half bright\n\ half-standout use half bright and standout mode\n\ bold use extra bright or bold\n\ - bold-standout use extra bright or bold with standout mode" + bold-standout use extra bright or bold with standout mode")); + + const std::string help_tui_border_mode + = (_("\ +This variable controls the attributes to use for the window borders:\n") + + help_attribute_mode); add_setshow_enum_cmd ("border-mode", no_class, tui_border_mode_enums, &tui_border_mode, _("\ Set the attribute mode to use for the TUI window borders."), _("\ Show the attribute mode to use for the TUI window borders."), - _("\ -This variable controls the attributes to use for the window borders:\n" - HELP_ATTRIBUTE_MODE), + help_tui_border_mode.c_str (), tui_set_var_cmd, show_tui_border_mode, &tui_setlist, &tui_showlist); + const std::string help_tui_active_border_mode + = (_("\ +This variable controls the attributes to use for the active window borders:\n") + + help_attribute_mode); + add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums, &tui_active_border_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" - HELP_ATTRIBUTE_MODE), + help_tui_active_border_mode.c_str (), tui_set_var_cmd, show_tui_active_border_mode, &tui_setlist, &tui_showlist); -#undef HELP_ATTRIBUTE_MODE - add_setshow_zuinteger_cmd ("tab-width", no_class, &internal_tab_width, _("\ Set the tab width, in characters, for the TUI."), _("\ base-commit: a3859ffba3a29b1409714cc2c44e4d0656de3021 -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-09 15:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-21 19:59 [PATCH] [gdb/tui] Factor out border-mode help text Tom de Vries 2023-05-22 9:09 ` Tom de Vries 2023-05-24 9:56 ` Bruno Larsen 2023-06-07 13:12 ` Tom de Vries 2023-06-09 13:39 ` Tom Tromey 2023-06-09 15:12 ` Tom de Vries
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).