[RFA,v2] Add "continue" response to pager

Message ID 87in79znmh.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey May 27, 2018, 5:14 a.m. UTC
  >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> That's fine with me.  I suspect that the "---" wrapping without
Pedro> spaces might have been inspired by some Unix /usr/bin/more
Pedro> implementation, because util-linux's 'more' prints "--More-- "
Pedro> by default.

In the end I removed the spaces.

Pedro> commit c3f814a14336b9d395f3abad739592929e2faaa0
Pedro> Author:     Pedro Alves <palves@redhat.com>
Pedro> AuthorDate: Fri Jul 25 10:07:38 2014 +0100
Pedro>     Fix paginate-*.exp races

Pedro> so it looks like the split is really no longer necessary.

With the new text it was sufficient to just update what was expected in
this test.  So I didn't end up backing out the old race fix.  It seemed
pretty safe to just leave it alone.

Here's the latest version.

Tom

commit ccd0b974d93f5a87b33fdfb38eea7ed12b00fca0
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Apr 25 08:52:00 2018 -0600

    Add "continue" response to pager
    
    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.
    
    I think this version addresses all review comments from versions 1-3.
    
    Tested by the buildbot.
    
    ChangeLog
    2018-05-26  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.
    
    doc/ChangeLog
    2018-05-26  Tom Tromey  <tom@tromey.com>
    
            PR cli/12326:
            * gdb.texinfo (Screen Size): Document "c" response to pagination
            prompt.
    
    testsuite/ChangeLog
    2018-05-26  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.
  

Comments

Eli Zaretskii May 27, 2018, 3:54 p.m. UTC | #1
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sat, 26 May 2018 23:14:14 -0600
> 
> commit ccd0b974d93f5a87b33fdfb38eea7ed12b00fca0
> Author: Tom Tromey <tom@tromey.com>
> Date:   Wed Apr 25 08:52:00 2018 -0600
> 
>     Add "continue" response to pager
>     
>     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.
>     
>     I think this version addresses all review comments from versions 1-3.
>     
>     Tested by the buildbot.
>     
>     ChangeLog
>     2018-05-26  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.
>     
>     doc/ChangeLog
>     2018-05-26  Tom Tromey  <tom@tromey.com>
>     
>             PR cli/12326:
>             * gdb.texinfo (Screen Size): Document "c" response to pagination
>             prompt.
>     
>     testsuite/ChangeLog
>     2018-05-26  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.

Thanks, this is okay for the documentation parts.
  
Pedro Alves May 28, 2018, 5:39 p.m. UTC | #2
On 05/27/2018 06:14 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> That's fine with me.  I suspect that the "---" wrapping without
> Pedro> spaces might have been inspired by some Unix /usr/bin/more
> Pedro> implementation, because util-linux's 'more' prints "--More-- "
> Pedro> by default.
> 
> In the end I removed the spaces.
> 
> Pedro> commit c3f814a14336b9d395f3abad739592929e2faaa0
> Pedro> Author:     Pedro Alves <palves@redhat.com>
> Pedro> AuthorDate: Fri Jul 25 10:07:38 2014 +0100
> Pedro>     Fix paginate-*.exp races
> 
> Pedro> so it looks like the split is really no longer necessary.
> 
> With the new text it was sufficient to just update what was expected in
> this test.  So I didn't end up backing out the old race fix.  It seemed
> pretty safe to just leave it alone.

That's fine with me (though you're removing it in the
gdb.python/ tests AFAICT).

> commit ccd0b974d93f5a87b33fdfb38eea7ed12b00fca0
> Author: Tom Tromey <tom@tromey.com>
> Date:   Wed Apr 25 08:52:00 2018 -0600
> 
>     Add "continue" response to pager

...

>     
>     I think this version addresses all review comments from versions 1-3.

I'd suggest removing this sentence before pushing.

>     
>     Tested by the buildbot.

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index ee66a38e08..65dd1d5afd 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -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 <RET> to continue, q to quit, c to continue without paging--"

Hmm, I'm a little surprised at the reversion to say "to continue"
instead of "for more" as in the earlier suggestions:

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

I had suggested "for more" for two reasons:

 - it's shorter
 - avoids ambiguity in the saying "continue" twice

