[Bug gdb/31033] New: Bogus bold at the end of command when quitting pagination early

[Bug gdb/31033] New: Bogus bold at the end of command when quitting pagination early

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

            Bug ID: 31033
           Summary: Bogus bold at the end of command when quitting
                    pagination early
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: blarsen at redhat dot com
  Target Milestone: ---

When a command has bold in its output, triggers pagination, and the user quits
pagination early, the next command will have bold at the end and the prompt
will be printed in bold as well. See this sample session, using <bold></bold>
to show where the output is stylized:

$ ./gdb -q cat     
Reading symbols from cat...
Reading symbols from .gnu_debugdata for /usr/bin/cat...
(No debugging symbols found in .gnu_debugdata for /usr/bin/cat)
(gdb) set height 2
(gdb) apropos inferior
<bold> add-inferior </bold> -- Add a new inferior.
--Type <RET> for more, q to quit, c to continue without paging--q
Quit
(gdb) inferior
[Current inferior is 1 <bold>[<null>] (/usr/bin/cat)]
(gdb)</bold> inferior
[Current inferior is 1 [<null>] (/usr/bin/cat)]
(gdb) 

This is a regression, since this behavior isn't present in GDB 13.2-3 in Fedora
37.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |32284
                 CC|                            |tromey at sourceware dot org


Referenced Bugs:

https://sourceware.org/bugzilla/show_bug.cgi?id=32284
[Bug 32284] [meta] CLI Styling
-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |tromey at sourceware dot org

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
I have a fix.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
I should probably write a regression test before sending this though.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

--- Comment #3 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

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

commit e7b7270ace7c64bbd252d9a152ae541fc28b734f
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Jun 16 17:20:57 2025 +0100

    gdb: styling fixes around and for the pagination prompt

    This commit fixes a couple of issues relating to the pagination
    prompt and styling.  The pagination prompt is this one:

      --Type <RET> for more, q to quit, c to continue without paging--

    I did try to split this into multiple patches, based on the three
    issues I describe below, but in the end, the fixes were all too
    interconnected, so it ended up as one patch that makes two related,
    but slightly different changes:

      1. Within the pager_file class, relying on the m_applied_style
      attribute of the wrapped m_stream, as is done when calling
      m_stream->emit_style_escape, is not correct, so stop doing that, and

      2. Failing to update m_applied_style within the pager_file class can
      leave that attribute out of date, which can then lead to styling
      errors later on, so ensure m_applied_style is always updated.

    The problems I have seen are:

      1. After quitting from a pagination prompt, the next command can
      incorrectly style its output.  This was reported as bug PR
      gdb/31033, and is fixed by this commit.

      2. The pagination prompt itself could be styled.  The pagination
      prompt should always be shown in the default style.

      3. After continuing the output at a pagination prompt, GDB can fail
      to restore the default style the next time the output (within the
      same command) switches back to the default style.

    There are tests for all these issues as part of this patch.

    The pager_file class is a sub-class of wrapped_file, this means that a
    pager_file is itself a ui_file, while it also manages a pointer to a
    ui_file object (called m_stream).  An instance of pager_file can be
    installed as the gdb_stdout ui_file object.

    Output sent to a pager_file is stored within an internal
    buffer (called m_wrap_buffer) until we have a complete line, when the
    content is flushed to the wrapped m_stream.  If sufficient lines have
    been written out then the pager_file will present the pagination
    prompt and allow the user to continue viewing output, or quit the
    current command.

    As a pager_file is a ui_file, it has an m_applied_style member
    variable.

    The managed stream (m_stream) is also a ui_file, and so also has an
    m_applied_style member variable.

    In some places within the pager_file class we attempt to change the
    current style of the m_stream using calls like this:

      m_stream->emit_style_escape (style);

    See pager_file::emit_style_escape, pager_file::prompt_for_continue,
    and pager_file::puts.  These calls will end up in
    ui_file::emit_style_escape, which tries to skip emitting unnecessary
    style escapes by checking if the requested style matches the current
    m_applied_style value.

    The m_applied_style value is updated by calls to the emit_style_escape
    function.

    The problem here is that most of the time pager_file doesn't change
    the style of m_stream by calling m_stream->emit_style_escape.  Most of
    the time, style changes are performed by pager_file writing the escape
    sequence into m_wrap_buffer, and then later flushing this buffer to
    m_stream by calling m_stream->puts.

    It has to be done this way.  Calling m_stream->emit_style_escape
    would, if it actually changed the style, immediately change the style
    by emitting an escape sequence.  But pager_file doesn't want that, it
    wants the style change to happen later, when m_wrap_buffer is
    flushed.

    To avoid excessive style escape sequences being written into
    m_wrap_buffer, the pager_file::m_applied_style performs a function
    similar to the m_applied_style within m_stream, it tracks the current
    style for the end of m_wrap_buffer, and only allows style escape
    sequences to be emitted if the style is actually changing.

    However, a consequence of this is the m_applied_style within m_stream,
    is not updated, which means it will be out of sync with the actual
    current style of m_stream.  If we then try to make a call to
    m_stream->emit_style_escape, if the style we are changing too happens
    to match the out of date style in m_stream->m_applied_style, then the
    style change will be ignored.

    And this is indeed what we see in pager_file::prompt_for_continue with
    the call:

      m_stream->emit_style_escape (ui_file_style ());

    As m_stream->m_applied_style is not being updated, it will always be
    the default style, however m_stream itself might not actually be in
    the default style.  This call then will not emit an escape sequence as
    the desired style matches the out of date m_applied_style.

    The fix in this case is to call m_stream->puts directly, passing in
    the escape sequence for the desired style.  This will result in an
    immediate change of style for m_stream, which fixes some of the
    problems described above.

    In fact, given that m_stream's m_applied_style is always going to be
    out of sync, I think we should change all of the
    m_stream->emit_style_escape calls to instead call m_stream->puts.

    However, just changing to use puts doesn't fix all the problems.

    I found that, if I run 'apropos time', then quit at the first
    pagination prompt.  If for the next command I run 'maintenance time' I
    see the expected output:

      "maintenance time" takes a numeric argument.

    However, everything after the first double quote is given the command
    name style rather than only styling the text between the double
    quotes.

    Here is GDB's stack while printing the above output:

      #2  0x0000000001050d56 in ui_out::vmessage (this=0x7fff1238a150,
