Re: [PATCH] Update TUI window title when changed

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

> I wrote a TUI window in Python, and I noticed that setting its title
> did not result in a refresh, so the new title did not appear.  This
> patch corrects this problem.

This looks fine, but I wonder if we should just go all the way and make
the window title private and force users to make use of the set_title
API?

The patch at the bottom of this email applies on top of yours and
implements this idea, what do you think?

Thanks,
Andrew

> ---
>  gdb/python/py-tui.c                     |  2 +-
>  gdb/testsuite/gdb.python/tui-window.exp |  4 ++++
>  gdb/testsuite/gdb.python/tui-window.py  |  5 +++++
>  gdb/tui/tui-data.c                      | 11 +++++++++++
>  gdb/tui/tui-data.h                      |  4 ++++
>  5 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
> index 84a435ead68..c52b04f9a51 100644
> --- a/gdb/python/py-tui.c
> +++ b/gdb/python/py-tui.c
> @@ -531,7 +531,7 @@ gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure)
>    if (value == nullptr)
>      return -1;
>  
> -  win->window->title = value.get ();
> +  win->window->set_title (value.get ());
>    return 0;
>  }
>  
> diff --git a/gdb/testsuite/gdb.python/tui-window.exp b/gdb/testsuite/gdb.python/tui-window.exp
> index 9b5d17c35d4..defc8c68b25 100644
> --- a/gdb/testsuite/gdb.python/tui-window.exp
> +++ b/gdb/testsuite/gdb.python/tui-window.exp
> @@ -54,6 +54,10 @@ Term::check_contents "error message after trying to delete title" \
>  Term::check_contents "title is unchanged" \
>      "This Is The Title"
>  
> +Term::command "python change_window_title ()"
> +Term::check_contents "test new title" \
> +    "New Title"
> +
>  Term::resize 51 51
>  # Remember that a resize request actually does two resizes...
>  Term::check_contents "Window was updated" "Test: 2"
> diff --git a/gdb/testsuite/gdb.python/tui-window.py b/gdb/testsuite/gdb.python/tui-window.py
> index 401cb5197ff..dc72cc4c0f7 100644
> --- a/gdb/testsuite/gdb.python/tui-window.py
> +++ b/gdb/testsuite/gdb.python/tui-window.py
> @@ -53,4 +53,9 @@ def failwin(win):
>      raise RuntimeError("Whoops")
>  
>  
> +# Change the title of the window.
> +def change_window_title():
> +    the_window.win.title = "New Title"
> +
> +
>  gdb.register_window_type("fail", failwin)
> diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
> index 0daed32b6e6..abd2ec2b5fd 100644
> --- a/gdb/tui/tui-data.c
> +++ b/gdb/tui/tui-data.c
> @@ -152,6 +152,17 @@ tui_prev_win (struct tui_win_info *cur_win)
>    return *iter;
>  }
>  
> +/* See tui-data.h.  */
> +
> +void
> +tui_win_info::set_title (const char *new_title)
> +{
> +  if (title != new_title)
> +    {
> +      title = new_title;
> +      check_and_display_highlight_if_needed ();
> +    }
> +}
>  
>  void
>  tui_win_info::rerender ()
> diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
> index c92e85c9da1..030ce2a5438 100644
> --- a/gdb/tui/tui-data.h
> +++ b/gdb/tui/tui-data.h
> @@ -146,6 +146,10 @@ struct tui_win_info
>  
>    void check_and_display_highlight_if_needed ();
>  
> +  /* A helper function to change the title and then redraw the
> +     surrounding box, if needed.  */
> +  void set_title (const char *new_title);
> +
>    /* Window handle.  */
>    std::unique_ptr<WINDOW, curses_deleter> handle;
>    /* Window width.  */
> -- 
> 2.41.0

---

diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index c52b04f9a51..f47f2278dda 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -509,7 +509,7 @@ gdbpy_tui_title (PyObject *self, void *closure)
 {
   gdbpy_tui_window *win = (gdbpy_tui_window *) self;
   REQUIRE_WINDOW (win);
-  return host_string_to_python_string (win->window->title.c_str ()).release ();
+  return host_string_to_python_string (win->window->title ().c_str ()).release ();
 }
 
 /* Set the title of the TUI window.  */
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index abd2ec2b5fd..fc90df25ddd 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -155,11 +155,11 @@ tui_prev_win (struct tui_win_info *cur_win)
 /* See tui-data.h.  */
 
 void
-tui_win_info::set_title (const char *new_title)
+tui_win_info::set_title (std::string &&new_title)
 {
-  if (title != new_title)
+  if (m_title != new_title)
     {
-      title = new_title;
+      m_title = new_title;
       check_and_display_highlight_if_needed ();
     }
 }
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 030ce2a5438..d0e8d56fa07 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -148,7 +148,11 @@ struct tui_win_info
 
   /* A helper function to change the title and then redraw the
      surrounding box, if needed.  */
-  void set_title (const char *new_title);
+  void set_title (std::string &&new_title);
+
+  /* Return a reference to the current window title.  */
+  const std::string &title () const
+  { return m_title; }
 
   /* Window handle.  */
   std::unique_ptr<WINDOW, curses_deleter> handle;
@@ -160,9 +164,6 @@ struct tui_win_info
   int x = 0;
   int y = 0;
 
-  /* Window title to display.  */
-  std::string title;
-
   /* Is this window highlighted?  */
   bool is_highlighted = false;
 
@@ -175,6 +176,10 @@ struct tui_win_info
   /* Scroll the contents horizontally.  This is only called via
      left_scroll and right_scroll.  */
   virtual void do_scroll_horizontal (int num_to_scroll) = 0;
+
+private:
+  /* Window title to display.  */
+  std::string m_title;
 };
 
 /* Constant definitions.  */
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 3a8fcfa5d27..1a351e60cee 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -210,7 +210,7 @@ tui_data_window::show_register_group (const reggroup *group,
   int regnum, pos;
 
   /* Make a new title showing which group we display.  */
-  title = string_printf ("Register group: %s", group->name ());
+  this->set_title (string_printf ("Register group: %s", group->name ()));
 
   /* See how many registers must be displayed.  */
   nr_regs = 0;
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 55cde258882..6625f0cf088 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -65,7 +65,7 @@ tui_source_window::set_contents (struct gdbarch *arch,
   int cur_line_no, cur_line;
   const char *s_filename = symtab_to_filename_for_display (s);
 
-  title = s_filename;
+  set_title (s_filename);
 
   m_fullname = make_unique_xstrdup (symtab_to_fullname (s));
 
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
index 385e6e5d68d..27006869931 100644
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -106,19 +106,19 @@ box_win (struct tui_win_info *win_info,
 #else
   box (win, tui_border_vline, tui_border_hline);
 #endif
-  if (!win_info->title.empty ())
+  if (!win_info->title ().empty ())
     {
       /* Emit "+-TITLE-+" -- so 2 characters on the right and 2 on
 	 the left.  */
       int max_len = win_info->width - 2 - 2;
 
-      if (win_info->title.size () <= max_len)
-	mvwaddstr (win, 0, 2, win_info->title.c_str ());
+      if (win_info->title ().size () <= max_len)
+	mvwaddstr (win, 0, 2, win_info->title ().c_str ());
       else
 	{
 	  std::string truncated
-	    = "..." + win_info->title.substr (win_info->title.size ()
-					      - max_len + 3);
+	    = "..." + win_info->title ().substr (win_info->title ().size ()
+						 - max_len + 3);
 	  mvwaddstr (win, 0, 2, truncated.c_str ());
 	}
     }