Truncate long TUI window titles
Commit Message
If a TUI window has a long title, it can overflow the title line.
This changes the TUI to use just the tail part of the title in this
case.
gdb/ChangeLog
2019-08-30 Tom Tromey <tom@tromey.com>
* tui/tui-wingeneral.c (box_win): Truncate long window titles.
gdb/testsuite/ChangeLog
2019-08-30 Tom Tromey <tom@tromey.com>
* gdb.tui/resize.exp: Remove setup_xfail.
* gdb.tui/regs.exp: Remove setup_xfail.
* gdb.tui/basic.exp: Remove setup_xfail.
---
gdb/ChangeLog | 4 ++++
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.tui/basic.exp | 6 ------
gdb/testsuite/gdb.tui/regs.exp | 3 ---
gdb/testsuite/gdb.tui/resize.exp | 2 --
gdb/tui/tui-wingeneral.c | 16 +++++++++++++++-
6 files changed, 25 insertions(+), 12 deletions(-)
Comments
* Tom Tromey <tom@tromey.com> [2019-08-30 15:28:34 -0600]:
> If a TUI window has a long title, it can overflow the title line.
> This changes the TUI to use just the tail part of the title in this
> case.
>
> gdb/ChangeLog
> 2019-08-30 Tom Tromey <tom@tromey.com>
>
> * tui/tui-wingeneral.c (box_win): Truncate long window titles.
>
> gdb/testsuite/ChangeLog
> 2019-08-30 Tom Tromey <tom@tromey.com>
>
> * gdb.tui/resize.exp: Remove setup_xfail.
> * gdb.tui/regs.exp: Remove setup_xfail.
> * gdb.tui/basic.exp: Remove setup_xfail.
Nice! I had just one small question...
> ---
> gdb/ChangeLog | 4 ++++
> gdb/testsuite/ChangeLog | 6 ++++++
> gdb/testsuite/gdb.tui/basic.exp | 6 ------
> gdb/testsuite/gdb.tui/regs.exp | 3 ---
> gdb/testsuite/gdb.tui/resize.exp | 2 --
> gdb/tui/tui-wingeneral.c | 16 +++++++++++++++-
> 6 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.tui/basic.exp b/gdb/testsuite/gdb.tui/basic.exp
> index f60011c386a..716c52f68a6 100644
> --- a/gdb/testsuite/gdb.tui/basic.exp
> +++ b/gdb/testsuite/gdb.tui/basic.exp
> @@ -35,9 +35,6 @@ gdb_assert {![string match "No Source Available" $text]} \
> Term::command "list main"
> Term::check_contents "list main" "21 *return 0"
>
> -# This check fails because the file name in the title overwrites the
> -# box.
> -setup_xfail *-*-*
> Term::check_box "source box" 0 0 80 15
>
> Term::command "layout asm"
> @@ -48,8 +45,5 @@ Term::check_box "asm box" 0 0 80 15
> Term::command "layout split"
> Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
>
> -# This check fails because the file name in the title overwrites the
> -# box.
> -setup_xfail *-*-*
> Term::check_box "source box in split layout" 0 0 80 8
> Term::check_box "asm box in split layout" 0 7 80 8
> diff --git a/gdb/testsuite/gdb.tui/regs.exp b/gdb/testsuite/gdb.tui/regs.exp
> index f17b1db8d40..1af943dd152 100644
> --- a/gdb/testsuite/gdb.tui/regs.exp
> +++ b/gdb/testsuite/gdb.tui/regs.exp
> @@ -38,9 +38,6 @@ Term::check_contents "source at startup" ">|21 *return 0"
>
> Term::command "layout regs"
> Term::check_box "register box" 0 0 80 8
> -# This check fails because the file name in the title overwrites the
> -# box.
> -setup_xfail *-*-*
> Term::check_box "source box in regs layout" 0 7 80 8
>
> set text [Term::get_line 1]
> diff --git a/gdb/testsuite/gdb.tui/resize.exp b/gdb/testsuite/gdb.tui/resize.exp
> index 8484e03c717..3b885e1198e 100644
> --- a/gdb/testsuite/gdb.tui/resize.exp
> +++ b/gdb/testsuite/gdb.tui/resize.exp
> @@ -37,6 +37,4 @@ if {![Term::enter_tui]} {
> Term::check_contents "source at startup" ">|21 *return 0"
>
> Term::resize 40 90
> -# Resizing seems to be somewhat broken.
> -setup_xfail *-*-*
> Term::check_box "source box after resize" 0 0 90 23
> diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
> index f900eab0133..0291bf1a335 100644
> --- a/gdb/tui/tui-wingeneral.c
> +++ b/gdb/tui/tui-wingeneral.c
> @@ -74,7 +74,21 @@ box_win (struct tui_win_info *win_info,
> box (win, tui_border_vline, tui_border_hline);
> #endif
> if (!win_info->title.empty ())
> - mvwaddstr (win, 0, 3, win_info->title.c_str ());
> + {
> + /* 2 chars for the corners, plus 3 more for the indent that's
> + used below. */
> + int max_len = win_info->width - 2 - 3;
I didn't really understand this comment. Currently long titles will
be laid out like this: +--...long title-+
You claim 2 for the corners, and 3 for the indent, but this seems an
odd way of describing it, why not just say 3 for the left edge and 2
for the right edge?
Also, is there a reason why you went with a 3/2 split for the top
border rather than having a balanced 3/3 or 2/2?
Thanks,
Andrew
> +
> + if (win_info->title.size () <= max_len)
> + mvwaddstr (win, 0, 3, win_info->title.c_str ());
> + else
> + {
> + std::string truncated
> + = "..." + win_info->title.substr (win_info->title.size ()
> + - max_len + 3);
> + mvwaddstr (win, 0, 3, truncated.c_str ());
> + }
> + }
> wattroff (win, attrs);
> }
>
> --
> 2.17.2
>
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>> + /* 2 chars for the corners, plus 3 more for the indent that's
>> + used below. */
>> + int max_len = win_info->width - 2 - 3;
Andrew> I didn't really understand this comment. Currently long titles will
Andrew> be laid out like this: +--...long title-+
Andrew> You claim 2 for the corners, and 3 for the indent, but this seems an
Andrew> odd way of describing it, why not just say 3 for the left edge and 2
Andrew> for the right edge?
No reason, I will change it.
Andrew> Also, is there a reason why you went with a 3/2 split for the top
Andrew> border rather than having a balanced 3/3 or 2/2?
No particularly good reason. I didn't change the left hand side to
preserve the historical decision. On the right I thought it would be
good to use as much space as possible, but still leave one "-" to make
it still resemble a line. 3 would work just as well -- I just didn't
give it a whole lot of consideration.
One thing I did consider is trying harder to truncate the file name at a
directory boundary. This can always be done if desirable, though.
Tom
* Tom Tromey <tom@tromey.com> [2019-09-03 17:33:33 -0600]:
> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
>
> >> + /* 2 chars for the corners, plus 3 more for the indent that's
> >> + used below. */
> >> + int max_len = win_info->width - 2 - 3;
>
> Andrew> I didn't really understand this comment. Currently long titles will
> Andrew> be laid out like this: +--...long title-+
>
> Andrew> You claim 2 for the corners, and 3 for the indent, but this seems an
> Andrew> odd way of describing it, why not just say 3 for the left edge and 2
> Andrew> for the right edge?
>
> No reason, I will change it.
Thanks, I just got confused as I assumed "indent below" was the
"...". I get confused easily!
>
> Andrew> Also, is there a reason why you went with a 3/2 split for the top
> Andrew> border rather than having a balanced 3/3 or 2/2?
>
> No particularly good reason. I didn't change the left hand side to
> preserve the historical decision. On the right I thought it would be
> good to use as much space as possible, but still leave one "-" to make
> it still resemble a line. 3 would work just as well -- I just didn't
> give it a whole lot of consideration.
I don't really mind, it just looked a bit weird and I thought there
might be an off-by-one bug in the code. I'd personally be happy with a
2/2 split.
In fact, given that no (sane) person is going to be pattern matching
against the TUI output we could be a little more dynamic. What if we
kept the 3 on the left for short titles, but moved to 2 of the left
once titles get long enough?
But meh, the patch works for me, and is a million times better than
before, so I'm happy with whatever you choose to push.
Thanks,
Andrew
>
> One thing I did consider is trying harder to truncate the file name at a
> directory boundary. This can always be done if desirable, though.
>
> Tom
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> I don't really mind, it just looked a bit weird and I thought there
Andrew> might be an off-by-one bug in the code. I'd personally be happy with a
Andrew> 2/2 split.
I changed it to 2/2, and I updated the comment. I'll push soon.
Tom
@@ -35,9 +35,6 @@ gdb_assert {![string match "No Source Available" $text]} \
Term::command "list main"
Term::check_contents "list main" "21 *return 0"
-# This check fails because the file name in the title overwrites the
-# box.
-setup_xfail *-*-*
Term::check_box "source box" 0 0 80 15
Term::command "layout asm"
@@ -48,8 +45,5 @@ Term::check_box "asm box" 0 0 80 15
Term::command "layout split"
Term::check_contents "split layout contents" "21 *return 0.*$hex <main>"
-# This check fails because the file name in the title overwrites the
-# box.
-setup_xfail *-*-*
Term::check_box "source box in split layout" 0 0 80 8
Term::check_box "asm box in split layout" 0 7 80 8
@@ -38,9 +38,6 @@ Term::check_contents "source at startup" ">|21 *return 0"
Term::command "layout regs"
Term::check_box "register box" 0 0 80 8
-# This check fails because the file name in the title overwrites the
-# box.
-setup_xfail *-*-*
Term::check_box "source box in regs layout" 0 7 80 8
set text [Term::get_line 1]
@@ -37,6 +37,4 @@ if {![Term::enter_tui]} {
Term::check_contents "source at startup" ">|21 *return 0"
Term::resize 40 90
-# Resizing seems to be somewhat broken.
-setup_xfail *-*-*
Term::check_box "source box after resize" 0 0 90 23
@@ -74,7 +74,21 @@ box_win (struct tui_win_info *win_info,
box (win, tui_border_vline, tui_border_hline);
#endif
if (!win_info->title.empty ())
- mvwaddstr (win, 0, 3, win_info->title.c_str ());
+ {
+ /* 2 chars for the corners, plus 3 more for the indent that's
+ used below. */
+ int max_len = win_info->width - 2 - 3;
+
+ if (win_info->title.size () <= max_len)
+ mvwaddstr (win, 0, 3, win_info->title.c_str ());
+ else
+ {
+ std::string truncated
+ = "..." + win_info->title.substr (win_info->title.size ()
+ - max_len + 3);
+ mvwaddstr (win, 0, 3, truncated.c_str ());
+ }
+ }
wattroff (win, attrs);
}