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

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

Commit Message

Tom Tromey May 29, 2018, 2:52 a.m. UTC
  >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> 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.

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

In this version I've reverted that as well.

Pedro> Hmm, I'm a little surprised at the reversion to say "to continue"
Pedro> instead of "for more" as in the earlier suggestions:
Pedro>    "--Type <RET> for more, q to quit, c to continue without paging--"
Pedro> vs
Pedro>    "--Type <RET> to continue, q to quit, c to continue without paging--"

Pedro> I had suggested "for more" for two reasons:
Pedro>  - it's shorter
Pedro>  - avoids ambiguity in the saying "continue" twice

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

I think I just didn't notice that detail, or if I did, I forgot at some
point along the way.

Here's a new version.

Tom

commit 0510dda7a558fa3255d6768262b934c101ea526f
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.
    
    Tested by the buildbot.
    
    ChangeLog
    2018-05-28  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-28  Tom Tromey  <tom@tromey.com>
    
            PR cli/12326:
            * gdb.texinfo (Screen Size): Document "c" response to pagination
            prompt.
    
    testsuite/ChangeLog
    2018-05-28  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: Update.
  

Comments

Pedro Alves May 29, 2018, 4:05 p.m. UTC | #1
On 05/29/2018 03:52 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> 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.
> 
> Pedro> That's fine with me (though you're removing it in the
> Pedro> gdb.python/ tests AFAICT).
> 
> In this version I've reverted that as well.

Thanks.

> Here's a new version.

LGTM.  One suggestion in the docs below.  Sorry about being picky...

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 62ba1d25a4c..f1c879098cf 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

I'd suggest saying something here closer to gdb's output.  For example,
above, "continue the output" doesn't suggest that it's only one more
page, and, using the verb "continue" for the "c" key I think makes it
easier to memorize / understand.  Like so:

 Type @key{RET} when you want to see one more page of output, @kbd{q}
 to discard the remaining output, or @kbd{c} to continue without paging
 for the rest of the current command.

No need to post a new patch for that, if Eli's OK with it.

Thanks,
Pedro Alves
  
Tom Tromey June 4, 2018, 9:47 p.m. UTC | #2
Hi Eli, not sure if you saw Pedro's question in this thread:

Pedro> I'd suggest saying something here closer to gdb's output.  For example,
Pedro> above, "continue the output" doesn't suggest that it's only one more
Pedro> page, and, using the verb "continue" for the "c" key I think makes it
Pedro> easier to memorize / understand.  Like so:

Pedro>  Type @key{RET} when you want to see one more page of output, @kbd{q}
Pedro>  to discard the remaining output, or @kbd{c} to continue without paging
Pedro>  for the rest of the current command.

Pedro> No need to post a new patch for that, if Eli's OK with it.

thanks,
Tom
  
Eli Zaretskii June 5, 2018, 2:45 p.m. UTC | #3
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org, eliz@gnu.org
> Date: Mon, 04 Jun 2018 15:47:52 -0600
> 
> Hi Eli, not sure if you saw Pedro's question in this thread:
> 
> Pedro> I'd suggest saying something here closer to gdb's output.  For example,
> Pedro> above, "continue the output" doesn't suggest that it's only one more
> Pedro> page, and, using the verb "continue" for the "c" key I think makes it
> Pedro> easier to memorize / understand.  Like so:
> 
> Pedro>  Type @key{RET} when you want to see one more page of output, @kbd{q}
> Pedro>  to discard the remaining output, or @kbd{c} to continue without paging
> Pedro>  for the rest of the current command.
> 
> Pedro> No need to post a new patch for that, if Eli's OK with it.

I'm OK with that.  Sorry for missing the cue.
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 98fb9551269..73b5638b71e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@ 
+2018-05-28  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-27  Tom Tromey  <tom@tromey.com>
 
 	* Makefile.in (DEPFILES): Don't reference REMOTE_OBS.
diff --git a/gdb/NEWS b/gdb/NEWS
index cef558039ed..265f52c748d 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 798ff3715ed..f61c70f24c3 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@ 
+2018-05-28  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 62ba1d25a4c..f1c879098cf 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 b2938b1bf1d..aad72d0596e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2018-05-28  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: Update.
+
 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 47db79fb8e4..f7557d3b94b 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 ab60a0cba0f..22b0cf6cb97 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 " for more, 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 f6bf93add0d..26c7b6320db 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -143,13 +143,13 @@  gdb_test_no_output "set height $lines"
 
 set test "verify pagination beforehand"
 gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
-    -re "---Type <return>" {
+    -re "--Type <RET>" {
 	exp_continue
     }
-    -re " to continue, or q <return>" {
+    -re " for more, q to quit" {
 	exp_continue
     }
-    -re " to quit---$" {
+    -re ", c to continue without paging--$" {
 	pass $test
     }
 }
@@ -159,13 +159,13 @@  gdb_test "python if gdb.execute('python print (\"\\\\n\" * $lines)', to_string=T
 
 set test "verify pagination afterwards"
 gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
-    -re "---Type <return>" {
+    -re "--Type <RET>" {
 	exp_continue
     }
-    -re " to continue, or q <return>" {
+    -re " for more, q to quit" {
 	exp_continue
     }
-    -re " to quit---$" {
+    -re ", c to continue without paging--$" {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ee66a38e08f..aef580b04d3 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> for more, 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 a2e933bc8dc..dd66d725fcb 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> for more, 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.  */