[binutils-gdb] gdb: fix line wrapping in new boxed hints when styling is enabled

Andrew Burgess aburgess@sourceware.org
Wed Jan 7 13:08:42 GMT 2026


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=de364c377b7416e15ba8aeada451757db9c541cc

commit de364c377b7416e15ba8aeada451757db9c541cc
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Dec 15 16:27:37 2025 +0000

    gdb: fix line wrapping in new boxed hints when styling is enabled
    
    I noticed that the startup hint text, the stuff that's placed into a
    box, is not line wrapping correctly.  For example:
    
      $ gdb -nw -nh -eiex 'set width 60'
      ... snip ...
    
      ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
      ┃ Find the GDB manual online at:                           ┃
      ┃ http://www.gnu.org/software/gdb/documentation/.          ┃
      ┃ For help, type "help".                                   ┃
      ┃ Type "apropos <word>" to search                          ┃
      ┃  for commands related to <word>                          ┃
      ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
    
      (gdb)
    
    Notice the final two lines within the box, there's no need to wrap
    after the word 'search', plenty more would fit on that line.  And
    indeed, if we switch off styling:
    
      $ gdb -nw -nh -eiex 'set width 60' -eiex 'set style enabled off'
      ... snip ...
    
      +----------------------------------------------------------+
      | Find the GDB manual online at:                           |
      | http://www.gnu.org/software/gdb/documentation/.          |
      | For help, type "help".                                   |
      | Type "apropos <word>" to search for commands related to  |
      |  <word>                                                  |
      +----------------------------------------------------------+
    
      (gdb)
    
    That's mostly fixed it, except I still think there's a stray white
    space character before '<word>' on the final line.
    
    The fact that the output is wrapped differently with styling on and
    off tells me that this is not an intentional choice.
    
    The problems are, I think all in box_one_message (from top.c).  There
    are a number of issues.
    
     1. Each time around the loop we count all escape characters in
        MESSAGE, not just the escape characters up to the point where we
        might wrap the message.  This has the potential to over count the
        escape characters.
    
     2. When splitting MESSAGE we search for a space character.  This
        search starts based on the terminal width, but neglects to take
        into account any escape characters prior to the split point.
    
     3. If we split a line after an alternative style has been activated,
        but before the style has been reset, then the border of the box on
        that line is going to be rendered in the alternative style.
    
     4. When computing what content needs to be moved onto the second
        line, we don't move past the space character (as found in point
        2).
    
    Now it just so happens that issues (1) and (3) can be ignored for now.
    The surrounding box is only added (and line wrapping performed) if the
    terminal is at least wide enough to fit the documentation URL (plus
    box borders).  This means a minimum width of 51 characters. On all
    the other lines, any styled output is always to the left of the line,
    occurring before character 51.  This means that counting all escape
    characters is the same as counting just the escape characters that
    will appear before the line break.  And we will never need to line
    break part way through some styled text.
    
    Obviously we _could_ fix this code properly, but it's not simple, and
    it would all be completely theoretical.  So in this commit I plan to
    add an assert that all escape sequences occur within the first 51
    characters, then if the above text ever changes we will immediately
    know that box_one_message will need to be rewritten.
    
    Fixing issue (2) is pretty easy, this line:
    
      line = message.substr (0, message.rfind (" ", width));
    
    just needs to be updated to take N_ESCAPE_CHARS into account.  We
    currently look for a space after WIDTH characters in MESSAGE, but
    MESSAGE also contains escape sequences, so we really need to search in
    for a space starting from 'WIDTH + N_ESCAPE_CHARS'.
    
    And fixing (4) is also easy, this line:
    
      message = message.substr (line.length ());
    
    finds the remainder of MESSAGE based on LINE.  But LINE was all
    content up to (but not including) the space character we found.  What
    we actually need to do is:
    
      message = message.substr (line.length () + 1);
    
    To add the assert that I discussed above, I've moved the escape
    characters counting code out of the line printing loop.  We now count
    the escape characters just once, and assert that these all fit within
    the WIDTH, this means they will all appear before any line break.
    
    While making these changes I've also made use of std::move to avoid
    copying a string in one place.
    
    Finally, the gdb.base/startup-hints.exp test has been expanded to
    cover both styled and non-styled output, as well as a greater range of
    terminal widths.

Diff:
---
 gdb/testsuite/gdb.base/startup-hints.exp | 226 ++++++++++++++++++++++---------
 gdb/top.c                                |  62 ++++++---
 2 files changed, 207 insertions(+), 81 deletions(-)

