[Bug cli/7234] Re-do GDB's output pager.

[Bug cli/7234] Re-do GDB's output pager.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dje at google dot com

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
*** Bug 17222 has been marked as a duplicate of this bug. ***

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

[Bug cli/7234] Re-do GDB's output pager.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

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

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
I looked into this a bit.  There are a lot of bad uses of unfiltered
output -- tedious but easy to fix, I've got a bunch of patches for that.

The difficult part, I think, is that there are spots that use
unfiltered output because the intervention of the pager at that
moment could cause trouble.  For example, targets may announce
things when infrun is doing its thing, and it's not at all clear
to me that this code is set up to handle a random quit().
You can look for print_thread_events to see some examples of
what's being printed.

One idea might be to let this sort of code protect itself
by setting some global variable.  This global would indicate
that the pager should temporarily be disabled.

Alternatively maybe these spots can be made quit-safe.
Or, maybe I'm just mistaken about this.

I'm in favor of implementing this idea, FWIW.  I think it would
remove a contribution / maintenance hurdle in gdb.  It would
probably simplify the pager code.  Also we could finally make
wrap_here a method on ui_file instead of this weird thing we
currently have, where code sometimes does:

      wrap_here ("        ");
      fputs_filtered (i == 0 ? ": " : ", ", stream);

... that is, mixes output to some parameterized stream with
calls to wrap_here, which actually only works for stdout.

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

[Bug cli/7234] Re-do GDB's output pager.

[tromey at sourceware dot org]

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

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
We already have set_batch_flag_and_restore_page_info for
temporarily disabling the pager, so that part is solved.
The question is more where to use it.

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

[Bug cli/7234] Re-do GDB's output pager.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

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

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
I have a series to get rid of this distinction and rely just 
on the stream.

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

[Bug cli/7234] Re-do GDB's output pager.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 3cd5229387926dbd2fe842328e7d923bbb41322c
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Jan 1 13:18:17 2022 -0700

    Change the pager to a ui_file

    This rewrites the output pager as a ui_file implementation.

    A new header is introduced to declare the pager class.  The
    implementation remains in utils.c for the time being, because there
    are some static globals there that must be used by this code.  (This
    could be cleaned up at some future date.)

    I went through all the text output in gdb to ensure that this change
    should be ok.  There are a few cases:

    * Any existing call to printf_unfiltered is required to be avoid the
      pager.  This is ensured directly in the implementation.

    * All remaining calls to the f*_unfiltered functions -- the ones that
      take an explicit ui_file -- either send to an unfiltered stream
      (e.g., gdb_stderr), which is obviously ok; or conditionally send to
      gdb_stdout

      I investigated all such calls by searching for:

        grep -e '\bf[a-z0-9_]*_unfiltered' *.[chyl] */*.[ch] | grep -v
gdb_stdlog | grep -v gdb_stderr

      This yields a number of candidates to check.

      * The breakpoint _print_recreate family, and
        save_trace_state_variables.  These are used for "save" commands
        and so are fine.

      * Things printing to a temporary stream.  Obviously ok.

      * Disassembly selftests.

      * print_gdb_help - this is non-obvious, but ok because paging isn't
        yet enabled at this point during startup.

      * serial.c - doens't use gdb_stdout

      * The code in compile/.  This is all printing to a file.

      * DWARF DIE dumping - doesn't reference gdb_stdout.

    * Calls to the _filtered form -- these are all clearly ok, because if
      they are using gdb_stdout, then filtering will still apply; and if
      not, then filtering never applied and still will not.

    Therefore, at this point, there is no longer any distinction between
    all the other _filtered and _unfiltered calls, and they can be
    unified.

    In this patch, take special note of the vfprintf_maybe_filtered and
    ui_file::vprintf change.  This is one instance of the above idea,
    erasing the distinction between filtered and unfiltered -- in this
    part of the change, the "unfiltered_output" flag is never passe to
    cli_ui_out.  Subsequent patches will go much further in this
    direction.

    Also note the can_emit_style_escape changes in ui-file.c.  Checking
    against gdb_stdout or gdb_stderr was always a bit of a hack; and now
    it is no longer needed, because this is decision can be more fully
    delegated to the particular ui_file implementation.

    ui_file::can_page is removed, because this patch removed the only call
    to it.

    I think this is the main part of fixing PR cli/7234.

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

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

[Bug cli/7234] Re-do GDB's output pager.

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.1
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
Fixed.

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