Re: [PATCH 08/14] Remove the TUI register window rerender overload

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

> After these restructurings, it should be clear that the rerender
> overload can be removed from the TUI register window.  This is done by
> moving a bit more logic from show_registers into update_register_data.
> After this, update_register_data simply updates the internal state,
> and rerender will write to the screen.  All the actual rendering work
> is done, ultimately, by display_registers_from.  This split between
> updating the mode and rendering makes it clear that the recursive case
> can't happen any longer.
> ---
>  gdb/tui/tui-regs.c | 55 ++++++++++++++++++++++++------------------------------
>  gdb/tui/tui-regs.h | 14 ++++++--------
>  2 files changed, 30 insertions(+), 39 deletions(-)
>
> diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
> index 7b6b669fe51..5726dc3fea8 100644
> --- a/gdb/tui/tui-regs.c
> +++ b/gdb/tui/tui-regs.c
> @@ -165,31 +165,33 @@ tui_data_window::first_reg_element_no_inline (int line_no) const
>     and refresh the window.  */
>  void
>  tui_data_window::show_registers (const reggroup *group)
> +{
> +  update_register_data (group);
> +  rerender ();
> +}
> +
> +/* Set the data window to display the registers of the register group
> +   using the given frame.  Values are refreshed only when
> +   refresh_values_only is true.  */
> +
> +void
> +tui_data_window::update_register_data (const reggroup *group)

Function comment is not correct.

>  {
>    if (group == nullptr)
>      group = general_reggroup;
>  
> -  if (target_has_registers () && target_has_stack () && target_has_memory ())
> -    update_register_data (group, get_selected_frame (nullptr));
> -  else
> +  if (!target_has_registers ()
> +      || !target_has_stack ()
> +      || !target_has_memory ())
>      {
> -      set_title (_("Registers"));
> +      set_title ("");

This is the title change I pointed out in the previous commit.  I
actually like the 'Registers' string as it means the window always has a
title, rather than the title appearing  / disappearing.

Thanks,
Andrew
>        m_current_group = nullptr;
>        m_regs_content.clear ();
> +      return;
>      }
>  
> -  rerender (false);
> -}
> -
> -
> -/* Set the data window to display the registers of the register group
> -   using the given frame.  Values are refreshed only when
> -   refresh_values_only is true.  */
> +  frame_info_ptr frame = get_selected_frame (nullptr);
>  
> -void
> -tui_data_window::update_register_data (const reggroup *group,
> -				       frame_info_ptr frame)
> -{
>    m_current_group = group;
>  
>    /* Make a new title showing which group we display.  */
> @@ -222,6 +224,9 @@ tui_data_window::update_register_data (const reggroup *group,
>  void
>  tui_data_window::display_registers_from (int start_element_no)
>  {
> +  werase (handle.get ());
> +  check_and_display_highlight_if_needed ();
> +
>    /* In case the regs window is not boxed, we'll write the last char in the
>       last line here, causing a scroll, so prevent that.  */
>    scrollok (handle.get (), FALSE);
> @@ -358,29 +363,18 @@ tui_data_window::erase_data_content (const char *prompt)
>  	x_pos = half_width - strlen (prompt);
>        display_string (height / 2, x_pos, prompt);
>      }
> -  tui_wrefresh (handle.get ());
>  }
>  
>  /* See tui-regs.h.  */
>  
>  void
> -tui_data_window::rerender (bool toplevel)
> +tui_data_window::rerender ()
>  {
>    if (m_regs_content.empty ())
> -    {
> -      if (toplevel && has_stack_frames ())
> -	{
> -	  frame_info_ptr fi = get_selected_frame (NULL);
> -	  check_register_values (fi);
> -	}
> -      else
> -	erase_data_content (_("[ Register Values Unavailable ]"));
> -    }
> +    erase_data_content (_("[ Register Values Unavailable ]"));
>    else
> -    {
> -      erase_data_content (NULL);
> -      display_registers_from (0);
> -    }
> +    display_registers_from (0);
> +  tui_wrefresh (handle.get ());
>  }
>  
>  
> @@ -402,7 +396,6 @@ tui_data_window::do_scroll_vertical (int num_to_scroll)
>    if (first_line >= 0)
>      {
>        first_line += num_to_scroll;
> -      erase_data_content (NULL);
>        display_registers_from_line (first_line);
>      }
>  }
> diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
> index 701b8707947..b31a5a3e5b8 100644
> --- a/gdb/tui/tui-regs.h
> +++ b/gdb/tui/tui-regs.h
> @@ -56,7 +56,10 @@ struct tui_register_info
>  /* The TUI registers window.  */
>  struct tui_data_window : public tui_win_info
>  {
> -  tui_data_window () = default;
> +  tui_data_window ()
> +  {
> +    update_register_data (nullptr);
> +  }
>  
>    DISABLE_COPY_AND_ASSIGN (tui_data_window);
>  
> @@ -81,11 +84,7 @@ struct tui_data_window : public tui_win_info
>    {
>    }
>  
> -  void rerender (bool toplevel);
> -  void rerender () override
> -  {
> -    rerender (true);
> -  }
> +  void rerender () override;
>  
>  private:
>  
> @@ -110,8 +109,7 @@ struct tui_data_window : public tui_win_info
>       display off the end of the register display.  */
>    void display_reg_element_at_line (int start_element_no, int start_line_no);
>  
> -  void update_register_data (const reggroup *group,
> -			     frame_info_ptr frame);
> +  void update_register_data (const reggroup *group);
>  
>    /* Answer the number of the last line in the regs display.  If there
>       are no registers (-1) is returned.  */
>
> -- 
> 2.43.0