[PATCH] TUI resize unification

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] TUI resize unification

Tom Tromey-2
The TUI currently has two different ways to resize a window: the
resize method, and the methods make_invisible_and_set_new_height and
make_visible_with_new_height.

There's no deep reason to have two different ways to resize a window,
so this patch unifies them, leaving just the "resize" method.

This also changes the locator to be handled more like an ordinary
window and less like an adjunct of the associated source window.

gdb/ChangeLog
2019-08-15  Tom Tromey  <[hidden email]>

        * tui/tui-io.c (tui_puts_internal): Check TUI_CMD_WIN before
        calling update_cmdwin_start_line.
        * tui/tui-winsource.h (struct tui_source_window_base)
        <do_make_visible_with_new_height, set_new_height>: Don't declare.
        <rerender>: Declare.
        * tui/tui-winsource.c (tui_source_window_base::update_tab_width):
        Call rerender.
        (tui_source_window_base::set_new_height): Remove.
        (tui_source_window_base::rerender): Rename from
        do_make_visible_with_new_height.
        * tui/tui-win.c (tui_resize_all, tui_adjust_win_heights): Use
        resize method.
        (tui_win_info::make_invisible_and_set_new_height)
        (tui_win_info::make_visible_with_new_height): Remove.
        * tui/tui-stack.h (struct tui_locator_window) <rerender>:
        Declare.
        * tui/tui-stack.c (tui_locator_window::rerender): New method.
        * tui/tui-regs.h (struct tui_data_window) <set_new_height,
        do_make_visible_with_new_height>: Don't declare.
        <rerender>: Declare.
        * tui/tui-regs.c (tui_data_window::rerender): Rename from
        set_new_height.
        (tui_data_window::do_make_visible_with_new_height): Remove.
        * tui/tui-layout.c (show_source_disasm_command, show_data): Don't
        call tui_show_locator_content.
        (tui_gen_win_info::resize): Call rerender.
        (show_source_or_disasm_and_command): Don't call
        tui_show_locator_content.
        * tui/tui-data.h (struct tui_gen_win_info) <rerender>: New
        method.
        (struct tui_win_info) <rerender>: Declare.
        <set_new_height, make_invisible_and_set_new_height,
        make_visible_with_new_height>: Don't declare.
        * tui/tui-data.c (tui_win_list::rerender): New method.
        * tui/tui-command.h (struct tui_cmd_window)
        <do_make_visible_with_new_height>: Don't declare.
        * tui/tui-command.c
        (tui_cmd_window::do_make_visible_with_new_height): Remove.

gdb/testsuite/ChangeLog
2019-08-15  Tom Tromey  <[hidden email]>

        * gdb.tui/empty.exp: Enable resizing tests.
---
 gdb/ChangeLog                   |  41 ++++++++++
 gdb/testsuite/ChangeLog         |   4 +
 gdb/testsuite/gdb.tui/empty.exp |  18 ++---
 gdb/tui/tui-command.c           |  12 ---
 gdb/tui/tui-command.h           |   2 -
 gdb/tui/tui-data.c              |   6 ++
 gdb/tui/tui-data.h              |  24 ++----
 gdb/tui/tui-io.c                |   3 +-
 gdb/tui/tui-layout.c            |   5 +-
 gdb/tui/tui-regs.c              |   9 +--
 gdb/tui/tui-regs.h              |   5 +-
 gdb/tui/tui-stack.c             |   5 ++
 gdb/tui/tui-stack.h             |   2 +
 gdb/tui/tui-win.c               | 139 ++++++++++++--------------------
 gdb/tui/tui-winsource.c         |  42 ++--------
 gdb/tui/tui-winsource.h         |   5 +-
 16 files changed, 139 insertions(+), 183 deletions(-)

diff --git a/gdb/testsuite/gdb.tui/empty.exp b/gdb/testsuite/gdb.tui/empty.exp
index 90e26b3316e..8c504562803 100644
--- a/gdb/testsuite/gdb.tui/empty.exp
+++ b/gdb/testsuite/gdb.tui/empty.exp
@@ -39,14 +39,14 @@ set layouts {
     {"no source" "No Source Available"}
     {"no regs" "Register Values Unavailable"}
  }}
