[v2,09/14] Simplify tui_data_win::erase_data_content

Message ID 20240120-tui-regs-cleanup-v2-9-a3cccc6a3573@tromey.com
State New
Headers
Series Cleanups for the TUi register window |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Jan. 20, 2024, 6:23 p.m. UTC
  There's only a single call to tui_data_win::erase_data_content now, so
remove the parameter and make it just render the "empty window" text.

Tested-By: Tom de Vries <tdevries@suse.de>
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
---
 gdb/tui/tui-regs.c | 22 ++++++++++------------
 gdb/tui/tui-regs.h |  2 +-
 2 files changed, 11 insertions(+), 13 deletions(-)
  

Comments

Andrew Burgess Feb. 8, 2024, 11:22 a.m. UTC | #1
Tom Tromey <tom@tromey.com> writes:

> There's only a single call to tui_data_win::erase_data_content now, so
> remove the parameter and make it just render the "empty window" text.
>
> Tested-By: Tom de Vries <tdevries@suse.de>
> Reviewed-By: Andrew Burgess <aburgess@redhat.com>
> ---
>  gdb/tui/tui-regs.c | 22 ++++++++++------------
>  gdb/tui/tui-regs.h |  2 +-
>  2 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> index 52cf6b7efdf..10d9bc6ef7d 100644
> --- a/gdb/tui/tui-regs.c
> +++ b/gdb/tui/tui-regs.c
> @@ -347,21 +347,19 @@ tui_data_window::first_data_item_displayed ()
>  }
>  
>  void
> -tui_data_window::erase_data_content (const char *prompt)
> +tui_data_window::erase_data_content ()
>  {
>    werase (handle.get ());
>    check_and_display_highlight_if_needed ();
> -  if (prompt != NULL)
> -    {
> -      int half_width = (width - box_size ()) / 2;
> -      int x_pos;
>  
> -      if (strlen (prompt) >= half_width)
> -	x_pos = 1;
> -      else
> -	x_pos = half_width - strlen (prompt);
> -      display_string (height / 2, x_pos, prompt);
> -    }
> +  const char *prompt = _("[ Register Values Unavailable ]");
> +  int half_width = (width - box_size ()) / 2;
> +  int x_pos;
> +  if (strlen (prompt) >= half_width)
> +    x_pos = 1;
> +  else
> +    x_pos = half_width - strlen (prompt);

Not your code, nor is this the only place this happens, but this string
placement algorithm is weird.  It places the string within one half of
the windows visible width, rather than in the centre (as I think it
should do).

We should say:

  if (strlen (prompt) / 2 >= half_width)
    x_pos = 1;
  else
    x_pos = half_width - strlen (prompt) / 2;

I only mention this because I noticed it.  We use the same incorrect
algorithm elsewhere in the TUI code, so I think this needs fixing in a
separate series after this work is merged.

Thanks,
Andrew


> +  display_string (height / 2, x_pos, prompt);
>  }
>  
>  /* See tui-regs.h.  */
> @@ -370,7 +368,7 @@ void
>  tui_data_window::rerender ()
>  {
>    if (m_regs_content.empty ())
> -    erase_data_content (_("[ Register Values Unavailable ]"));
> +    erase_data_content ();
>    else
>      display_registers_from (0);
>    tui_wrefresh (handle.get ());
> diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
> index 7f1c30ca5d6..880f360bec6 100644
> --- a/gdb/tui/tui-regs.h
> +++ b/gdb/tui/tui-regs.h
> @@ -130,7 +130,7 @@ struct tui_data_window : public tui_win_info
>       past the register area (-1) is returned.  */
>    int first_reg_element_no_inline (int line_no) const;
>  
> -  void erase_data_content (const char *prompt);
> +  void erase_data_content ();
>  
>    /* Information about each register in the current register group.  */
>    std::vector<tui_register_info> m_regs_content;
>
> -- 
> 2.43.0
  
Tom Tromey Feb. 8, 2024, 7:19 p.m. UTC | #2
Andrew> I only mention this because I noticed it.  We use the same incorrect
Andrew> algorithm elsewhere in the TUI code, so I think this needs fixing in a
Andrew> separate series after this work is merged.

I filed this to not forget:

https://sourceware.org/bugzilla/show_bug.cgi?id=31355

Tom
  

Patch

diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 52cf6b7efdf..10d9bc6ef7d 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -347,21 +347,19 @@  tui_data_window::first_data_item_displayed ()
 }
 
 void
-tui_data_window::erase_data_content (const char *prompt)
+tui_data_window::erase_data_content ()
 {
   werase (handle.get ());
   check_and_display_highlight_if_needed ();
-  if (prompt != NULL)
-    {
-      int half_width = (width - box_size ()) / 2;
-      int x_pos;
 
-      if (strlen (prompt) >= half_width)
-	x_pos = 1;
-      else
-	x_pos = half_width - strlen (prompt);
-      display_string (height / 2, x_pos, prompt);
-    }
+  const char *prompt = _("[ Register Values Unavailable ]");
+  int half_width = (width - box_size ()) / 2;
+  int x_pos;
+  if (strlen (prompt) >= half_width)
+    x_pos = 1;
+  else
+    x_pos = half_width - strlen (prompt);
+  display_string (height / 2, x_pos, prompt);
 }
 
 /* See tui-regs.h.  */
@@ -370,7 +368,7 @@  void
 tui_data_window::rerender ()
 {
   if (m_regs_content.empty ())
-    erase_data_content (_("[ Register Values Unavailable ]"));
+    erase_data_content ();
   else
     display_registers_from (0);
   tui_wrefresh (handle.get ());
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index 7f1c30ca5d6..880f360bec6 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -130,7 +130,7 @@  struct tui_data_window : public tui_win_info
      past the register area (-1) is returned.  */
   int first_reg_element_no_inline (int line_no) const;
 
-  void erase_data_content (const char *prompt);
+  void erase_data_content ();
 
   /* Information about each register in the current register group.  */
   std::vector<tui_register_info> m_regs_content;