diff --git a/gdb/testsuite/gdb.base/startup-hints.exp b/gdb/testsuite/gdb.base/startup-hints.exp
index 717b147c429..2290a34eeaa 100644
--- a/gdb/testsuite/gdb.base/startup-hints.exp
+++ b/gdb/testsuite/gdb.base/startup-hints.exp
@@ -26,79 +26,162 @@ if { [build_executable "build" $testfile $srcfile] != 0 } {
     return
 }
 
+# Take a list of PARTS and combine them into a single string, which is then
+# returned.
+#
+# Each part in PARTS is itself a list with two elements, the first is the
+# name of a style (see TCL proc 'style'), and the second is a literal piece
+# of text to which the style should be applied.
+#
+# When STYLED is true then the style name from each part is used to add
+# escape sequences to the result string.  When STYLED is false no style
+# escapes are added.
+#
+# When REGEXP is true then the literal text for each part is converted to a
+# regexp (i.e. escape characters are added as needed) within the returned
+# string.
+proc parts_to_string { styled regexp parts } {
+    set str ""
+
+    foreach p $parts {
+	if { $styled } {
+	    set style [lindex $p 0]
+	} else {
+	    set style none
+	}
+
+	set text [lindex $p 1]
+	if { $regexp } {
+	    set text [string_to_regexp $text]
+	}
+
+	append str [style $text $style]
+    }
+
+    return $str
+}
+
 # Return a single regexp string for the startup hint text when the
 # terminal width is WIDTH.  For narrow terminals GDB will just print
 # the hint text.  For wider terminals GDB places the hint into a box.