-    {asm asm {{3 0 77 15}} {{3 0 87 24}}
+    {asm asm {{3 0 77 15}} {{3 0 87 23}}
  {"no asm" "No Assembly Available"}}
     {regs asm-regs {{0 0 80 8} {3 7 77 9}} {{0 0 90 13} {3 13 87 14}}
  {
     {"no asm" "No Assembly Available"}
     {"no regs" "Register Values Unavailable"}
  }}
-    {split split {{3 0 77 8} {3 7 77 9}} {{3 0 87 14} {3 14 87 14}}
+    {split split {{3 0 77 8} {3 7 77 9}} {{3 0 87 13} {3 13 87 14}}
  {
     {"no source" "No Source Available"}
     {"no asm" "No Assembly Available"}
@@ -91,13 +91,11 @@ foreach layout $layouts {
     check_text $text_list
  }
 
- # FIXME: resizing is broken enough that we don't test it for
- # now.
- # Term::resize 40 90
- # with_test_prefix 90x40 {
- #     check_boxes $large_boxes
- #     check_text $text_list
- # }
- # Term::resize 24 80
+ Term::resize 40 90
+ with_test_prefix 90x40 {
+    check_boxes $large_boxes
+    check_text $text_list
+ }
+ Term::resize 24 80
     }
 }
diff --git a/gdb/tui/tui-command.c b/gdb/tui/tui-command.c
index ddbd8bccea5..f2040a75417 100644
--- a/gdb/tui/tui-command.c
+++ b/gdb/tui/tui-command.c
@@ -31,18 +31,6 @@
 
 /* See tui-command.h.  */
 
-void
-tui_cmd_window::do_make_visible_with_new_height ()
-{
-#ifdef HAVE_WRESIZE
-  wresize (handle, height, width);
-#endif
-  mvwin (handle, origin.y, origin.x);
-  wmove (handle, 0, 0);
-}
-
-/* See tui-command.h.  */
-
 int
 tui_cmd_window::max_height () const
 {
diff --git a/gdb/tui/tui-command.h b/gdb/tui/tui-command.h
index 1fce0a18126..79516941c98 100644
--- a/gdb/tui/tui-command.h
+++ b/gdb/tui/tui-command.h
@@ -74,8 +74,6 @@ protected:
   void do_scroll_horizontal (int num_to_scroll) override
   {
   }
-
-  void do_make_visible_with_new_height () override;
 };
 
 /* Refresh the command window.  */
diff --git a/gdb/tui/tui-data.c b/gdb/tui/tui-data.c
index fe1f73f02e8..dc2c810b0c1 100644
--- a/gdb/tui/tui-data.c
+++ b/gdb/tui/tui-data.c
@@ -245,3 +245,9 @@ tui_gen_win_info::~tui_gen_win_info ()
   tui_delete_win (handle);
   xfree (title);
 }
+
+void
+tui_win_info::rerender ()
+{
+  check_and_display_highlight_if_needed ();
+}
diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 338867917ef..6c7ab056b48 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -46,6 +46,12 @@ protected:
   {
   }
 
+  /* This is called after the window is resized, and should update the
+     window's contents.  */
+  virtual void rerender ()
+  {
+  }
+
 public:
 
   virtual ~tui_gen_win_info ();
@@ -164,9 +170,7 @@ protected:
      left_scroll and right_scroll.  */
   virtual void do_scroll_horizontal (int num_to_scroll) = 0;
 
-  /* Called after make_visible_with_new_height sets the new height.
-     Should update the window.  */
-  virtual void do_make_visible_with_new_height () = 0;
+  void rerender () override;
 
 public:
 
@@ -180,12 +184,6 @@ public:
   {
   }
 
-  /* Called after a TUI window is given a new height; this updates any
-     related auxiliary windows.  */
-  virtual void set_new_height (int height)
-  {
-  }
-
   /* Compute the maximum height of this window.  */
   virtual int max_height () const;
 
@@ -194,14 +192,6 @@ public:
   {
   }
 
-  /* Function make the target window (and auxiliary windows associated
-     with the target) invisible, and set the new height and
-     location.  */
-  void make_invisible_and_set_new_height (int height);
-
-  /* Make the window visible after the height has been changed.  */
-  void make_visible_with_new_height ();
-
   /* Set whether this window is highglighted.  */
   void set_highlight (bool highlight)
   {
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 938f5ab158d..7bdba3f64ff 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -495,7 +495,8 @@ tui_puts_internal (WINDOW *w, const char *string, int *height)
     }
  }
     }
