[RFA,v2] Add "continue" response to pager
Commit Message
This adds a "continue" response to the pager. If the user types "c"
in response to the pager prompt, pagination will be disabled for the
duration of one command -- but re-enabled afterward. This is handy if
you type a command that produces a lot of output, and you don't want
to baby-sit it by typing "return" each time the prompt comes up.
This is v2 of a patch originally sent here but never committed.
https://sourceware.org/ml/gdb-patches/2014-06/msg00862.html
I think this version addresses all comments in that thread.
Tested by the buildbot.
gdb/ChangeLog
2018-04-25 Tom Tromey <tom@tromey.com>
PR cli/12326:
* NEWS: Add entry about pager.
* utils.c (pagination_disabled_for_command): New global.
(prompt_for_continue): Allow "c" response to prompt.
(reinitialize_more_filter): Clear
pagination_disabled_for_command.
(fputs_maybe_filtered): Check pagination_disabled_for_command.
gdb/doc/ChangeLog
2018-04-25 Tom Tromey <tom@tromey.com>
* gdb.texinfo (Screen Size): Document "c" response to pagination
prompt.
gdb/testsuite/ChangeLog
2018-04-25 Tom Tromey <tom@tromey.com>
PR cli/12326:
* gdb.cp/static-print-quit.exp: Update.
* lib/gdb.exp (pagination_prompt): Update.
* gdb.base/page.exp: Use pagination_prompt. Add new tests.
* gdb.python/python.exp: Use pagination_prompt.
---
gdb/ChangeLog | 10 +++++++++
gdb/NEWS | 3 +++
gdb/doc/ChangeLog | 5 +++++
gdb/doc/gdb.texinfo | 5 +++--
gdb/testsuite/ChangeLog | 8 ++++++++
gdb/testsuite/gdb.base/page.exp | 21 ++++++++++++++++++-
gdb/testsuite/gdb.cp/static-print-quit.exp | 9 +-------
gdb/testsuite/gdb.python/python.exp | 33 +++++++++++-------------------
gdb/testsuite/lib/gdb.exp | 3 ++-
gdb/utils.c | 27 +++++++++++++++++++-----
10 files changed, 86 insertions(+), 38 deletions(-)
Comments
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> This adds a "continue" response to the pager. If the user types "c"
Tom> in response to the pager prompt, pagination will be disabled for the
Tom> duration of one command -- but re-enabled afterward. This is handy if
Tom> you type a command that produces a lot of output, and you don't want
Tom> to baby-sit it by typing "return" each time the prompt comes up.
Ping.
Tom
On 04/25/2018 03:52 PM, Tom Tromey wrote:
> This adds a "continue" response to the pager. If the user types "c"
> in response to the pager prompt, pagination will be disabled for the
> duration of one command -- but re-enabled afterward. This is handy if
> you type a command that produces a lot of output, and you don't want
> to baby-sit it by typing "return" each time the prompt comes up.
>
> This is v2 of a patch originally sent here but never committed.
>
> https://sourceware.org/ml/gdb-patches/2014-06/msg00862.html
>
> I think this version addresses all comments in that thread.
>
> Tested by the buildbot.
Thanks, cool feature. I've wanted it before too.
I gave the patch a quick try, and I have to say that it looks a
bit odd to me that the pager prompt takes two lines:
---Type <return> to continue, q <return> to quit, or c <return> to
continue without paging---
I'm wondering whether we can shorten that, to, say:
---Type <return> for more, q to quit, c to continue without paging---
That seems clear enough to me, and is still 69 cols. I doubt
anyone would miss the indication to type enter after q or c.
We could shorten it more, like, say:
---Type <return> for more, q to quit, c to continue without paging--- # 69 cols
---Type <ret> for more, q to quit, c to continue without paging--- # 66 cols
--Type <return> for more, q to quit, c to continue without paging-- # 67 cols
--Type <ret> for more, q to quit, c to continue without paging-- # 64 cols
> diff --git a/gdb/testsuite/gdb.cp/static-print-quit.exp b/gdb/testsuite/gdb.cp/static-print-quit.exp
> index ab60a0cba0..81ea61f7ce 100644
> --- a/gdb/testsuite/gdb.cp/static-print-quit.exp
> +++ b/gdb/testsuite/gdb.cp/static-print-quit.exp
> @@ -41,14 +41,7 @@ gdb_test_multiple "print c" $test {
>
> set test "print c - q <return>"
> gdb_test_multiple "" $test {
> - -re " to continue, or q <return>" {
> - pass $test
> - }
> -}
> -
> -set test "print c - to quit"
> -gdb_test_multiple "" $test {
> - -re " to quit---$" {
> + -re " continue without paging---$" {
> pass $test
> }
> }
This looks like it was split in two for some reason:
commit 7db6f30f9d4707b04e7292d7d8bfa5d37eaff67f
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
AuthorDate: Fri May 6 13:38:35 2011 +0000
gdb/testsuite/
Fix a race.
* gdb.cp/static-print-quit.exp (print c): Split to ...
(print c - <return>, print c - q <return>, print c - to quit):
... these. Make the testfile untested on gdb-7.1.
Did you look up the patch submission, see if it still makes sense?
> diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
> index cee195f315..8b296582a9 100644
> --- a/gdb/testsuite/gdb.python/python.exp
> +++ b/gdb/testsuite/gdb.python/python.exp
> @@ -139,33 +139,24 @@ set lines 10
> gdb_test_no_output "set height $lines"
>
> set test "verify pagination beforehand"
> -gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
> - -re "---Type <return>" {
> - exp_continue
> - }
> - -re " to continue, or q <return>" {
> - exp_continue
> - }
> - -re " to quit---$" {
> - pass $test
> - }
So I'm wondering whether this was split for a
similar reason. Likewise other testsuite spots in the patch.
> diff --git a/gdb/utils.c b/gdb/utils.c
> index b957b0dc5c..4478c52166 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -1282,6 +1282,10 @@ show_chars_per_line (struct ui_file *file, int from_tty,
> /* Current count of lines printed on this page, chars on this line. */
> static unsigned int lines_printed, chars_printed;
>
> +/* True if pagination is disabled for just one command. */
> +
> +static int pagination_disabled_for_command;
Use bool?
> +
> /* Buffer and start column of buffered text, for doing smarter word-
> wrapping. When someone calls wrap_here(), we start buffering output
> that comes through fputs_filtered(). If we see a newline, we just
> @@ -1466,12 +1470,14 @@ prompt_for_continue (void)
> prompt_for_continue_wait_time. */
> using namespace std::chrono;
> steady_clock::time_point prompt_started = steady_clock::now ();
> + int disable_pagination = pagination_disabled_for_command;
Ditto.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I gave the patch a quick try, and I have to say that it looks a
Pedro> bit odd to me that the pager prompt takes two lines:
Pedro> ---Type <return> to continue, q <return> to quit, or c <return> to
Pedro> continue without paging---
Pedro> I'm wondering whether we can shorten that, to, say:
Pedro> ---Type <return> for more, q to quit, c to continue without paging---
Yeah, I think that would be better.
The Emacs docs write <RET> for return. So maybe:
Pedro> --Type <ret> for more, q to quit, c to continue without paging-- # 64 cols
Though I guess I would tend to add a couple of spaces, so:
-- Type <RET> for more, q to quit, c to continue without paging -- # 66 cols
I would like to get the spelling worked out before updating the patch.
Pedro> Did you look up the patch submission, see if it still makes sense?
Nope, but if we shorten the text then perhaps the change won't be
needed.
>> +static int pagination_disabled_for_command;
Pedro> Use bool?
I made this change locally.
Tom
On 05/09/2018 10:28 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> I gave the patch a quick try, and I have to say that it looks a
> Pedro> bit odd to me that the pager prompt takes two lines:
>
> Pedro> ---Type <return> to continue, q <return> to quit, or c <return> to
> Pedro> continue without paging---
>
> Pedro> I'm wondering whether we can shorten that, to, say:
>
> Pedro> ---Type <return> for more, q to quit, c to continue without paging---
>
> Yeah, I think that would be better.
>
> The Emacs docs write <RET> for return. So maybe:
>
> Pedro> --Type <ret> for more, q to quit, c to continue without paging-- # 64 cols
>
> Though I guess I would tend to add a couple of spaces, so:
>
> -- Type <RET> for more, q to quit, c to continue without paging -- # 66 cols
That's fine with me. I suspect that the "---" wrapping without
spaces might have been inspired by some Unix /usr/bin/more
implementation, because util-linux's 'more' prints "--More-- "
by default.
> I would like to get the spelling worked out before updating the patch.
>
> Pedro> Did you look up the patch submission, see if it still makes sense?
>
> Nope, but if we shorten the text then perhaps the change won't be
> needed.
A racy test won't usually be an issue with size of the text, but
about how the regexps might match differently depending
on how much the expect buffer happens to get filled with.
I found the original submission, here:
https://sourceware.org/ml/gdb-patches/2011-05/msg00150.html
which points at:
~~~~~~~~~~~~
However, when using read1(), the "<return>" part matches with
this snippet from lib/gdb.exp:
"<return>" {
send_gdb "\n"
perror "Window too small."
fail "$message"
...
~~~~~~~~~~~~
However, someone *cough* changed that "<return>" to
$pagination_prompt instead:
-re "$pagination_prompt" {
send_gdb "\n"
perror "Window too small."
fail "$message"
in:
commit c3f814a14336b9d395f3abad739592929e2faaa0
Author: Pedro Alves <palves@redhat.com>
AuthorDate: Fri Jul 25 10:07:38 2014 +0100
Fix paginate-*.exp races
so it looks like the split is really no longer necessary.
A brief passage about this in the commit log would be nice,
so that we have a history link in case we need to revisit.
Please confirm that the affected tests pass cleanly with
make check-read1, just to make sure.
Thanks,
Pedro Alves
@@ -3,6 +3,9 @@
*** Changes since GDB 8.1
+* The pager now allows a "c" response, meaning to disable the pager
+ for the rest of the current command.
+
* The commands 'info variables/functions/types' now show the source line
numbers of symbol definitions when available.
@@ -23874,8 +23874,9 @@ Print ten commands just after the commands last printed.
Certain commands to @value{GDBN} may produce large amounts of
information output to the screen. To help you read all of it,
@value{GDBN} pauses and asks you for input at the end of each page of
-output. Type @key{RET} when you want to continue the output, or @kbd{q}
-to discard the remaining output. Also, the screen width setting
+output. Type @key{RET} when you want to continue the output, @kbd{q}
+to discard the remaining output, or @kbd{c} to disable paging for the
+rest of the current command. Also, the screen width setting
determines when to wrap lines of output. Depending on what is being
printed, @value{GDBN} tries to break the line at a readable place,
rather than simply letting it overflow onto the following line.
@@ -47,7 +47,7 @@ gdb_test "show pagination" "State of pagination is on.*" "pagination is on"
gdb_test_no_output "set height 10"
send_gdb "help\n"
gdb_expect_list "paged help" \
- ".*---Type <return> to continue, or q <return> to quit---" {
+ ".*$pagination_prompt" {
"List of classes of commands:"
""
"aliases -- Aliases of other commands"
@@ -60,6 +60,25 @@ gdb_expect_list "paged help" \
}
gdb_test "q"
+gdb_test_no_output "set height 5"
+send_gdb "printf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\n11\"\n"
+gdb_expect_list "paged count" \
+ ".*$pagination_prompt" {
+ 1
+ 2
+ 3
+ 4
+}
+send_gdb "c\n"
+gdb_expect_list "paged count remainder" "${gdb_prompt} " {
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ 11
+}
gdb_exit
return 0
@@ -41,14 +41,7 @@ gdb_test_multiple "print c" $test {
set test "print c - q <return>"
gdb_test_multiple "" $test {
- -re " to continue, or q <return>" {
- pass $test
- }
-}
-
-set test "print c - to quit"
-gdb_test_multiple "" $test {
- -re " to quit---$" {
+ -re " continue without paging---$" {
pass $test
}
}
@@ -139,33 +139,24 @@ set lines 10
gdb_test_no_output "set height $lines"
set test "verify pagination beforehand"
-gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
- -re "---Type <return>" {
- exp_continue
- }
- -re " to continue, or q <return>" {
- exp_continue
- }
- -re " to quit---$" {
- pass $test
- }
+set arglist {}
+for {set i 0} {$i < $lines} {incr i} {
+ lappend arglist ""
}
+lappend arglist ""
+send_gdb "python print (\"\\n\" * $lines)\n"
+gdb_expect_list $test \
+ ".*$pagination_prompt" \
+ $arglist
gdb_test "q" "Quit.*Error while executing Python.*" "verify pagination beforehand: q"
gdb_test "python if gdb.execute('python print (\"\\\\n\" * $lines)', to_string=True) == \"\\n\" * [expr $lines + 1]: print (\"yes\")" "yes" "gdb.execute does not page"
set test "verify pagination afterwards"
-gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
- -re "---Type <return>" {
- exp_continue
- }
- -re " to continue, or q <return>" {
- exp_continue
- }
- -re " to quit---$" {
- pass $test
- }
-}
+send_gdb "python print (\"\\n\" * $lines)\n"
+gdb_expect_list $test \
+ ".*$pagination_prompt" \
+ $arglist
gdb_test "q" "Quit.*Error while executing Python.*" "verify pagination afterwards: q"
gdb_test_no_output "set height 0"
@@ -82,7 +82,8 @@ if ![info exists gdb_prompt] then {
}
# A regexp that matches the pagination prompt.
-set pagination_prompt [string_to_regexp "---Type <return> to continue, or q <return> to quit---"]
+set pagination_prompt \
+ "---Type <return> to continue, q <return> to quit, or c <return> to\r\n *continue without paging---"
# The variable fullname_syntax_POSIX is a regexp which matches a POSIX
# absolute path ie. /foo/
@@ -1282,6 +1282,10 @@ show_chars_per_line (struct ui_file *file, int from_tty,
/* Current count of lines printed on this page, chars on this line. */
static unsigned int lines_printed, chars_printed;
+/* True if pagination is disabled for just one command. */
+
+static int pagination_disabled_for_command;
+
/* Buffer and start column of buffered text, for doing smarter word-
wrapping. When someone calls wrap_here(), we start buffering output
that comes through fputs_filtered(). If we see a newline, we just
@@ -1466,12 +1470,14 @@ prompt_for_continue (void)
prompt_for_continue_wait_time. */
using namespace std::chrono;
steady_clock::time_point prompt_started = steady_clock::now ();
+ int disable_pagination = pagination_disabled_for_command;
if (annotation_level > 1)
printf_unfiltered (("\n\032\032pre-prompt-for-continue\n"));
strcpy (cont_prompt,
- "---Type <return> to continue, or q <return> to quit---");
+ "---Type <return> to continue, q <return> to quit, or c <return> to\n"
+ " continue without paging---");
if (annotation_level > 1)
strcat (cont_prompt, "\n\032\032prompt-for-continue\n");
@@ -1501,11 +1507,14 @@ prompt_for_continue (void)
if (p[0] == 'q')
/* Do not call quit here; there is no possibility of SIGINT. */
throw_quit ("Quit");
+ if (p[0] == 'c')
+ disable_pagination = 1;
}
/* Now we have to do this again, so that GDB will know that it doesn't
need to save the ---Type <return>--- line at the top of the screen. */
reinitialize_more_filter ();
+ pagination_disabled_for_command = disable_pagination;
dont_repeat (); /* Forget prev cmd -- CR won't repeat it. */
}
@@ -1535,6 +1544,7 @@ reinitialize_more_filter (void)
{
lines_printed = 0;
chars_printed = 0;
+ pagination_disabled_for_command = 0;
}
/* Indicate that if the next sequence of characters overflows the line,
@@ -1679,6 +1689,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
/* Don't do any filtering if it is disabled. */
if (stream != gdb_stdout
|| !pagination_enabled
+ || pagination_disabled_for_command
|| batch_flag
|| (lines_per_page == UINT_MAX && chars_per_line == UINT_MAX)
|| top_level_interpreter () == NULL
@@ -1695,8 +1706,11 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
lineptr = linebuffer;
while (*lineptr)
{
- /* Possible new page. */
- if (filter && (lines_printed >= lines_per_page - 1))
+ /* Possible new page. Note that PAGINATION_DISABLED_FOR_COMMAND
+ might be set during this loop, so we must continue to check
+ it here. */
+ if (filter && (lines_printed >= lines_per_page - 1)
+ && !pagination_disabled_for_command)
prompt_for_continue ();
while (*lineptr && *lineptr != '\n')
@@ -1736,8 +1750,11 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
if (wrap_column)
fputc_unfiltered ('\n', stream);
- /* Possible new page. */
- if (lines_printed >= lines_per_page - 1)
+ /* Possible new page. Note that
+ PAGINATION_DISABLED_FOR_COMMAND might be set during
+ this loop, so we must continue to check it here. */
+ if (lines_printed >= lines_per_page - 1
+ && !pagination_disabled_for_command)
prompt_for_continue ();
/* Now output indentation and wrapped string. */