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