-  update_cmdwin_start_line ();
+  if (TUI_CMD_WIN != nullptr && w == TUI_CMD_WIN->handle)
+    update_cmdwin_start_line ();
   if (saw_nl)
     wrefresh (w);
 }
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index e0e804bac2a..d81b8f9aa40 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -522,7 +522,6 @@ show_source_disasm_command (void)
    (src_height + asm_height) - 1);
   TUI_SRC_WIN->m_has_locator = false;
   TUI_DISASM_WIN->m_has_locator = true;
-  tui_show_locator_content ();
   TUI_DISASM_WIN->show_source_content ();
 
   if (TUI_CMD_WIN == NULL)
@@ -580,7 +579,6 @@ show_data (enum tui_layout_type new_layout)
        0, total_height);
 
   base->m_has_locator = true;
-  tui_show_locator_content ();
   current_layout = new_layout;
 }
 
@@ -611,6 +609,8 @@ tui_gen_win_info::resize (int height_, int width_,
 
   if (handle == nullptr)
     tui_make_window (this);
+
+  rerender ();
 }
 
 /* Show the Source/Command or the Disassem layout.  */
@@ -651,7 +651,6 @@ show_source_or_disasm_and_command (enum tui_layout_type layout_type)
     0);
 
   win_info->m_has_locator = true;
-  tui_show_locator_content ();
   win_info->show_source_content ();
 
   if (TUI_CMD_WIN == NULL)
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index bb8d545c47b..89faefa8f60 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -521,7 +521,7 @@ tui_data_window::do_scroll_vertical (int num_to_scroll)
 /* See tui-regs.h.  */
 
 void
-tui_data_window::set_new_height (int height)
+tui_data_window::rerender ()
 {
   /* Delete all data item windows.  */
   for (auto &&win : regs_content)
@@ -529,13 +529,6 @@ tui_data_window::set_new_height (int height)
       tui_delete_win (win->handle);
       win->handle = NULL;
     }
-}
-
-/* See tui-regs.h.  */
-
-void
-tui_data_window::do_make_visible_with_new_height ()
-{
   display_all_data ();
 }
 
diff --git a/gdb/tui/tui-regs.h b/gdb/tui/tui-regs.h
index b2abfc2078d..7b0bb505a8e 100644
--- a/gdb/tui/tui-regs.h
+++ b/gdb/tui/tui-regs.h
@@ -55,8 +55,6 @@ struct tui_data_window : public tui_win_info
 
   void refresh_all () override;
 
-  void set_new_height (int height) override;
-
   void refresh_window () override;
 
   const char *name () const override
@@ -111,7 +109,6 @@ protected:
   void do_scroll_horizontal (int num_to_scroll) override
   {
   }
-  void do_make_visible_with_new_height () override;
 
   /* Return the index of the first element displayed.  If none are
      displayed, then return -1.  */
@@ -122,6 +119,8 @@ protected:
      of the display height.  This function checks that we won't
      display off the end of the register display.  */
   void display_reg_element_at_line (int start_element_no, int start_line_no);
+
+  void rerender () override;
 };
 
 extern void tui_check_register_values (struct frame_info *);
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index f4d6d3865e1..001133c7b5e 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -277,6 +277,11 @@ tui_show_locator_content (void)
     }
 }
 
+void
+tui_locator_window::rerender ()
+{
+  tui_show_locator_content ();
+}
 
 /* Set the filename portion of the locator.  */
 static void
