public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] First use of tui_layout
@ 2019-10-27 21:44 Tom Tromey (Code Review)
  2019-11-06 13:30 ` Tom Tromey (Code Review)
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-27 21:44 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This does change the layout a little bit.  In particular, in the old
layout, boxed windows would share their box, but now separate boxes
are drawn.  This could be fixed with a bit of extra effort -- I'm
wondering what others think of the effect.

gdb/ChangeLog
2019-10-27  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-10-27  Tom Tromey  <tom@tromey.com>

	* gdb.tui/regs.exp: Update.
	* gdb.tui/basic.exp: Update.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/gdb.tui/regs.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
6 files changed, 74 insertions(+), 170 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cb3f0dd..0aa6334 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2019-10-27  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
+2019-10-27  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-layout.h (class tui_layout_base)
 	(class tui_layout_window, class tui_layout_split): New.
 	* tui/tui-layout.c (tui_get_window_by_name)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 89c6494..7eb8121 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2019-10-27  Tom Tromey  <tom@tromey.com>
 
+	* gdb.tui/regs.exp: Update.
+	* gdb.tui/basic.exp: Update.
+
+2019-10-27  Tom Tromey  <tom@tromey.com>
+
 	* lib/tuiterm.exp (_accept): Add wait_for parameter.  Check output
 	after any command.  Expect prompt after WAIT_FOR is seen.
 	(enter_tui): Enable resize messages.
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..3c2a0ab 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -40,10 +40,10 @@
 Term::command "layout asm"
 Term::check_contents "asm window shows main" "$hex <main>"
 
-Term::check_box "asm box" 0 0 80 15
+Term::check_box "asm box" 0 0 80 16
 
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
 
 Term::check_box "source box in split layout" 0 0 80 8
-Term::check_box "asm box in split layout" 0 7 80 8
+Term::check_box "asm box in split layout" 0 8 80 8
diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
index 1af943d..7722cc9 100644
--- a/gdb/testsuite/gdb.tui/regs.exp
+++ b/gdb/testsuite/gdb.tui/regs.exp
@@ -38,7 +38,7 @@
 
 Term::command "layout regs"
 Term::check_box "register box" 0 0 80 8
-Term::check_box "source box in regs layout" 0 7 80 8
+Term::check_box "source box in regs layout" 0 8 80 8
 
 set text [Term::get_line 1]
 # Just check for any register window content at all.
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index f3004ab..db958f6 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 static tui_gen_win_info *
@@ -819,6 +667,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -843,4 +723,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 2af5a77..0bc0de5 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -160,4 +160,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newchange

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
@ 2019-11-06 13:30 ` Tom Tromey (Code Review)
  2019-11-06 14:36 ` [review v2] " Tom Tromey (Code Review)
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-06 13:30 UTC (permalink / raw)
  To: gdb-patches

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................


Patch Set 1:

(1 comment)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368/1//COMMIT_MSG 
Commit Message:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368/1//COMMIT_MSG@12 
PS1, Line 12: 
 7 | First use of tui_layout
 8 | 
 9 | This patch introduces the first use of tui_layout, by changing
10 | show_layout to clone and use the appropriate tui_layout.
11 | 
12 > This does change the layout a little bit.  In particular, in the old
13 > layout, boxed windows would share their box, but now separate boxes
14 > are drawn.  This could be fixed with a bit of extra effort -- I'm
15 > wondering what others think of the effect.
16 | 
17 | gdb/ChangeLog
18 | 2019-10-27  Tom Tromey  <tom@tromey.com>
19 | 
20 | 	* tui/tui-layout.h (tui_apply_current_layout): Declare.

I decided to make the window borders overlap, as they do now,
so I am going to send a new version of the series to gerrit.
My reason was that overlapping borders saves some screen real
estate for actual content.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Wed, 06 Nov 2019 13:30:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v2] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
  2019-11-06 13:30 ` Tom Tromey (Code Review)
@ 2019-11-06 14:36 ` Tom Tromey (Code Review)
  2019-11-14 22:51 ` [review v3] " Tom Tromey (Code Review)
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-06 14:36 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This resulted in one minor layout change, and also in the unintended
-- but good -- side effect that the title of each boxed window is now
visible.

gdb/ChangeLog
2019-11-06  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-11-06  Tom Tromey  <tom@tromey.com>

	* gdb.tui/basic.exp: Update.
	* lib/tuiterm.exp (_check_box): Don't check bottom border.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/lib/tuiterm.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