-proc build_hint_re { width } {
+#
+# When WITH_STYLE is true then the output is expected to be styled, and
+# escape sequences will be added to the result to take this into account.
+proc build_hint_re { width with_style } {
     # GDB imposes a maximum width of 80.
     if { $width > 80 || $width == 0 } {
 	set width 80
     }
 
-    if { $width > 50 } {
-
-	# These lines are shorter than 50 characters, and so are never
-	# wrapped.
-	set msg {
-	    {Find the GDB manual online at:}
-	    {http://www.gnu.org/software/gdb/documentation/.}
-	    {For help, type "help".}
-	}
-
-	# The final line is wrapped based on the terminal width.
-	if { $width > 66 } {
-	    lappend msg {Type "apropos <word>" to search for commands related to <word>.}
-	} elseif { $width > 58 } {
-	    lappend msg {Type "apropos <word>" to search for commands related to} {<word>.}
-	} elseif { $width > 55 } {
-	    lappend msg {Type "apropos <word>" to search for commands related} {to <word>.}
-	} elseif { $width > 47 } {
-	    lappend msg {Type "apropos <word>" to search for commands} { related to <word>.}
-	}
+    # Build a list of lines in MSG.  Each line is stored as a list of parts.
+    # Each part is a two item list, the first item is a style name (see the
+    # TCL 'style' proc), and the second item is some text.  Thus a single
+    # line looks like:
+    #
+    # { { STYLE TEXT } { STYLE TEXT } { STYLE TEXT } }
+    #
+    # The final value of MSG will be a list of these lines.
+    #
+    # We do things this way because we need to do two different things with
+    # the contents of MSG, first, we need to calculate the total length of
+    # each line, as seen in GDB's output.  For this we need to get the sum
+    # of the lengths of each TEXT part.  With this length we can then
+    # calculate the ammount of padding needed for a given terminal width.
+    #
+    # The second use of MSG is to build the regexp that is used to actually
+    # match GDB's output.  For this we use STYLE to add escape sequences to
+    # the output, and each TEXT is converted to a regexp, which might add
+    # escape sequences.
+    #
+    # The first 3 lines in MSG are never line wrapped.  The documentation
+    # URL is used by GDB as the minimum required width for adding a box
+    # around the hint.
+    set msg {
+	{{none {Find the GDB manual online at:}}}
+	{{file {http://www.gnu.org/software/gdb/documentation/}} {none {.}}}
+	{{none {For help, type "}} {command {help}} {none {".}}}}
+
+    # The final line is longer than the documentation URL, and so it will be
+    # line wrapped based on the terminal width.  The documentation URL
+    # defines a minimum width at which a box will be drawn as 51 characters,
+    # as such the number of places the final line can be wrapped is small
+    # enough that we just define each possibility.
+    #
+    # For terminals less than 51 characters no box is added, and GDB just
+    # relies on the terminal to wrap the output for us, so in this case we
+    # don't need to manually split the line here.
+    if { $width > 66 || $width < 51 } {
+	lappend msg \
+	    {{none {Type "}}
+	     {command {apropos <word>}}
+	     {none {" to search for commands related to <word>.}}}
+    } elseif { $width > 58 } {
+	lappend msg \
+	    {{none {Type "}}
+	     {command {apropos <word>}}
+	     {none {" to search for commands related to}}} \
+	    {{none {<word>.}}}
+    } elseif { $width > 55 } {
+	lappend msg \
+	    {{none {Type "}}
+	     {command {apropos <word>}}
+	     {none {" to search for commands related}}}	 \
+	    {{none {to <word>.}}}
+    } elseif { $width > 47 } {
+	lappend msg \
+	    {{none {Type "}}
+	     {command {apropos <word>}}
+	     {none {" to search for commands}}} \
+	    {{none {related to <word>.}}}
+    }
 
-	# Place the lines into a box, padding with whitespace so that
-	# the sides are correctly aligned.
-	set top_bottom "+[string repeat "-" [expr {$width - 2}]]+"
+    if { $width > 50 } {
+	# For terminal widths greater than 50, place the hint text into a
+	# box, padding with whitespace so that the sides are correctly
+	# aligned.
+	#
+	# We convert each line in MSG to a string twice.  The first time no
+	# styling is applied, nor do we convert the parts of the line into a
+	# regexp, this allows the actual length of the line (as seen in
+	# GDB's output) to be calculated, and from this we calculate the
+	# padding required.
+	#
+	# After that we create the line from MSG a second time, only this
+	# time, styling is added when WITH_STYLE is true, and each part of
+	# the line is converted to a regexp, which might add escape
+	# characters.
+	set top_bottom [string_to_regexp "+[string repeat "-" [expr {$width - 2}]]+"]
 	set lines [list $top_bottom]
+	set side [string_to_regexp "|"]
 	foreach m $msg {
-	    set space_count [expr {$width - 4 - [string length $m]}]
+	    set plain [parts_to_string false false $m]
+	    set space_count [expr {$width - 4 - [string length $plain]}]
 	    set spaces [string repeat " " $space_count]
-	    lappend lines "| $m$spaces |"
+	    set maybe_styled [parts_to_string $with_style true $m]
+	    lappend lines "$side $maybe_styled$spaces $side"
 	}
 	lappend lines $top_bottom
     } else {
-	# We tell GDB that the terminal width is WIDTH, but in reality
-	# the actual terminal width is unlimited.  As such, GDB drops
-	# the box around the hint, but doesn't inject any additional
-	# newlines (it assumes the terminal will break the lines for
-	# us).  This is why, despite the narrow terminal, we expect
-	# each line without any line breaks.
-	set lines {{Find the GDB manual online at:} \
-		       {http://www.gnu.org/software/gdb/documentation/.} \
-		       {For help, type "help".} \
-		       {Type "apropos <word>" to search for commands related to <word>.}}
+	# For narrow terminals no box is added.  Just convert each line in
+	# MSG to a regexp, adding style escape sequences when WITH_STYLE is
+	# true.
+	set lines {}
+	foreach m $msg {
+	    lappend lines [parts_to_string $with_style true $m]
+	}
     }
 
     # Add blank line before and after current lines.
     set lines [linsert $lines 0 ""]
     lappend lines ""
 
-    # Convert every line to a regexp.  Also log the expected output
-    # for debugging.
-    set lines_re {}
-    verbose -log -- "Expected message format:"
-    foreach l $lines {
-	verbose -log -- "$l"
-	lappend lines_re [string_to_regexp $l]
-    }
-
     # Join the regexp together.
-    return [multi_line {*}$lines_re]
+    return [multi_line {*}$lines]
 }
 
 # Tell GDB to start with a terminal width of WIDTH, then start GDB.
 # Check that the hint text is formatted correctly.
-proc_with_prefix test_for_hint_with_width { width load_exec } {
+proc_with_prefix test_for_hint_with_width { width load_exec with_style } {
     global GDBFLAGS
 
     save_vars { GDBFLAGS } {
@@ -107,13 +190,26 @@ proc_with_prefix test_for_hint_with_width { width load_exec } {
 	    append GDBFLAGS " \"$::binfile\""
 	}
 	gdb_exit
-	gdb_spawn
+
+	if { $with_style } {
+	    with_ansi_styling_terminal {
+		gdb_spawn
+	    }
+	} else {
+	    gdb_spawn
+	}
     }
 
-    set hint_re [build_hint_re $width]
+    set hint_re [build_hint_re $width $with_style]
 
     if { $load_exec } {
-	append hint_re "\r\nReading symbols from [string_to_regexp $::binfile]\\.\\.\\."
+	if { $with_style } {
+	    set style file
+	} else {
+	    set style none
+	}
+
+	append hint_re "\r\nReading symbols from [style [string_to_regexp $::binfile] $style]\\.\\.\\."
     }
 
     gdb_test "" $hint_re \
@@ -123,22 +219,24 @@ proc_with_prefix test_for_hint_with_width { width load_exec } {
 save_vars { INTERNAL_GDBFLAGS } {
     set INTERNAL_GDBFLAGS [string map {"-q" ""} $INTERNAL_GDBFLAGS]
 
-    foreach_with_prefix load_exec { false true } {
+    foreach_with_prefix with_style { false true } {
+	foreach_with_prefix load_exec { false true } {
 
-	# Width 0 actually means unlimited.  The other small sizes
-	# check that GDB doesn't trigger undefined behaviour by trying
-	# to create strings with a negative length.
-	for { set width 0 } { $width <= 5 } { incr width } {
-	    test_for_hint_with_width $width $load_exec
-	}
+	    # Width 0 actually means unlimited.  The other small sizes
+	    # check that GDB doesn't trigger undefined behaviour by trying
+	    # to create strings with a negative length.
+	    for { set width 0 } { $width <= 5 } { incr width } {
+		test_for_hint_with_width $width $load_exec $with_style
+	    }
 
-	# These widths cover the point where we transition from using
-	# an unboxed hint to a boxed hint.
-	for { set width 45 } { $width <= 55 } { incr width } {
-	    test_for_hint_with_width $width $load_exec
-	}
+	    # These widths cover the point where we transition from using
+	    # an unboxed hint to a boxed hint.
+	    for { set width 45 } { $width <= 60 } { incr width } {
+		test_for_hint_with_width $width $load_exec $with_style
+	    }
 
-	# Very large widths are treated like a width of 80.
-	test_for_hint_with_width 100 $load_exec
+	    # Very large widths are treated like a width of 80.
+	    test_for_hint_with_width 100 $load_exec $with_style
+	}
     }
 }
diff --git a/gdb/top.c b/gdb/top.c
index 0f9fedf18f6..e0ae0689c04 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1377,35 +1377,63 @@ static void
 box_one_message (ui_file *stream, std::string message, int width)
 {
   const char *wall = emojis_ok () ? bd_heavy_vertical : "|";
+
+  /* We know that the box we are displaying the text in is at least as
+     long as the documentation URL.
+
+     Additionally, we know that on non-URL lines, any styling occurs in the
+     first part of the line, before we might need to line wrap.  We take
+     advantage of this to simplify this function slightly.
+
+     First, count the total number of escape characters on this line.  */
+  int n_escape_chars = 0;
+  const char *escape = message.c_str ();
+  while ((escape = strchr (escape, '\033')) != nullptr)
+    {
+      int tmp;
+      if (skip_ansi_escape (escape, &tmp))
+	n_escape_chars += tmp;
+      else
+	gdb_assert_not_reached ("unknown escape sequence");
+      escape += tmp;
+    }
+
+  /* ESCAPE points to the first character after the last escape sequence in
+     MESSAGE.  The number of non-escape sequence characters up to this
+     point should always be less than width.  If this fails then we need to
+     wrap the escape sequence block, and this function isn't written with
+     that in mind.  */
+  gdb_assert ((escape - message.c_str () - n_escape_chars) < width);
+
   while (!message.empty ())
     {
       std::string line;
-      int n_escape_chars = 0;
-      const char *escape = message.c_str ();
-      while ((escape = strchr (escape, '\033')) != nullptr)
-	{
-	  int tmp;
-	  if (skip_ansi_escape (escape, &tmp))
-	    n_escape_chars += tmp;
-	  else
-	    break;
-	  escape += tmp;
-	}
       if ((message.length () - n_escape_chars) > width)
 	{
-	  line = message.substr (0, message.rfind (" ", width));
-	  message = message.substr (line.length ());
+	  /* Place the left hand part of MESSAGE into LINE.  Take account
+	     of any escape characters in MESSAGE.  */
+	  line = message.substr (0, message.rfind (' ',
+						   width + n_escape_chars));
+
+	  /* The '+ 1' here skips the space that rfind just found.  */
+	  message = message.substr (line.length () + 1);
 	}
       else
-	{
-	  line = message;
-	  message = "";
-	}
+	line = std::move (message);
 
+      /* Pad LINE with spaces so that the right border is in the correct
+	 place.  */
       if ((line.length () - n_escape_chars) < width)
 	line.append (width - line.length () + n_escape_chars, ' ');
 
       gdb_printf (stream, "%s %s %s\n", wall, line.c_str (), wall);
+
+      /* As we know all escape characters fall in the first part of the
+	 line (before any wrapping), then after printing the first part of
+	 the line (which might be all of the line), we know there are no
+	 remaining escape characters.  If there were then the assert above
+	 this loop would have triggered.  */
+      n_escape_chars = 0;
     }
 }


More information about the Gdb-cvs mailing list