diff --git a/gdb/tui/tui-stack.h b/gdb/tui/tui-stack.h
index 51c976b2f56..951cf2c407b 100644
--- a/gdb/tui/tui-stack.h
+++ b/gdb/tui/tui-stack.h
@@ -43,6 +43,8 @@ struct tui_locator_window : public tui_gen_win_info
     proc_name[0] = 0;
   }
 
+  void rerender () override;
+
   char full_name[MAX_LOCATOR_ELEMENT_LEN];
   char proc_name[MAX_LOCATOR_ELEMENT_LEN];
   int line_no = 0;
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index aa07dfc8225..872e002b6cb 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -38,6 +38,7 @@
 #include "tui/tui-io.h"
 #include "tui/tui-command.h"
 #include "tui/tui-data.h"
+#include "tui/tui-layout.h"
 #include "tui/tui-wingeneral.h"
 #include "tui/tui-stack.h"
 #include "tui/tui-regs.h"
@@ -572,47 +573,38 @@ tui_resize_all (void)
  case SRC_COMMAND:
  case DISASSEM_COMMAND:
   src_win = *(tui_source_windows ().begin ());
-  first_win = src_win;
-  first_win->width += width_diff;
-  locator->width += width_diff;
   /* Check for invalid heights.  */
   if (height_diff == 0)
-    new_height = first_win->height;
-  else if ((first_win->height + split_diff) >=
+    new_height = src_win->height;
+  else if ((src_win->height + split_diff) >=
    (screenheight - MIN_CMD_WIN_HEIGHT - 1))
     new_height = screenheight - MIN_CMD_WIN_HEIGHT - 1;
-  else if ((first_win->height + split_diff) <= 0)
+  else if ((src_win->height + split_diff) <= 0)
     new_height = MIN_WIN_HEIGHT;
   else
-    new_height = first_win->height + split_diff;
+    new_height = src_win->height + split_diff;
+
+  src_win->resize (new_height, screenwidth, 0, 0);
 
-  locator->origin.y = new_height + 1;
-  first_win->make_invisible_and_set_new_height (new_height);
-  TUI_CMD_WIN->origin.y = locator->origin.y + 1;
-  TUI_CMD_WIN->width += width_diff;
-  new_height = screenheight - TUI_CMD_WIN->origin.y;
-  TUI_CMD_WIN->make_invisible_and_set_new_height (new_height);
-  first_win->make_visible_with_new_height ();
-  TUI_CMD_WIN->make_visible_with_new_height ();
-  if (src_win->content.empty ())
-    src_win->erase_source_content ();
+  locator->resize (2 /* 1 */, screenwidth,
+   0, new_height);
+
+  new_height = screenheight - (new_height + 1);
+  TUI_CMD_WIN->resize (new_height, screenwidth,
+       0, locator->origin.y + 1);
   break;
  default:
   if (cur_layout == SRC_DISASSEM_COMMAND)
     {
       src_win = TUI_SRC_WIN;
       first_win = src_win;
-      first_win->width += width_diff;
       second_win = TUI_DISASM_WIN;
-      second_win->width += width_diff;
     }
   else
     {
       first_win = TUI_DATA_WIN;
-      first_win->width += width_diff;
       src_win = *(tui_source_windows ().begin ());
       second_win = src_win;
-      second_win->width += width_diff;
     }
   /* Change the first window's height/width.  */
   /* Check for invalid heights.  */
@@ -626,9 +618,8 @@ tui_resize_all (void)
     new_height = MIN_WIN_HEIGHT;
   else
     new_height = first_win->height + split_diff;
-  first_win->make_invisible_and_set_new_height (new_height);
 
-  locator->width += width_diff;
+  first_win->resize (new_height, screenwidth, 0, 0);
 
   /* Change the second window's height/width.  */
   /* Check for invalid heights.  */
@@ -648,18 +639,17 @@ tui_resize_all (void)
     new_height = MIN_WIN_HEIGHT;
   else
     new_height = second_win->height + split_diff;
-  second_win->origin.y = first_win->height - 1;
-  second_win->make_invisible_and_set_new_height (new_height);
+
+  second_win->resize (new_height, screenwidth,
+      0, first_win->height - 1);
+
+  locator->resize (2 /* 1 */, screenwidth,
+   0, second_win->origin.y + new_height);
 
   /* Change the command window's height/width.  */
-  TUI_CMD_WIN->origin.y = locator->origin.y + 1;
-  TUI_CMD_WIN->make_invisible_and_set_new_height (TUI_CMD_WIN->height
-  + cmd_split_diff);
-  first_win->make_visible_with_new_height ();
-  second_win->make_visible_with_new_height ();
-  TUI_CMD_WIN->make_visible_with_new_height ();
-  if (src_win->content.empty ())
-    src_win->erase_source_content ();
+  new_height = screenheight - (locator->origin.y + 1);
+  TUI_CMD_WIN->resize (new_height, screenwidth,
+       0, locator->origin.y + 1);
   break;
  }
 