in_style=..., format=0x1c05af0 "", args=0x7fff1238a288) at
../../src/gdb/ui-out.c:754
      #3  0x000000000104db88 in ui_file::vprintf (this=0x3f9edb0,
format=0x1c05ad0 "\"%ps\" takes a numeric argument.\n", args=0x7fff1238a288) at
../../src/gdb/ui-file.c:73
      #4  0x00000000010bc754 in gdb_vprintf (stream=0x3f9edb0, format=0x1c05ad0
"\"%ps\" takes a numeric argument.\n", args=0x7fff1238a288) at
../../src/gdb/utils.c:1905
      #5  0x00000000010bca20 in gdb_printf (format=0x1c05ad0 "\"%ps\" takes a
numeric argument.\n") at ../../src/gdb/utils.c:1945
      #6  0x0000000000b6b29e in maintenance_time_display (args=0x0, from_tty=1)
at ../../src/gdb/maint.c:128

    The interesting frames here are #3, in here `this` is the pager_file
    for GDB's stdout, and this passes its m_applied_style to frame #2 as
    the `in_style` argument.

    If the m_applied_style is wrong, then frame #2 will believe that the
    wrong style is currently in use as the default style, and so, after
    printing 'maintenance time' GDB will switch back to the wrong style.

    So the question is, why is pager_file::m_applied_style wrong?

    In pager_file::prompt_for_continue, there is an attempt to switch back
    to the default style using:

      m_stream->emit_style_escape (ui_file_style ());

    If this is changed to a puts call (see above) then this still leaves
    pager_file::m_applied_style out of date.

    The right fix in this case is, I think, to instead do this:

      this->emit_style_escape (ui_file_style ());

    this will update pager_file::m_applied_style, and also send the
    default style to m_stream using a puts call.

    While writing the tests I noticed that I was getting unnecessary style
    reset sequences emitted.

    The problem is that, around pagination, we don't really know what
    style is currently applied to m_stream.  The
    pager_file::m_applied_style tracks the style at the end of
    m_wrap_buffer, but this can run ahead of the current m_stream style.
    For example, if the screen is currently full, such that the next
    character of output will trigger the pagination prompt, if the next
    call is actually to pager_file::emit_style_escape, then
    pager_file::m_applied_style will be updated, but the style of m_stream
    will remain unchanged.  When the next character is written to
    pager_file::puts then the pagination prompt will be presented, and GDB
    will try to switch m_stream back to the default style.  Whether an
    escape is emitted or not will depend on the m_applied_style value,
    which we know is different than the actual style of m_stream.

    It is, after all, only when m_wrap_buffer is flushed to m_stream that
    the style of m_stream actually change.

    And so, this commit also adds pager_file::m_stream_style.  This new
    variable tracks the current style of m_stream.  This really is a
    replacement for m_stream's ui_file::m_applied_style, which is not
    accessible from pager_file.

    When content is flushed from m_wrap_buffer to m_stream then the
    current value of pager_file::m_applied_style becomes the current style
    of m_stream.  But, when m_wrap_buffer is filling up, but before it is
    flushed, then pager_file::m_applied_style can change, but
    m_stream_style will remain unchanged.

    Now in pager_file::emit_style_escape we are able to skip some of the
    direct calls to m_stream->puts() used to emit style escapes.

    After all this there are still a few calls to
    m_stream->emit_style_escape().  These are all in the wrap_here support
    code.  I think that these calls are technically broken, but don't
    actually cause any issues due to the way styling works in GDB.  I
    certainly haven't been able to trigger any bugs from these calls yet.
    I plan to "fix" these in the next commit just for completeness.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31033

    Approved-By: Tom Tromey <tom@tromey.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[aburgess at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |aburgess at redhat dot com
         Resolution|---                         |FIXED

--- Comment #4 from Andrew Burgess <aburgess at redhat dot com> ---
This should now be fixed on master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/31033] Bogus bold at the end of command when quitting pagination early

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31033

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |17.1

-- 
You are receiving this mail because:
You are on the CC list for the bug.