It's not a big deal, but since you didn't comment on that,
I can't be sure whether you changed it back on purpose
or by accident, hence I'm speaking up.  :-)

Otherwise it LGTM.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 33c6a712a8..8d2863524f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@ 
+2018-05-26  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.
+
 2018-05-25  Tom Tromey  <tom@tromey.com>
 
 	* value.c (value::location): Initialize.
diff --git a/gdb/NEWS b/gdb/NEWS
index cef558039e..265f52c748 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -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.
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 798ff3715e..511debe8ff 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@ 
+2018-05-26  Tom Tromey	<tom@tromey.com>
+
+	PR cli/12326:
+	* gdb.texinfo (Screen Size): Document "c" response to pagination
+	prompt.
+
 2018-05-04  Tom Tromey  <tom@tromey.com>
 
 	PR python/22731:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 62ba1d25a4..f1c879098c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23877,8 +23877,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.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b2938b1bf1..ef2cd6b53d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2018-05-26  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.
+
 2018-05-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	PR gdb/23203
diff --git a/gdb/testsuite/gdb.base/page.exp b/gdb/testsuite/gdb.base/page.exp
index 47db79fb8e..f7557d3b94 100644
--- a/gdb/testsuite/gdb.base/page.exp
+++ b/gdb/testsuite/gdb.base/page.exp
@@ -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
diff --git a/gdb/testsuite/gdb.cp/static-print-quit.exp b/gdb/testsuite/gdb.cp/static-print-quit.exp
index ab60a0cba0..f8c94a9028 100644
--- a/gdb/testsuite/gdb.cp/static-print-quit.exp
+++ b/gdb/testsuite/gdb.cp/static-print-quit.exp
@@ -29,10 +29,10 @@  gdb_test_no_output "set height 2"
 
 set test "print c - <return>"
 gdb_test_multiple "print c" $test {
-    -re "\\$\[0-9\]+ = \{loooooooooooooooooooooooooooooooooooooooooooooong = 0, static field = \{\r\n---Type <return>" {
+    -re "\\$\[0-9\]+ = \{loooooooooooooooooooooooooooooooooooooooooooooong = 0, static field = \{\r\n--Type <RET>" {
 	pass $test
     }
-    -re "\r\n---Type <return>" {
+    -re "\r\n--Type <RET>" {
 	# gdb-7.1 did not crash with this testcase but it had the same bug.
 	untested "bug does not reproduce"
 	return 0
@@ -41,14 +41,14 @@  gdb_test_multiple "print c" $test {
 
 set test "print c - q <return>"
 gdb_test_multiple "" $test {
-    -re " to continue, or q <return>" {
+    -re " to continue, q to quit, " {
 	pass $test
     }
 }
 
-set test "print c - to quit"
+set test "print c - remainder"
 gdb_test_multiple "" $test {
-    -re " to quit---$" {
+    -re "c to continue without paging--$" {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index f6bf93add0..b921b829fd 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -142,33 +142,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"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ee66a38e08..65dd1d5afd 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -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 <RET> to continue, q to quit, c to continue without paging--"
 
 # The variable fullname_syntax_POSIX is a regexp which matches a POSIX 
 # absolute path ie. /foo/ 
diff --git a/gdb/utils.c b/gdb/utils.c
index a2e933bc8d..9383eadd02 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1283,6 +1283,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 bool 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
@@ -1467,12 +1471,14 @@  prompt_for_continue (void)
      prompt_for_continue_wait_time.  */
   using namespace std::chrono;
   steady_clock::time_point prompt_started = steady_clock::now ();
+  bool 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 <RET> to continue, q to quit, "
+	  "c to continue without paging--");
   if (annotation_level > 1)
     strcat (cont_prompt, "\n\032\032prompt-for-continue\n");
 
@@ -1502,11 +1508,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 = true;
     }
 
   /* 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.  */
 }
@@ -1536,6 +1545,7 @@  reinitialize_more_filter (void)
 {
   lines_printed = 0;
   chars_printed = 0;
+  pagination_disabled_for_command = false;
 }
 
 /* Indicate that if the next sequence of characters overflows the line,
@@ -1680,6 +1690,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
@@ -1696,8 +1707,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')
@@ -1737,8 +1751,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.  */