@@ -1036,6 +1026,7 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
   struct tui_win_info *win_info;
   struct tui_locator_window *locator = tui_locator_win_info_ptr ();
   enum tui_layout_type cur_layout = tui_current_layout ();
+  int width = tui_term_width ();
 
   diff = (new_height - primary_win_info->height) * (-1);
   if (cur_layout == SRC_COMMAND
@@ -1043,7 +1034,8 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
     {
       struct tui_win_info *src_win_info;
 
-      primary_win_info->make_invisible_and_set_new_height (new_height);
+      primary_win_info->resize (new_height, width,
+ 0, primary_win_info->origin.y);
       if (primary_win_info->type == CMD_WIN)
  {
   win_info = *(tui_source_windows ().begin ());
@@ -1054,11 +1046,9 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
   win_info = tui_win_list[CMD_WIN];
   src_win_info = primary_win_info;
  }
-      win_info->make_invisible_and_set_new_height
- (win_info->height + diff);
+      win_info->resize (win_info->height + diff, width,
+ 0, win_info->origin.y);
       TUI_CMD_WIN->origin.y = locator->origin.y + 1;
-      win_info->make_visible_with_new_height ();
-      primary_win_info->make_visible_with_new_height ();
       if ((src_win_info->type == SRC_WIN
    || src_win_info->type == DISASSEM_WIN))
  {
@@ -1121,13 +1111,18 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
       second_split_diff++;
       first_split_diff--;
     }
-  first_win->make_invisible_and_set_new_height
-    (first_win->height + first_split_diff);
-  second_win->origin.y = first_win->height - 1;
-  second_win->make_invisible_and_set_new_height
-    (second_win->height + second_split_diff);
-  TUI_CMD_WIN->origin.y = locator->origin.y + 1;
-  TUI_CMD_WIN->make_invisible_and_set_new_height (new_height);
+  first_win->resize (first_win->height + first_split_diff,
+     width,
+     0, first_win->origin.y);
+  second_win->resize (second_win->height + second_split_diff,
+      width,
+      0, first_win->height - 1);
+  locator->resize (2 /* 1 */, width,
+   0, (second_win->origin.y
+       + second_win->height + 1));
+
+  TUI_CMD_WIN->resize (new_height, width,
+       0, locator->origin.y + 1);
  }
       else
  {
@@ -1148,26 +1143,26 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
  }
     }
   if (primary_win_info == first_win)
-    first_win->make_invisible_and_set_new_height (new_height);
+    first_win->resize (new_height, width, 0, 0);
   else
-    first_win->make_invisible_and_set_new_height
-      (first_win->height);
+    first_win->resize (first_win->height, width, 0, 0);
   second_win->origin.y = first_win->height - 1;
   if (primary_win_info == second_win)
-    second_win->make_invisible_and_set_new_height (new_height);
+    second_win->resize (new_height, width,
+ 0, first_win->height - 1);
   else
-    second_win->make_invisible_and_set_new_height
-      (second_win->height);
+    second_win->resize (second_win->height, width,
+ 0, first_win->height - 1);
+  locator->resize (2 /* 1 */, width,
+   0, (second_win->origin.y
+       + second_win->height + 1));
   TUI_CMD_WIN->origin.y = locator->origin.y + 1;
   if ((TUI_CMD_WIN->height + diff) < 1)
-    TUI_CMD_WIN->make_invisible_and_set_new_height (1);
+    TUI_CMD_WIN->resize (1, width, 0, locator->origin.y + 1);
   else
-    TUI_CMD_WIN->make_invisible_and_set_new_height
-      (TUI_CMD_WIN->height + diff);
+    TUI_CMD_WIN->resize (TUI_CMD_WIN->height + diff, width,
+ 0, locator->origin.y + 1);
  }
-      TUI_CMD_WIN->make_visible_with_new_height ();
-      second_win->make_visible_with_new_height ();
-      first_win->make_visible_with_new_height ();
       if (src1 != nullptr && src1->content.empty ())
  src1->erase_source_content ();
       if (second_win->content.empty ())
@@ -1179,36 +1174,6 @@ tui_adjust_win_heights (struct tui_win_info *primary_win_info,
   return status;
 }
 
-
-/* See tui-data.h.  */
-
-void
-tui_win_info::make_invisible_and_set_new_height (int height_)
-{
-  make_visible (false);
-  height = height_;
-  if (height > 1)
-    viewport_height = height - 1;
-  else
-    viewport_height = height;
-  if (this != TUI_CMD_WIN)
-    viewport_height--;
-
-  /* Now deal with the auxiliary windows associated with win_info.  */
-  set_new_height (height);
-}
-
-
-/* See tui-data.h.  */
-
-void
-tui_win_info::make_visible_with_new_height ()
-{
-  make_visible (true);
-  check_and_display_highlight_if_needed ();
-  do_make_visible_with_new_height ();
-}
-
 /* See tui-data.h.  */
 
 int
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 613213fab5f..b6ce652d09d 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -315,42 +315,13 @@ tui_source_window_base::refresh_all ()
 void
 tui_source_window_base::update_tab_width ()
 {
-  /* We don't really change the height of any windows, but
-     calling these 2 functions causes a complete regeneration
-     and redisplay of the window's contents, which will take
-     the new tab width into account.  */
-  make_invisible_and_set_new_height (height);
-  make_visible_with_new_height ();
+  werase (handle);
+  rerender ();
 }
 
-/* See tui-data.h.  */
-
-void
-tui_source_window_base::set_new_height (int height)
-{
-  execution_info->make_visible (false);
-  execution_info->height = height;
-  execution_info->origin.y = origin.y;
-  if (height > 1)
-    execution_info->viewport_height = height - 1;
-  else
-    execution_info->viewport_height = height;
-  execution_info->viewport_height--;
-
-  if (m_has_locator)
-    {
-      tui_locator_window *gen_win_info = tui_locator_win_info_ptr ();
-      gen_win_info->make_visible (false);
-      gen_win_info->origin.y = origin.y + height;
-    }
-}
-
-/* See tui-data.h.  */
-
 void
-tui_source_window_base::do_make_visible_with_new_height ()
+tui_source_window_base::rerender ()
 {
-  execution_info->make_visible (true);
   if (!content.empty ())
     {
       struct tui_line_or_address line_or_addr;
@@ -382,11 +353,8 @@ tui_source_window_base::do_make_visible_with_new_height ()
  }
       tui_update_source_window (this, gdbarch, s, line, TRUE);
     }
-  if (m_has_locator)
-    {
-      tui_locator_win_info_ptr ()->make_visible (true);
-      tui_show_locator_content ();
-    }
+  else
+    erase_source_content ();
 }
 
 /* See tui-data.h.  */
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 9945e9f9b0e..7304e726a76 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -98,11 +98,12 @@ protected:
   DISABLE_COPY_AND_ASSIGN (tui_source_window_base);
 
   void do_scroll_horizontal (int num_to_scroll) override;
-  void do_make_visible_with_new_height () override;
 
   /* Erase the content and display STRING.  */
   void do_erase_source_content (const char *string);
 
+  void rerender () override;
+
 public:
 
   void clear_detail ();
@@ -118,8 +119,6 @@ public:
   /* Set the location of the execution point.  */
   void set_is_exec_point_at (struct tui_line_or_address l);
 
-  void set_new_height (int height) override;
-
   void update_tab_width () override;
 
   virtual bool location_matches_p (struct bp_location *loc, int line_no) = 0;
--
2.17.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] TUI resize unification

Pedro Alves-7
On 8/15/19 8:22 PM, Tom Tromey wrote:

> The TUI currently has two different ways to resize a window: the
> resize method, and the methods make_invisible_and_set_new_height and
> make_visible_with_new_height.
>
> There's no deep reason to have two different ways to resize a window,
> so this patch unifies them, leaving just the "resize" method.
>
> This also changes the locator to be handled more like an ordinary
> window and less like an adjunct of the associated source window.
>
> gdb/ChangeLog
> 2019-08-15  Tom Tromey  <[hidden email]>
>
> * tui/tui-io.c (tui_puts_internal): Check TUI_CMD_WIN before
> calling update_cmdwin_start_line.
> * tui/tui-winsource.h (struct tui_source_window_base)
> <do_make_visible_with_new_height, set_new_height>: Don't declare.
> <rerender>: Declare.
> * tui/tui-winsource.c (tui_source_window_base::update_tab_width):
> Call rerender.
> (tui_source_window_base::set_new_height): Remove.
> (tui_source_window_base::rerender): Rename from
> do_make_visible_with_new_height.
> * tui/tui-win.c (tui_resize_all, tui_adjust_win_heights): Use
> resize method.
> (tui_win_info::make_invisible_and_set_new_height)
> (tui_win_info::make_visible_with_new_height): Remove.
> * tui/tui-stack.h (struct tui_locator_window) <rerender>:
> Declare.
> * tui/tui-stack.c (tui_locator_window::rerender): New method.
> * tui/tui-regs.h (struct tui_data_window) <set_new_height,
> do_make_visible_with_new_height>: Don't declare.
> <rerender>: Declare.
> * tui/tui-regs.c (tui_data_window::rerender): Rename from
> set_new_height.
> (tui_data_window::do_make_visible_with_new_height): Remove.
> * tui/tui-layout.c (show_source_disasm_command, show_data): Don't
> call tui_show_locator_content.
> (tui_gen_win_info::resize): Call rerender.
> (show_source_or_disasm_and_command): Don't call
> tui_show_locator_content.
> * tui/tui-data.h (struct tui_gen_win_info) <rerender>: New
> method.
> (struct tui_win_info) <rerender>: Declare.
> <set_new_height, make_invisible_and_set_new_height,
> make_visible_with_new_height>: Don't declare.
> * tui/tui-data.c (tui_win_list::rerender): New method.
> * tui/tui-command.h (struct tui_cmd_window)
> <do_make_visible_with_new_height>: Don't declare.
> * tui/tui-command.c
> (tui_cmd_window::do_make_visible_with_new_height): Remove.
>
> gdb/testsuite/ChangeLog
> 2019-08-15  Tom Tromey  <[hidden email]>
>
> * gdb.tui/empty.exp: Enable resizing tests.

Looks good to me.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

New FAILs on gdb.tui/empty.exp (was: Re: [PATCH] TUI resize unification)

Sergio Durigan Junior
In reply to this post by Tom Tromey-2
On Thursday, August 15 2019, Tom Tromey wrote:

> The TUI currently has two different ways to resize a window: the
> resize method, and the methods make_invisible_and_set_new_height and
> make_visible_with_new_height.
>
> There's no deep reason to have two different ways to resize a window,
> so this patch unifies them, leaving just the "resize" method.
>
> This also changes the locator to be handled more like an ordinary
> window and less like an adjunct of the associated source window.

Hi Tom,

This commit has introduced new FAILs on gdb.tui/empty.exp:

  PASS -> FAIL: gdb.tui/empty.exp: asm-regs: 80x24: box 1
  new FAIL: gdb.tui/empty.exp: asm-regs: 90x40: box 1
  new FAIL: gdb.tui/empty.exp: asm: 90x40: box 1
  new FAIL: gdb.tui/empty.exp: split-regs: 90x40: box 1
  PASS -> FAIL: gdb.tui/empty.exp: split: 80x24: box 1
  new FAIL: gdb.tui/empty.exp: split: 90x40: box 1
  PASS -> FAIL: gdb.tui/empty.exp: src-regs: 80x24: box 1
  new FAIL: gdb.tui/empty.exp: src-regs: 90x40: box 1
  new FAIL: gdb.tui/empty.exp: src: 90x40: box 1

You can see the full Buildbot report here (with links to the logs, as
usual):

  https://sourceware.org/ml/gdb-testers/2019-q3/msg02696.html

I caught these while release a new Fedora GDB.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

Re: New FAILs on gdb.tui/empty.exp

Sergio Durigan Junior
On Sunday, August 18 2019, I wrote:

> On Thursday, August 15 2019, Tom Tromey wrote:
>
>> The TUI currently has two different ways to resize a window: the
>> resize method, and the methods make_invisible_and_set_new_height and
>> make_visible_with_new_height.
>>
>> There's no deep reason to have two different ways to resize a window,
>> so this patch unifies them, leaving just the "resize" method.
>>
>> This also changes the locator to be handled more like an ordinary
>> window and less like an adjunct of the associated source window.
>
> Hi Tom,
>
> This commit has introduced new FAILs on gdb.tui/empty.exp:
>
>   PASS -> FAIL: gdb.tui/empty.exp: asm-regs: 80x24: box 1
>   new FAIL: gdb.tui/empty.exp: asm-regs: 90x40: box 1
>   new FAIL: gdb.tui/empty.exp: asm: 90x40: box 1
>   new FAIL: gdb.tui/empty.exp: split-regs: 90x40: box 1
>   PASS -> FAIL: gdb.tui/empty.exp: split: 80x24: box 1
>   new FAIL: gdb.tui/empty.exp: split: 90x40: box 1
>   PASS -> FAIL: gdb.tui/empty.exp: src-regs: 80x24: box 1
>   new FAIL: gdb.tui/empty.exp: src-regs: 90x40: box 1
>   new FAIL: gdb.tui/empty.exp: src: 90x40: box 1
>
> You can see the full Buildbot report here (with links to the logs, as
> usual):
>
>   https://sourceware.org/ml/gdb-testers/2019-q3/msg02696.html
>
> I caught these while release a new Fedora GDB.

I forgot to say, but these seem to be racy; they go from UNRESOLVED to
FAIL and back to UNRESOLVED, apparently.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

Re: New FAILs on gdb.tui/empty.exp

Tom Tromey-2
>>>>> "Sergio" == Sergio Durigan Junior <[hidden email]> writes:

>> PASS -> FAIL: gdb.tui/empty.exp: asm-regs: 80x24: box 1

Sergio> I forgot to say, but these seem to be racy; they go from UNRESOLVED to
Sergio> FAIL and back to UNRESOLVED, apparently.

I couldn't reproduce these with the current tree, but I can reproduce
various failures with other patches of mine.

I dug into this and it was a zany adventure.

First, empty.exp is just wrong in parts, which I didn't notice when I
wrote it.  In particular, the test for text appearing in the window does
not work -- you can change the text to anything and it will still pass.

I hacked tuiterm to dump the screen on every event, which was also
educational.  I saw weird redraws, where it looked like the window size
was somehow wrong.  This lead me into the expect source code, where I
discovered it processes "stty" arguments in order -- i.e., there are two
window size changes, not a single one as you'd expect.  So, gdb sees
SIGWINCH twice, but because tuiterm doesn't know this, it results in
apparently weird redraws.

I really went down the rabbit-hole on this, trying to find a way for the
terminal to reliably decide "gdb thinks the resize is done".  I haven't
yet found a non-racy way to make this work, but the closest has been a
special "maint set" that tells gdb to print a message with a "resize
serial number" after each SIGWINCH.

Considering that I have ~50 more TUI patches in the queue, and that
fixing this particular race is hard, I think what I would like to do is
disable this test for the time being, and then fix it once the
refactorings are in.  My logic here is (a) laziness but also (b) the
refactorings make it substantially simpler to find and fix weird
behavior in the TUI.  Here in particular I'm thinking of tracking down
some extra complete screen redraws that I see.

Let me know what you think.  Also, I welcome ideas about making the
terminal implementation reliably detect "gdb is done".

Tom