6 files changed, 75 insertions(+), 175 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3b3bbb2..1e11090 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2019-11-06  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
+2019-11-06  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-layout.h (class tui_layout_base)
 	(class tui_layout_window, class tui_layout_split): New.
 	* tui/tui-layout.c (tui_get_window_by_name)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 54e00a1..d21cc05 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2019-11-06  Tom Tromey  <tom@tromey.com>
 
+	* gdb.tui/basic.exp: Update.
+	* lib/tuiterm.exp (_check_box): Don't check bottom border.
+
+2019-11-06  Tom Tromey  <tom@tromey.com>
+
 	* lib/tuiterm.exp (_accept): Add wait_for parameter.  Check output
 	after any command.  Expect prompt after WAIT_FOR is seen.
 	(enter_tui): Enable resize messages.
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..cf61c28 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -40,7 +40,7 @@
 Term::command "layout asm"
 Term::check_contents "asm window shows main" "$hex <main>"
 
-Term::check_box "asm box" 0 0 80 15
+Term::check_box "asm box" 0 0 80 16
 
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcba028..81247d5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -555,13 +555,9 @@
 	    return "lr corner"
 	}
 
-	for {set i [expr {$x + 1}]} {$i < $x2 - 1} {incr i} {
-	    # Note we do not check the top border of the box, because
-	    # it will contain a title.
-	    if {[get_char $i $y2] != "-"} {
-		return "bottom border $i"
-	    }
-	}
+	# Note we do not check the horizonal borders of the box.  The
+	# top will contain a title, and the bottom may as well, if it
+	# is overlapped by some other border.
 	for {set i [expr {$y + 1}]} {$i < $y2 - 1} {incr i} {
 	    if {[get_char $x $i] != "|"} {
 		return "left side $i"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 11a0eb9..8f372e6 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 static tui_gen_win_info *
@@ -858,6 +706,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -882,4 +762,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index d7f0731..a9346ef 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -174,4 +174,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 2
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v3] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
  2019-11-06 13:30 ` Tom Tromey (Code Review)
  2019-11-06 14:36 ` [review v2] " Tom Tromey (Code Review)
@ 2019-11-14 22:51 ` Tom Tromey (Code Review)
  2019-11-18 21:12 ` Andrew Burgess (Code Review)
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-14 22:51 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This resulted in one minor layout change, and also in the unintended
-- but good -- side effect that the title of each boxed window is now
visible.

2019-11-12  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-11-12  Tom Tromey  <tom@tromey.com>

	* gdb.tui/basic.exp: Update.
	* lib/tuiterm.exp (_check_box): Don't check bottom border.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/lib/tuiterm.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
6 files changed, 75 insertions(+), 175 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ea58cbb..161cf4a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2019-11-12  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
+2019-11-12  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-layout.h (class tui_layout_base)
 	(class tui_layout_window, class tui_layout_split): New.
 	* tui/tui-layout.c (tui_get_window_by_name)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 3a4d229..ff2139e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-12  Tom Tromey  <tom@tromey.com>
+
+	* gdb.tui/basic.exp: Update.
+	* lib/tuiterm.exp (_check_box): Don't check bottom border.
+
 2019-11-14  Tom Tromey  <tromey@adacore.com>
 
 	* gdb.base/gdbvars.exp (test_convenience_variables): Add
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..cf61c28 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -40,7 +40,7 @@
 Term::command "layout asm"
 Term::check_contents "asm window shows main" "$hex <main>"
 
-Term::check_box "asm box" 0 0 80 15
+Term::check_box "asm box" 0 0 80 16
 
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcba028..81247d5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -555,13 +555,9 @@
 	    return "lr corner"
 	}
 
-	for {set i [expr {$x + 1}]} {$i < $x2 - 1} {incr i} {
-	    # Note we do not check the top border of the box, because
-	    # it will contain a title.
-	    if {[get_char $i $y2] != "-"} {
-		return "bottom border $i"
-	    }
-	}
+	# Note we do not check the horizonal borders of the box.  The
+	# top will contain a title, and the bottom may as well, if it
+	# is overlapped by some other border.
 	for {set i [expr {$y + 1}]} {$i < $y2 - 1} {incr i} {
 	    if {[get_char $x $i] != "|"} {
 		return "left side $i"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 0fc2f36..097d9f6 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 /* Helper function that returns a TUI window, given its name.  */
@@ -888,6 +736,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -912,4 +792,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index d7f0731..a9346ef 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -174,4 +174,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v3] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (2 preceding siblings ...)
  2019-11-14 22:51 ` [review v3] " Tom Tromey (Code Review)
@ 2019-11-18 21:12 ` Andrew Burgess (Code Review)
  2019-11-19 13:11 ` Tom Tromey (Code Review)
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-18 21:12 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................


Patch Set 3:

I noticed something a little strange, I wonder if this is intended, or an unexpected feature.

I started up a two debug sessions one with current HEAD GDB, and one with your patches up to this point.  Both terminals were 51 tall and 100 wide.  In each session I did:

  gdb TESTFILE
  (gdb) start
  (gdb) tui enable

The first thing I spotted was that the CMD window was different sizes between current HEAD and your patch - this isn't a problem, just an observation.

Then I did:

  (gdb) layout next

In current HEAD notice that the CMD window doesn't change size, while in your patch I am seeing the CMD window get smaller by I think 1 line.

After this:

  (gdb) layout prev

In HEAD again no change in size.  In your patch the CMD window doesn't return back to its original size, it remains at the slightly larger size.

In the previous patch there's a comment about the command window not changing size when the layout changes, which is why this seems so odd.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Comment-Date: Mon, 18 Nov 2019 21:12:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v3] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (3 preceding siblings ...)
  2019-11-18 21:12 ` Andrew Burgess (Code Review)
@ 2019-11-19 13:11 ` Tom Tromey (Code Review)
  2019-11-19 20:36 ` [review v4] " Tom Tromey (Code Review)
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-19 13:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................


Patch Set 3:

> Patch Set 3:
> 
> I noticed something a little strange, I wonder if this is intended, or an unexpected feature.
[...]
> In current HEAD notice that the CMD window doesn't change size, while in your patch I am seeing the CMD window get smaller by I think 1 line.

I debugged this and ended up rewriting some of the layout logic.
I think it will work in the next revision of the patch.

This also revealed that I had inadvertently swapped the location of the source
and register windows in the regs view.  And, then this showed that the register
window has a bug where it must be displayed at the top of the terminal -- not
important for this series, but something that must be fixed for the next series.

So, thanks for noticing this!


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 3
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 13:11:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v4] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (4 preceding siblings ...)
  2019-11-19 13:11 ` Tom Tromey (Code Review)
@ 2019-11-19 20:36 ` Tom Tromey (Code Review)
  2019-11-26 23:52 ` Andrew Burgess (Code Review)
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-19 20:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This resulted in one minor layout change, and also in the unintended
-- but good -- side effect that the title of each boxed window is now
visible.

2019-11-18  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-11-18  Tom Tromey  <tom@tromey.com>

	* gdb.tui/regs.exp: Update.
	* gdb.tui/empty.exp (layouts): Update.
	* gdb.tui/basic.exp: Update.
	* lib/tuiterm.exp (_check_box): Don't check bottom border.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/gdb.tui/empty.exp
M gdb/testsuite/gdb.tui/regs.exp
M gdb/testsuite/lib/tuiterm.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
8 files changed, 84 insertions(+), 182 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d462479..74d4210 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2019-11-18  Tom Tromey  <tom@tromey.com>
+
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
 2019-11-12  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-layout.h (class tui_layout_base)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 32dc308..9994007 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-18  Tom Tromey  <tom@tromey.com>
+
+	* gdb.tui/regs.exp: Update.
+	* gdb.tui/empty.exp (layouts): Update.
+	* gdb.tui/basic.exp: Update.
+	* lib/tuiterm.exp (_check_box): Don't check bottom border.
+
 2019-11-19  Tom Tromey  <tom@tromey.com>
 
 	* gdb.tui/winheight.exp: New file.
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..f9d57d1 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -45,5 +45,5 @@
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
 
-Term::check_box "source box in split layout" 0 0 80 8
-Term::check_box "asm box in split layout" 0 7 80 8
+Term::check_box "source box in split layout" 0 0 80 7
+Term::check_box "asm box in split layout" 0 6 80 9
diff --git a/gdb/testsuite/gdb.tui/empty.exp b/gdb/testsuite/gdb.tui/empty.exp
index b6ee350..861f564 100644
--- a/gdb/testsuite/gdb.tui/empty.exp
+++ b/gdb/testsuite/gdb.tui/empty.exp
@@ -34,7 +34,7 @@
 set layouts {
     {src src {{0 0 80 15}} {{0 0 90 23}}
 	{{"no source" "No Source Available"}}}
-    {regs src-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs src-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no regs" "Register Values Unavailable"}
@@ -43,17 +43,17 @@
 	{
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs asm-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs asm-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
 	}}
-    {split split {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {split split {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs split-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs split-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
index 1af943d..dcecd03 100644
--- a/gdb/testsuite/gdb.tui/regs.exp
+++ b/gdb/testsuite/gdb.tui/regs.exp
@@ -37,8 +37,8 @@
 Term::check_contents "source at startup" ">|21 *return 0"
 
 Term::command "layout regs"
-Term::check_box "register box" 0 0 80 8
-Term::check_box "source box in regs layout" 0 7 80 8
+Term::check_box "register box" 0 0 80 7
+Term::check_box "source box in regs layout" 0 6 80 9
 
 set text [Term::get_line 1]
 # Just check for any register window content at all.
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcba028..81247d5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -555,13 +555,9 @@
 	    return "lr corner"
 	}
 
-	for {set i [expr {$x + 1}]} {$i < $x2 - 1} {incr i} {
-	    # Note we do not check the top border of the box, because
-	    # it will contain a title.
-	    if {[get_char $i $y2] != "-"} {
-		return "bottom border $i"
-	    }
-	}
+	# Note we do not check the horizonal borders of the box.  The
+	# top will contain a title, and the bottom may as well, if it
+	# is overlapped by some other border.
 	for {set i [expr {$y + 1}]} {$i < $y2 - 1} {incr i} {
 	    if {[get_char $x $i] != "|"} {
 		return "left side $i"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 91f270d..1884c2a 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 /* Helper function that returns a TUI window, given its name.  */
@@ -901,6 +749,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -925,4 +805,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index d7f0731..a9346ef 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -174,4 +174,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [review v4] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (5 preceding siblings ...)
  2019-11-19 20:36 ` [review v4] " Tom Tromey (Code Review)
@ 2019-11-26 23:52 ` Andrew Burgess (Code Review)
  2019-12-11 23:21 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-12-11 23:21 ` Sourceware to Gerrit sync (Code Review)
  8 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-26 23:52 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................


Patch Set 4: Code-Review+2

LGTM.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 4
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 23:52:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pushed] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (7 preceding siblings ...)
  2019-12-11 23:21 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-12-11 23:21 ` Sourceware to Gerrit sync (Code Review)
  8 siblings, 0 replies; 10+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-11 23:21 UTC (permalink / raw)
  To: Tom Tromey, Andrew Burgess, gdb-patches

The original change was created by Tom Tromey.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This resulted in one minor layout change, and also in the unintended
-- but good -- side effect that the title of each boxed window is now
visible.

gdb/ChangeLog
2019-12-11  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-12-11  Tom Tromey  <tom@tromey.com>

	* gdb.tui/regs.exp: Update.
	* gdb.tui/empty.exp (layouts): Update.
	* gdb.tui/basic.exp: Update.
	* lib/tuiterm.exp (_check_box): Don't check bottom border.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/gdb.tui/empty.exp
M gdb/testsuite/gdb.tui/regs.exp
M gdb/testsuite/lib/tuiterm.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
8 files changed, 84 insertions(+), 182 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 595b987..4c98df3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2019-12-11  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
+2019-12-11  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-layout.h (class tui_layout_base)
 	(class tui_layout_window, class tui_layout_split): New.
 	* tui/tui-layout.c (tui_get_window_by_name)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 17d4993..9b7a81e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2019-12-11  Tom Tromey  <tom@tromey.com>
+
+	* gdb.tui/regs.exp: Update.
+	* gdb.tui/empty.exp (layouts): Update.
+	* gdb.tui/basic.exp: Update.
+	* lib/tuiterm.exp (_check_box): Don't check bottom border.
+
 2019-12-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.base/options.exp: Add -raw-values in the print completion list.
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..f9d57d1 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -45,5 +45,5 @@
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
 
-Term::check_box "source box in split layout" 0 0 80 8
-Term::check_box "asm box in split layout" 0 7 80 8
+Term::check_box "source box in split layout" 0 0 80 7
+Term::check_box "asm box in split layout" 0 6 80 9
diff --git a/gdb/testsuite/gdb.tui/empty.exp b/gdb/testsuite/gdb.tui/empty.exp
index b6ee350..861f564 100644
--- a/gdb/testsuite/gdb.tui/empty.exp
+++ b/gdb/testsuite/gdb.tui/empty.exp
@@ -34,7 +34,7 @@
 set layouts {
     {src src {{0 0 80 15}} {{0 0 90 23}}
 	{{"no source" "No Source Available"}}}
-    {regs src-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs src-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no regs" "Register Values Unavailable"}
@@ -43,17 +43,17 @@
 	{
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs asm-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs asm-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
 	}}
-    {split split {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {split split {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs split-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs split-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
index 1af943d..dcecd03 100644
--- a/gdb/testsuite/gdb.tui/regs.exp
+++ b/gdb/testsuite/gdb.tui/regs.exp
@@ -37,8 +37,8 @@
 Term::check_contents "source at startup" ">|21 *return 0"
 
 Term::command "layout regs"
-Term::check_box "register box" 0 0 80 8
-Term::check_box "source box in regs layout" 0 7 80 8
+Term::check_box "register box" 0 0 80 7
+Term::check_box "source box in regs layout" 0 6 80 9
 
 set text [Term::get_line 1]
 # Just check for any register window content at all.
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcba028..81247d5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -555,13 +555,9 @@
 	    return "lr corner"
 	}
 
-	for {set i [expr {$x + 1}]} {$i < $x2 - 1} {incr i} {
-	    # Note we do not check the top border of the box, because
-	    # it will contain a title.
-	    if {[get_char $i $y2] != "-"} {
-		return "bottom border $i"
-	    }
-	}
+	# Note we do not check the horizonal borders of the box.  The
+	# top will contain a title, and the bottom may as well, if it
+	# is overlapped by some other border.
 	for {set i [expr {$y + 1}]} {$i < $y2 - 1} {incr i} {
 	    if {[get_char $x $i] != "|"} {
 		return "left side $i"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 91f270d..1884c2a 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 /* Helper function that returns a TUI window, given its name.  */
@@ -901,6 +749,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -925,4 +805,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index d7f0731..a9346ef 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -174,4 +174,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pushed] First use of tui_layout
  2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
                   ` (6 preceding siblings ...)
  2019-11-26 23:52 ` Andrew Burgess (Code Review)
@ 2019-12-11 23:21 ` Sourceware to Gerrit sync (Code Review)
  2019-12-11 23:21 ` Sourceware to Gerrit sync (Code Review)
  8 siblings, 0 replies; 10+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-11 23:21 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches; +Cc: Andrew Burgess

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/368
......................................................................

First use of tui_layout

This patch introduces the first use of tui_layout, by changing
show_layout to clone and use the appropriate tui_layout.

This resulted in one minor layout change, and also in the unintended
-- but good -- side effect that the title of each boxed window is now
visible.

gdb/ChangeLog
2019-12-11  Tom Tromey  <tom@tromey.com>

	* tui/tui-layout.h (tui_apply_current_layout): Declare.
	* tui/tui-layout.c (standard_layouts, applied_layout): New
	globals.
	(tui_apply_current_layout): New function.
	(show_layout): Set applied_layout.  Call
	tui_apply_current_layout.
	(show_source_command, show_disasm_command)
	(show_source_disasm_command, show_data)
	(show_source_or_disasm_and_command): Remove.
	(initialize_layouts): New function.
	(_initialize_tui_layout): Call initialize_layouts.

gdb/testsuite/ChangeLog
2019-12-11  Tom Tromey  <tom@tromey.com>

	* gdb.tui/regs.exp: Update.
	* gdb.tui/empty.exp (layouts): Update.
	* gdb.tui/basic.exp: Update.
	* lib/tuiterm.exp (_check_box): Don't check bottom border.

Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
---
M gdb/ChangeLog
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.tui/basic.exp
M gdb/testsuite/gdb.tui/empty.exp
M gdb/testsuite/gdb.tui/regs.exp
M gdb/testsuite/lib/tuiterm.exp
M gdb/tui/tui-layout.c
M gdb/tui/tui-layout.h
8 files changed, 84 insertions(+), 182 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 595b987..4c98df3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2019-12-11  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-layout.h (tui_apply_current_layout): Declare.
+	* tui/tui-layout.c (standard_layouts, applied_layout): New
+	globals.
+	(tui_apply_current_layout): New function.
+	(show_layout): Set applied_layout.  Call
+	tui_apply_current_layout.
+	(show_source_command, show_disasm_command)
+	(show_source_disasm_command, show_data)
+	(show_source_or_disasm_and_command): Remove.
+	(initialize_layouts): New function.
+	(_initialize_tui_layout): Call initialize_layouts.
+
+2019-12-11  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-layout.h (class tui_layout_base)
 	(class tui_layout_window, class tui_layout_split): New.
 	* tui/tui-layout.c (tui_get_window_by_name)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 17d4993..9b7a81e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2019-12-11  Tom Tromey  <tom@tromey.com>
+
+	* gdb.tui/regs.exp: Update.
+	* gdb.tui/empty.exp (layouts): Update.
+	* gdb.tui/basic.exp: Update.
+	* lib/tuiterm.exp (_check_box): Don't check bottom border.
+
 2019-12-11  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.base/options.exp: Add -raw-values in the print completion list.
diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
index 716c52f..f9d57d1 100644
--- a/gdb/testsuite/gdb.tui/basic.exp
+++ b/gdb/testsuite/gdb.tui/basic.exp
@@ -45,5 +45,5 @@
 Term::command "layout split"
 Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
 
-Term::check_box "source box in split layout" 0 0 80 8
-Term::check_box "asm box in split layout" 0 7 80 8
+Term::check_box "source box in split layout" 0 0 80 7
+Term::check_box "asm box in split layout" 0 6 80 9
diff --git a/gdb/testsuite/gdb.tui/empty.exp b/gdb/testsuite/gdb.tui/empty.exp
index b6ee350..861f564 100644
--- a/gdb/testsuite/gdb.tui/empty.exp
+++ b/gdb/testsuite/gdb.tui/empty.exp
@@ -34,7 +34,7 @@
 set layouts {
     {src src {{0 0 80 15}} {{0 0 90 23}}
 	{{"no source" "No Source Available"}}}
-    {regs src-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs src-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no regs" "Register Values Unavailable"}
@@ -43,17 +43,17 @@
 	{
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs asm-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs asm-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
 	}}
-    {split split {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {split split {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no source" "No Source Available"}
 	    {"no asm" "No Assembly Available"}
 	}}
-    {regs split-regs {{0 0 80 8} {0 7 80 8}} {{0 0 90 13} {0 12 90 13}}
+    {regs split-regs {{0 0 80 7} {0 6 80 9}} {{0 0 90 12} {0 11 90 14}}
 	{
 	    {"no asm" "No Assembly Available"}
 	    {"no regs" "Register Values Unavailable"}
diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
index 1af943d..dcecd03 100644
--- a/gdb/testsuite/gdb.tui/regs.exp
+++ b/gdb/testsuite/gdb.tui/regs.exp
@@ -37,8 +37,8 @@
 Term::check_contents "source at startup" ">|21 *return 0"
 
 Term::command "layout regs"
-Term::check_box "register box" 0 0 80 8
-Term::check_box "source box in regs layout" 0 7 80 8
+Term::check_box "register box" 0 0 80 7
+Term::check_box "source box in regs layout" 0 6 80 9
 
 set text [Term::get_line 1]
 # Just check for any register window content at all.
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcba028..81247d5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -555,13 +555,9 @@
 	    return "lr corner"
 	}
 
-	for {set i [expr {$x + 1}]} {$i < $x2 - 1} {incr i} {
-	    # Note we do not check the top border of the box, because
-	    # it will contain a title.
-	    if {[get_char $i $y2] != "-"} {
-		return "bottom border $i"
-	    }
-	}
+	# Note we do not check the horizonal borders of the box.  The
+	# top will contain a title, and the bottom may as well, if it
+	# is overlapped by some other border.
 	for {set i [expr {$y + 1}]} {$i < $y2 - 1} {incr i} {
 	    if {[get_char $x $i] != "|"} {
 		return "left side $i"
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 91f270d..1884c2a 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -41,17 +41,18 @@
 #include "gdb_curses.h"
 
 static void show_layout (enum tui_layout_type);
-static void show_source_or_disasm_and_command (enum tui_layout_type);
-static void show_source_command (void);
-static void show_disasm_command (void);
-static void show_source_disasm_command (void);
-static void show_data (enum tui_layout_type);
 static enum tui_layout_type next_layout (void);
 static enum tui_layout_type prev_layout (void);
 static void tui_layout_command (const char *, int);
 static void extract_display_start_addr (struct gdbarch **, CORE_ADDR *);
 
 
+/* The pre-defined layouts.  */
+static tui_layout_split *standard_layouts[UNDEFINED_LAYOUT];
+
+/* The layout that is currently applied.  */
+static std::unique_ptr<tui_layout_base> applied_layout;
+
 static enum tui_layout_type current_layout = UNDEFINED_LAYOUT;
 
 /* Accessor for the current layout.  */
@@ -61,6 +62,13 @@
   return current_layout;
 }
 
+/* See tui-layout.h.  */
+
+void
+tui_apply_current_layout ()
+{
+  applied_layout->apply (0, 0, tui_term_width (), tui_term_height ());
+}
 
 /* Show the screen layout defined.  */
 static void
@@ -71,26 +79,8 @@
   if (layout != cur_layout)
     {
       tui_make_all_invisible ();
-      switch (layout)
-	{
-	case SRC_DATA_COMMAND:
-	case DISASSEM_DATA_COMMAND:
-	  show_data (layout);
-	  break;
-	  /* Now show the new layout.  */
-	case SRC_COMMAND:
-	  show_source_command ();
-	  break;
-	case DISASSEM_COMMAND:
-	  show_disasm_command ();
-	  break;
-	case SRC_DISASSEM_COMMAND:
-	  show_source_disasm_command ();
-	  break;
-	default:
-	  break;
-	}
-
+      applied_layout = standard_layouts[layout]->clone ();
+      tui_apply_current_layout ();
       current_layout = layout;
       tui_delete_invisible_windows ();
     }
@@ -364,105 +354,6 @@
   return (enum tui_layout_type) new_layout;
 }
 
-/* Show the Source/Command layout.  */
-static void
-show_source_command (void)
-{
-  show_source_or_disasm_and_command (SRC_COMMAND);
-}
-
-
-/* Show the Dissassem/Command layout.  */
-static void
-show_disasm_command (void)
-{
-  show_source_or_disasm_and_command (DISASSEM_COMMAND);
-}
-
-
-/* Show the Source/Disassem/Command layout.  */
-static void
-show_source_disasm_command (void)
-{
-  int cmd_height, src_height, asm_height;
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-
-  src_height = (tui_term_height () - cmd_height) / 2;
-  asm_height = tui_term_height () - (src_height + cmd_height);
-
-  if (TUI_SRC_WIN == NULL)
-    tui_win_list[SRC_WIN] = new tui_source_window ();
-  TUI_SRC_WIN->resize (src_height,
-		       tui_term_width (),
-		       0,
-		       0);
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_DISASM_WIN == NULL)
-    tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-  TUI_DISASM_WIN->resize (asm_height,
-			  tui_term_width (),
-			  0,
-			  src_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, (src_height + asm_height) - 1);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       tui_term_height () - cmd_height);
-}
-
-
-/* Show the Source/Data/Command or the Dissassembly/Data/Command
-   layout.  */
-static void
-show_data (enum tui_layout_type new_layout)
-{
-  int total_height = (tui_term_height () - TUI_CMD_WIN->height);
-  int src_height, data_height;
-  enum tui_win_type win_type;
-
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  data_height = total_height / 2;
-  src_height = total_height - data_height;
-  if (tui_win_list[DATA_WIN] == nullptr)
-    tui_win_list[DATA_WIN] = new tui_data_window ();
-  tui_win_list[DATA_WIN]->resize (data_height, tui_term_width (), 0, 0);
-
-  if (new_layout == SRC_DATA_COMMAND)
-    win_type = SRC_WIN;
-  else
-    win_type = DISASSEM_WIN;
-
-  if (tui_win_list[win_type] == NULL)
-    {
-      if (win_type == SRC_WIN)
-	tui_win_list[win_type] = new tui_source_window ();
-      else
-	tui_win_list[win_type] = new tui_disasm_window ();
-    }
-
-  tui_win_list[win_type]->resize (src_height,
-				  tui_term_width (),
-				  0,
-				  data_height - 1);
-  locator->resize (1, tui_term_width (),
-		   0, total_height - 1);
-  TUI_CMD_WIN->resize (TUI_CMD_WIN->height, tui_term_width (),
-		       0, total_height);
-}
-
 void
 tui_gen_win_info::resize (int height_, int width_,
 			  int origin_x_, int origin_y_)
@@ -498,49 +389,6 @@
   rerender ();
 }
 
-/* Show the Source/Command or the Disassem layout.  */
-static void
-show_source_or_disasm_and_command (enum tui_layout_type layout_type)
-{
-  struct tui_source_window_base *win_info;
-  int src_height, cmd_height;
-  struct tui_locator_window *locator = tui_locator_win_info_ptr ();
-  gdb_assert (locator != nullptr);
-
-  if (TUI_CMD_WIN != NULL)
-    cmd_height = TUI_CMD_WIN->height;
-  else
-    cmd_height = tui_term_height () / 3;
-  src_height = tui_term_height () - cmd_height;
-
-  if (layout_type == SRC_COMMAND)
-    {
-      if (tui_win_list[SRC_WIN] == nullptr)
-	tui_win_list[SRC_WIN] = new tui_source_window ();
-      win_info = TUI_SRC_WIN;
-    }
-  else
-    {
-      if (tui_win_list[DISASSEM_WIN] == nullptr)
-	tui_win_list[DISASSEM_WIN] = new tui_disasm_window ();
-      win_info = TUI_DISASM_WIN;
-    }
-
-  locator->resize (1, tui_term_width (),
-		   0, src_height - 1);
-  win_info->resize (src_height - 1,
-		    tui_term_width (),
-		    0,
-		    0);
-
-  if (TUI_CMD_WIN == NULL)
-    tui_win_list[CMD_WIN] = new tui_cmd_window ();
-  TUI_CMD_WIN->resize (cmd_height,
-		       tui_term_width (),
-		       0,
-		       src_height);
-}
-
 \f
 
 /* Helper function that returns a TUI window, given its name.  */
@@ -901,6 +749,38 @@
   m_applied = true;
 }
 
+static void
+initialize_layouts ()
+{
+  standard_layouts[SRC_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_COMMAND]->add_window ("src", 2);
+  standard_layouts[SRC_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_COMMAND]->add_window ("asm", 2);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[DISASSEM_DATA_COMMAND] = new tui_layout_split ();
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("regs", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("asm", 1);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("locator", 0);
+  standard_layouts[DISASSEM_DATA_COMMAND]->add_window ("cmd", 1);
+
+  standard_layouts[SRC_DISASSEM_COMMAND] = new tui_layout_split ();
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("src", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("asm", 1);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("locator", 0);
+  standard_layouts[SRC_DISASSEM_COMMAND]->add_window ("cmd", 1);
+}
+
 \f
 
 /* Function to initialize gdb commands, for tui window layout
@@ -925,4 +805,6 @@
            the register window is displayed with \n\
            the window that has current logical focus."));
   set_cmd_completer (cmd, layout_completer);
+
+  initialize_layouts ();
 }
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index d7f0731..a9346ef 100644
--- a/gdb/tui/tui-layout.h
+++ b/gdb/tui/tui-layout.h
@@ -174,4 +174,7 @@
 extern void tui_add_win_to_layout (enum tui_win_type);
 extern void tui_set_layout (enum tui_layout_type);
 
+/* Apply the current layout.  */
+extern void tui_apply_current_layout ();
+
 #endif /* TUI_TUI_LAYOUT_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: If1ee06ee58f4803e8c213f4ab0f5bb59f4650ec2
Gerrit-Change-Number: 368
Gerrit-PatchSet: 5
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-12-11 23:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-27 21:44 [review] First use of tui_layout Tom Tromey (Code Review)
2019-11-06 13:30 ` Tom Tromey (Code Review)
2019-11-06 14:36 ` [review v2] " Tom Tromey (Code Review)
2019-11-14 22:51 ` [review v3] " Tom Tromey (Code Review)
2019-11-18 21:12 ` Andrew Burgess (Code Review)
2019-11-19 13:11 ` Tom Tromey (Code Review)
2019-11-19 20:36 ` [review v4] " Tom Tromey (Code Review)
2019-11-26 23:52 ` Andrew Burgess (Code Review)
2019-12-11 23:21 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-11 23:21 ` Sourceware to Gerrit sync (Code Review)

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