Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] [gdb/cli] Improve the pagination prompt
@ 2025-07-15 19:37 Tom de Vries
  2025-07-16 11:30 ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2025-07-15 19:37 UTC (permalink / raw)
  To: gdb-patches

Currently the pagination prompt is:
...
--Type <RET> for more, q to quit, c to continue without paging--
...

PR cli/33149 points out an inconsistency.

While typing <RET> works as advertised, pressing q or c doesn't have any
other effect than echoing the character to screen.  An additional <RET> is
needed.

This is a regression since commit eb6af80922a ("Add "continue" response to
pager"), which changed:
...
---Type <return> to continue, or q <return> to quit---
...
into:
...
--Type <RET> for more, q to quit, c to continue without paging--
...

Fix this by using the slightly longer (69 instead of 64 chars):
...
--Type <RET> (more), q<RET> (quit), or c<RET> (continue, no paging)--
...

Also update the documentation to clarify the same issue.

In the testsuite, update the pagination_prompt variable and use it in more
test-cases.

In some test-cases, matching the pagination prompt was split up to address a
matching race but that's no longer necessary, thanks to commit c3f814a1433
("Fix paginate-*.exp races").

Tested on aarch64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33149
---
 gdb/doc/gdb.texinfo                        |  6 +++---
 gdb/testsuite/gdb.cp/static-print-quit.exp | 23 ++++------------------
 gdb/testsuite/gdb.python/py-cmd.exp        |  8 +-------
 gdb/testsuite/gdb.python/python.exp        | 16 ++-------------
 gdb/testsuite/lib/gdb.exp                  |  4 +++-
 gdb/utils.c                                |  8 +++++---
 6 files changed, 18 insertions(+), 47 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 35b770f8138..af434d62918 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27921,9 +27921,9 @@ 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 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.  Also, the screen
-width setting determines when to wrap lines of output.  Depending on
+@kbd{q} @key{RET} to discard the remaining output, or @kbd{c} @key{RET} to
+continue without 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/gdb.cp/static-print-quit.exp b/gdb/testsuite/gdb.cp/static-print-quit.exp
index 8e0b61d86e7..e4bce7f0d8a 100644
--- a/gdb/testsuite/gdb.cp/static-print-quit.exp
+++ b/gdb/testsuite/gdb.cp/static-print-quit.exp
@@ -27,32 +27,17 @@ clean_restart $testfile.o
 gdb_test_no_output "set width 80"
 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 <RET>" {
-	pass $test
+gdb_test_multiple "print c" "" {
+    -re "\\$\[0-9\]+ = \{loooooooooooooooooooooooooooooooooooooooooooooong = 0, static field = \{\r\n$pagination_prompt$" {
+	pass $gdb_test_name
     }
-    -re "\r\n--Type <RET>" {
+    -re "\r\n$pagination_prompt$" {
 	# gdb-7.1 did not crash with this testcase but it had the same bug.
 	untested "bug does not reproduce"
 	return 0
     }
 }
 
-set test "print c - q <return>"
-gdb_test_multiple "" $test {
-    -re " for more, q to quit, " {
-	pass $test
-    }
-}
-
-set test "print c - remainder"
-gdb_test_multiple "" $test {
-    -re "c to continue without paging--$" {
-	pass $test
-    }
-}
-
 gdb_test "q" ".*"
 
 # Now the obstack is uninitialized.  Exercise it.
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index 5ac57122f99..1fa3c73067b 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -278,13 +278,7 @@ gdb_test_multiline "input multi-line-output command" \
 
 set test "verify pagination from test_multiline"
 gdb_test_multiple "test_multiline" $test {
-    -re "--Type <RET>" {
-	exp_continue
-    }
-    -re " for more, q to quit" {
-	exp_continue
-    }
-    -re ", c to continue without paging--$" {
+    -re "$pagination_prompt$" {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 6b2f671f5e4..96977dff629 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -152,13 +152,7 @@ gdb_test_no_output "set height $lines"
 
 set test "verify pagination beforehand"
 gdb_test_multiple "python print (\"\\n\" * $lines)" $test {
-    -re "--Type <RET>" {
-	exp_continue
-    }
-    -re " for more, q to quit" {
-	exp_continue
-    }
-    -re ", c to continue without paging--$" {
+    -re "$pagination_prompt$" {
 	pass $test
     }
 }
@@ -168,13 +162,7 @@ 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 <RET>" {
-	exp_continue
-    }
-    -re " for more, q to quit" {
-	exp_continue
-    }
-    -re ", c to continue without paging--$" {
+    -re "$pagination_prompt$" {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 777d64d14d1..5e07ac2738e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -289,7 +289,9 @@ if {![info exists gdb_prompt]} {
 
 # A regexp that matches the pagination prompt.
 set pagination_prompt \
-    "--Type <RET> for more, q to quit, c to continue without paging--"
+    [string_list_to_regexp \
+	 "--Type <RET> (more), q<RET> (quit)," \
+	 " or c<RET> (continue, no 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 10d3d51e481..e44ba45e287 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1449,9 +1449,11 @@ pager_file::prompt_for_continue ()
   if (annotation_level > 1)
     m_stream->puts (("\n\032\032pre-prompt-for-continue\n"));
 
-  strcpy (cont_prompt,
-	  "--Type <RET> for more, q to quit, "
-	  "c to continue without paging--");
+  /* Keep the pagination prompt within 70ish chars to fit in a single
+     line in, say a VT100 (80x25) terminal.  */
+  strcpy (cont_prompt, "\
+--Type <RET> (more), q<RET> (quit), or c<RET> (continue, no paging)--");
+
   if (annotation_level > 1)
     strcat (cont_prompt, "\n\032\032prompt-for-continue\n");
 

base-commit: 83be472a61653094ea04a23b1bcadbaca0a57f84
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [gdb/cli] Improve the pagination prompt
  2025-07-15 19:37 [PATCH] [gdb/cli] Improve the pagination prompt Tom de Vries
@ 2025-07-16 11:30 ` Eli Zaretskii
  2025-07-17 10:07   ` Tom de Vries
  0 siblings, 1 reply; 4+ messages in thread
From: Eli Zaretskii @ 2025-07-16 11:30 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

> From: Tom de Vries <tdevries@suse.de>
> Date: Tue, 15 Jul 2025 21:37:47 +0200
> 
> Currently the pagination prompt is:
> ...
> --Type <RET> for more, q to quit, c to continue without paging--
> ...
> 
> PR cli/33149 points out an inconsistency.
> 
> While typing <RET> works as advertised, pressing q or c doesn't have any
> other effect than echoing the character to screen.  An additional <RET> is
> needed.
> 
> This is a regression since commit eb6af80922a ("Add "continue" response to
> pager"), which changed:
> ...
> ---Type <return> to continue, or q <return> to quit---
> ...
> into:
> ...
> --Type <RET> for more, q to quit, c to continue without paging--
> ...
> 
> Fix this by using the slightly longer (69 instead of 64 chars):
> ...
> --Type <RET> (more), q<RET> (quit), or c<RET> (continue, no paging)--
> ...

Thanks, but why <RET> is described as "more" whereas c<RET> as
"continue, no paging" instead of "more, no paging"?

> 
> Also update the documentation to clarify the same issue.
> 
> In the testsuite, update the pagination_prompt variable and use it in more
> test-cases.
> 
> In some test-cases, matching the pagination prompt was split up to address a
> matching race but that's no longer necessary, thanks to commit c3f814a1433
> ("Fix paginate-*.exp races").
> 
> Tested on aarch64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33149
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -27921,9 +27921,9 @@ 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 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.  Also, the screen
> -width setting determines when to wrap lines of output.  Depending on
> +@kbd{q} @key{RET} to discard the remaining output, or @kbd{c} @key{RET} to

The markup here is incorrect, it should be @kbd{q @key{RET}}, i.e. the
entire user input should be inside @kbd.

Please also correct "Type @key{RET}" to say "Type @kbd{@key{RET}}"
instead, for the same reason.

The principles are: @key{FOO} just describes the key labeled "FOO";
anything the user should type should be in @kbd.

Also, no NEWS entry for this?

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [gdb/cli] Improve the pagination prompt
  2025-07-16 11:30 ` Eli Zaretskii
@ 2025-07-17 10:07   ` Tom de Vries
  2025-07-17 10:16     ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2025-07-17 10:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 7/16/25 13:30, Eli Zaretskii wrote:
>> From: Tom de Vries <tdevries@suse.de>
>> Date: Tue, 15 Jul 2025 21:37:47 +0200
>>
>> Currently the pagination prompt is:
>> ...
>> --Type <RET> for more, q to quit, c to continue without paging--
>> ...
>>
>> PR cli/33149 points out an inconsistency.
>>
>> While typing <RET> works as advertised, pressing q or c doesn't have any
>> other effect than echoing the character to screen.  An additional <RET> is
>> needed.
>>
>> This is a regression since commit eb6af80922a ("Add "continue" response to
>> pager"), which changed:
>> ...
>> ---Type <return> to continue, or q <return> to quit---
>> ...
>> into:
>> ...
>> --Type <RET> for more, q to quit, c to continue without paging--
>> ...
>>
>> Fix this by using the slightly longer (69 instead of 64 chars):
>> ...
>> --Type <RET> (more), q<RET> (quit), or c<RET> (continue, no paging)--
>> ...
> 
> Thanks, but why <RET> is described as "more" whereas c<RET> as
> "continue, no paging" instead of "more, no paging"?
> 

Hi Eli,

thanks for the review.

So you're proposing the following:
...
--Type <RET> (more), q<RET> (quit), or c<RET> (more, no paging)--
...

I think q<RET> is described as "quit" and c<RET> as "continue, no 
paging" because of the first letter matching, and that matching is not 
there for "c<RET> (more, no paging)".

I also considered:
...
--Type <RET> (continue), q<RET> (quit), or c<RET> (continue, no paging)--
...
but it's longer and no so clear either.

Personally I like:
...
--Type <RET> (more), q<RET> (quit), or c<RET> (cat)--
...
which is short but requires knowledge of what's the difference of using 
more vs cat.

Anyway, I've submitted a v2 but left this unchanged ( 
https://sourceware.org/pipermail/gdb-patches/2025-July/219287.html ).

>>
>> Also update the documentation to clarify the same issue.
>>
>> In the testsuite, update the pagination_prompt variable and use it in more
>> test-cases.
>>
>> In some test-cases, matching the pagination prompt was split up to address a
>> matching race but that's no longer necessary, thanks to commit c3f814a1433
>> ("Fix paginate-*.exp races").
>>
>> Tested on aarch64-linux.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33149
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -27921,9 +27921,9 @@ 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 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.  Also, the screen
>> -width setting determines when to wrap lines of output.  Depending on
>> +@kbd{q} @key{RET} to discard the remaining output, or @kbd{c} @key{RET} to
> 
> The markup here is incorrect, it should be @kbd{q @key{RET}}, i.e. the
> entire user input should be inside @kbd.
> 
> Please also correct "Type @key{RET}" to say "Type @kbd{@key{RET}}"
> instead, for the same reason.
> 
> The principles are: @key{FOO} just describes the key labeled "FOO";
> anything the user should type should be in @kbd.
> 

Fixed in the v2.

> Also, no NEWS entry for this?
> 

Also added in the v2.

Thanks,
- Tom

> Reviewed-By: Eli Zaretskii <eliz@gnu.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [gdb/cli] Improve the pagination prompt
  2025-07-17 10:07   ` Tom de Vries
@ 2025-07-17 10:16     ` Eli Zaretskii
  0 siblings, 0 replies; 4+ messages in thread
From: Eli Zaretskii @ 2025-07-17 10:16 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches

> Date: Thu, 17 Jul 2025 12:07:55 +0200
> Cc: gdb-patches@sourceware.org
> From: Tom de Vries <tdevries@suse.de>
> 
> So you're proposing the following:
> ...
> --Type <RET> (more), q<RET> (quit), or c<RET> (more, no paging)--
> ...
> 
> I think q<RET> is described as "quit" and c<RET> as "continue, no 
> paging" because of the first letter matching, and that matching is not 
> there for "c<RET> (more, no paging)".
> 
> I also considered:
> ...
> --Type <RET> (continue), q<RET> (quit), or c<RET> (continue, no paging)--
> ...
> but it's longer and no so clear either.

If you care about the mnemonic value of "c" in "continue", then go for
the last variant.  It looks better to me than what we have.

Of course, this is a very minor issue.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-17 10:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-15 19:37 [PATCH] [gdb/cli] Improve the pagination prompt Tom de Vries
2025-07-16 11:30 ` Eli Zaretskii
2025-07-17 10:07   ` Tom de Vries
2025-07-17 10:16     ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox