[review] First use of tui_layout
Commit Message
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(-)
Comments
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.
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.
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!
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.
@@ -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)
@@ -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.
@@ -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
@@ -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.
@@ -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);
-}
-
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);
+}
+
/* 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 ();
}
@@ -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 */