bug#62958: [PATCH] Set PAGER=cat in comint.el

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

Tags: patch


Paging is useless and annoying in comint-derived commands such as
async-shell-command and M-x shell.  It is a frequent footgun for new
Emacs users when they try to run commands which start a pager in such
modes.  I know a number of users who set PAGER=cat in their Emacs to
avoid it.

Simply adding (setenv "PAGER" "cat") globally is not correct, since
that will break modes like term, which support paging quite well.
It's only and exactly the comint-derived modes which don't need
paging.

Changing the default to "cat" in this way might be a bit
controversial...

In GNU Emacs 29.0.60 (build 3, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-03-13 built on
 igm-qws-u22796a
Repository revision: e759905d2e0828eac4c8164b09113b40f6899656
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: CentOS Linux 7 (Core)

Configured using:
 'configure --with-x-toolkit=lucid --with-modules
 --with-gif=ifavailable'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Set-PAGER-cat-in-comint.el.patch --]
[-- Type: text/patch, Size: 1915 bytes --]

From 0069a0253c73567b131ddc75d57e29d405d11a72 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Wed, 19 Apr 2023 17:44:54 -0400
Subject: [PATCH] Set PAGER=cat in comint.el

Paging is useless and annoying in comint-derived commands such as
async-shell-command and M-x shell.  It is a frequent footgun for new
Emacs users when they try to run commands which start a pager in such
modes.  I know a number of users who set PAGER=cat in their Emacs to
avoid it.

Simply adding (setenv "PAGER" "cat") globally is not correct, since
that will break modes like term, which support paging quite well.
It's only and exactly the comint-derived modes which don't need
paging.

Changing the default to "cat" in this way might be a bit
controversial...

* lisp/comint.el (comint-pager):
Add, defaulted to "cat".
(comint-exec-1):
Use comint-pager to set PAGER.
---
 lisp/comint.el | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lisp/comint.el b/lisp/comint.el
index 682b555a33c..915907c1bb9 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -258,6 +258,15 @@ comint-input-ring-file-name
 		 file)
   :group 'comint)
 
+(defcustom comint-pager "cat"
+  "If non-nil, the value to use for PAGER.
+gWhen this is nil, comint doesn't set PAGER at all."
+  :version "30.1"
+  :type '(choice (const :tag "Don't set PAGER" nil)
+                 (const :tag "cat" "cat")
+		 string)
+  :group 'comint)
+
 (defvar comint-input-ring-file-prefix nil
   "The prefix to skip when parsing the input ring file.
 This is useful in Zsh when the extended_history option is on.")
@@ -864,6 +873,7 @@ comint-exec-1
 	 (nconc
           (comint-term-environment)
 	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
+          (when comint-pager (list (format "PAGER=%s" comint-pager)))
 	  process-environment))
 	(default-directory
 	  (if (file-accessible-directory-p default-directory)
-- 
2.30.2

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Wed, 19 Apr 2023 17:57:38 -0400
> 
> Simply adding (setenv "PAGER" "cat") globally is not correct, since
> that will break modes like term, which support paging quite well.
> It's only and exactly the comint-derived modes which don't need
> paging.
> 
> Changing the default to "cat" in this way might be a bit
> controversial...

Sorry, this default cannot be universally correct.  You assume that
'cat' is always available, which is not true on non-Posix platforms.
So at the very least the value should be set according to
executable-find.

Having said that, I'm not really sure the default should be "cat" even
if it is available, since AFAIK you are the only one who is unhappy
with the current situation.  So why not leave the default value as it
is, i.e. nil?

> +(defcustom comint-pager "cat"
> +  "If non-nil, the value to use for PAGER.

This is too terse.  It should at least say that the value should be a
program name, a string.  Bonus point for explaining what is PAGER, for
those who don't necessarily know off-hand.

> +gWhen this is nil, comint doesn't set PAGER at all."
   ^
Typo.  Also, "set PAGER" is again too terse.

> +  :version "30.1"
> +  :type '(choice (const :tag "Don't set PAGER" nil)
> +                 (const :tag "cat" "cat")

The tag of "cat" is too terse again.  It should at least include the
word "program" somewhere.

> @@ -864,6 +873,7 @@ comint-exec-1
>  	 (nconc
>            (comint-term-environment)
>  	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
> +          (when comint-pager (list (format "PAGER=%s" comint-pager)))

Should this test that comint-pager is a string?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Wed, 19 Apr 2023 17:57:38 -0400
>> 
>> Simply adding (setenv "PAGER" "cat") globally is not correct, since
>> that will break modes like term, which support paging quite well.
>> It's only and exactly the comint-derived modes which don't need
>> paging.
>> 
>> Changing the default to "cat" in this way might be a bit
>> controversial...
>
> Sorry, this default cannot be universally correct.  You assume that
> 'cat' is always available, which is not true on non-Posix platforms.
> So at the very least the value should be set according to
> executable-find.

executable-find is not correct in the case of "cat" unfortunately,
because there are programs (git, for one) which if they see PAGER=cat,
just don't start a pager at all, for greater efficiency.  (which is
desirable behavior)

> Having said that, I'm not really sure the default should be "cat" even
> if it is available, since AFAIK you are the only one who is unhappy
> with the current situation.  So why not leave the default value as it
> is, i.e. nil?

Yes okay, I'll leave the default as it is.

>> +(defcustom comint-pager "cat"
>> +  "If non-nil, the value to use for PAGER.
>
> This is too terse.  It should at least say that the value should be a
> program name, a string.  Bonus point for explaining what is PAGER, for
> those who don't necessarily know off-hand.

Hm, yes, I have updated the docstring to explain the issue, it's worth
having a good explanation.  Hopefully this isn't too verbose now :)

>> +gWhen this is nil, comint doesn't set PAGER at all."
>    ^
> Typo.  Also, "set PAGER" is again too terse.
>
>> +  :version "30.1"
>> +  :type '(choice (const :tag "Don't set PAGER" nil)
>> +                 (const :tag "cat" "cat")
>
> The tag of "cat" is too terse again.  It should at least include the
> word "program" somewhere.
>
>> @@ -864,6 +873,7 @@ comint-exec-1
>>  	 (nconc
>>            (comint-term-environment)
>>  	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
>> +          (when comint-pager (list (format "PAGER=%s" comint-pager)))
>
> Should this test that comint-pager is a string?

I don't think that's necessary; doing
(if (stringp comint-pager) (list (format "PAGER=%s" comint-pager)))
would have unexpected behavior if comint-pager was accidentally set to a
non-string; doing
(when comint-pager (progn (assert (stringp comint-pager))
  (list (format "PAGER=%s" comint-pager))))
is a bit verbose and looks weird and is probably not that important.

Revised patch:

commit a375cb9c1260edb40a92f11fdec7f6910538135d
Author: Spencer Baugh <sbaugh@janestreet.com>
Date:   Wed Apr 19 17:44:54 2023 -0400

  Set PAGER=cat in comint.el
  
  Paging can be undesirable in comint-derived commands such as
  async-shell-command and M-x shell.  It is a frequent footgun for new
  Emacs users when they try to run commands which start a pager in such
  modes.
  
  Simply adding (setenv "PAGER" "cat") globally is not correct, since
  that will break modes like term, which support paging quite well.
  It's only and exactly the comint-derived modes which don't need
  paging.
  
  * lisp/comint.el (comint-pager):
  Add.
  (comint-exec-1):
  Use comint-pager to set PAGER.


diff --git a/lisp/comint.el b/lisp/comint.el
index 682b555a33c..37e189e4bbf 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -258,6 +258,49 @@ comint-input-ring-file-name
 		 file)
   :group 'comint)
 
+(defcustom comint-pager nil
+  "If non-nil, name of the program to use as a pager.
+
+If non-nil, comint sets the PAGER environment variable to this
+value before starting a subprocess.  PAGER controls the pager
+that will be used.  If you prefer to not use a pager, you can set
+this variable to \"cat\".
+
+If nil, the PAGER environment variable is not set and the default
+pager will be used.  On Unix systems, typically this is \"less\".
+
+Some programs start a pager before producing output.  A pager
+supports viewing text page by page, so that if the parent program
+produces more output than will fit on the screen, that output can
+be viewed incrementally.
+
+When a program is running under Emacs, this behavior can be
+undesirable, since Emacs itself supports viewing text page by
+page, and a pager requires input from the user before it will
+show more text.  Furthermore, comint is not a full fledged
+terminal emulator, so a pager will typically behave badly.
+
+However, pagers also provide backpressure: They will not consume
+more output from the parent program than the user has actually
+viewed, which on Unix means the output pipe will fill up and the
+parent program will be stopped from producing unnecessary output.
+Many programs (such as \"git log\") take advantage of this by
+producing large amounts of output by default and relying on the
+pager to not consume text that the user doesn't view.
+
+Emacs and comint do not keep track of what text the user has
+viewed, so they can't provide backpressure like a pager does.
+This means users who do not use a pager should be careful to not
+run commands which produce a lot of output.  Users can avoid this
+by limiting the amount of output (such as with \"git log -n10\")
+or by using native Emacs interfaces instead (such as
+`vc-print-log').")
+  :version "30.1"
+  :type '(choice (const :tag "Don't set PAGER" nil)
+                 (const :tag "Don't do paging (PAGER=cat)" "cat")
+		 string)
+  :group 'comint)
+
 (defvar comint-input-ring-file-prefix nil
   "The prefix to skip when parsing the input ring file.
 This is useful in Zsh when the extended_history option is on.")
@@ -864,6 +907,7 @@ comint-exec-1
 	 (nconc
           (comint-term-environment)
 	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
+          (when comint-pager (list (format "PAGER=%s" comint-pager)))
 	  process-environment))
 	(default-directory
 	  (if (file-accessible-directory-p default-directory)

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: 62958@debbugs.gnu.org
> Date: Thu, 20 Apr 2023 11:47:42 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Sorry, this default cannot be universally correct.  You assume that
> > 'cat' is always available, which is not true on non-Posix platforms.
> > So at the very least the value should be set according to
> > executable-find.
> 
> executable-find is not correct in the case of "cat" unfortunately,
> because there are programs (git, for one) which if they see PAGER=cat,
> just don't start a pager at all, for greater efficiency.  (which is
> desirable behavior)

Is this about removing the leading directories from the value of
executable-find?  If so, that is trivial to do, and is not the main
point of what I wrote.

> > Should this test that comint-pager is a string?
> 
> I don't think that's necessary; doing
> (if (stringp comint-pager) (list (format "PAGER=%s" comint-pager)))
> would have unexpected behavior if comint-pager was accidentally set to a
> non-string; doing
> (when comint-pager (progn (assert (stringp comint-pager))
>   (list (format "PAGER=%s" comint-pager))))
> is a bit verbose and looks weird and is probably not that important.

So we are okay with the user setting the variable to a symbol or a
list or a vector?

Thanks.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: 62958@debbugs.gnu.org
>> Date: Thu, 20 Apr 2023 11:47:42 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Sorry, this default cannot be universally correct.  You assume that
>> > 'cat' is always available, which is not true on non-Posix platforms.
>> > So at the very least the value should be set according to
>> > executable-find.
>> 
>> executable-find is not correct in the case of "cat" unfortunately,
>> because there are programs (git, for one) which if they see PAGER=cat,
>> just don't start a pager at all, for greater efficiency.  (which is
>> desirable behavior)
>
> Is this about removing the leading directories from the value of
> executable-find?  If so, that is trivial to do, and is not the main
> point of what I wrote.

Yes.  But anyway, the default is nil now, so it should be fine.  Unless
you'd like the custom option of "cat" to not show up on non-UNIX
platforms, somehow?

>> > Should this test that comint-pager is a string?
>> 
>> I don't think that's necessary; doing
>> (if (stringp comint-pager) (list (format "PAGER=%s" comint-pager)))
>> would have unexpected behavior if comint-pager was accidentally set to a
>> non-string; doing
>> (when comint-pager (progn (assert (stringp comint-pager))
>>   (list (format "PAGER=%s" comint-pager))))
>> is a bit verbose and looks weird and is probably not that important.
>
> So we are okay with the user setting the variable to a symbol or a
> list or a vector?

Fair enough, I added a check:

diff --git a/lisp/comint.el b/lisp/comint.el
index 682b555a33c..a145751565f 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -258,6 +258,49 @@ comint-input-ring-file-name
 		 file)
   :group 'comint)
 
+(defcustom comint-pager nil
+  "If non-nil, name of the program to use as a pager.
+
+If non-nil, comint sets the PAGER environment variable to this
+value before starting a subprocess.  PAGER controls the pager
+that will be used.  If you prefer to not use a pager, you can set
+this variable to \"cat\".
+
+If nil, the PAGER environment variable is not set and the default
+pager will be used.  On Unix systems, typically this is \"less\".
+
+Some programs start a pager before producing output.  A pager
+supports viewing text page by page, so that if the parent program
+produces more output than will fit on the screen, that output can
+be viewed incrementally.
+
+When a program is running under Emacs, this behavior can be
+undesirable, since Emacs itself supports viewing text page by
+page, and a pager requires input from the user before it will
+show more text.  Furthermore, comint is not a full fledged
+terminal emulator, so a pager will typically behave badly.
+
+However, pagers also provide backpressure: They will not consume
+more output from the parent program than the user has actually
+viewed, which on Unix means the output pipe will fill up and the
+parent program will be stopped from producing unnecessary output.
+Many programs (such as \"git log\") take advantage of this by
+producing large amounts of output by default and relying on the
+pager to not consume text that the user doesn't view.
+
+Emacs and comint do not keep track of what text the user has
+viewed, so they can't provide backpressure like a pager does.
+This means users who do not use a pager should be careful to not
+run commands which produce a lot of output.  Users can avoid this
+by limiting the amount of output (such as with \"git log -n10\")
+or by using native Emacs interfaces instead (such as
+`vc-print-log')."
+  :version "30.1"
+  :type '(choice (const :tag "Use default PAGER" nil)
+                 (const :tag "Don't do paging (PAGER=cat)" "cat")
+		 string)
+  :group 'comint)
+
 (defvar comint-input-ring-file-prefix nil
   "The prefix to skip when parsing the input ring file.
 This is useful in Zsh when the extended_history option is on.")
@@ -864,6 +907,10 @@ comint-exec-1
 	 (nconc
           (comint-term-environment)
 	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
+          (when comint-pager
+            (if (stringp comint-pager)
+                (list (format "PAGER=%s" comint-pager))
+              (error "comint-pager should be a string: %s" comint-pager)))
 	  process-environment))
 	(default-directory
 	  (if (file-accessible-directory-p default-directory)

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Wed, 19 Apr 2023 17:57:38 -0400
>> 
>> Simply adding (setenv "PAGER" "cat") globally is not correct, since
>> that will break modes like term, which support paging quite well.
>> It's only and exactly the comint-derived modes which don't need
>> paging.
>> 
>> Changing the default to "cat" in this way might be a bit
>> controversial...
>
> Sorry, this default cannot be universally correct.  You assume that
> 'cat' is always available, which is not true on non-Posix platforms.
> So at the very least the value should be set according to
> executable-find.

What systems respect PAGER (and would have it presumably set to
something like less), but would not provide cat?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Spencer Baugh <sbaugh@janestreet.com>,  62958@debbugs.gnu.org
> Date: Wed, 26 Apr 2023 07:54:34 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Changing the default to "cat" in this way might be a bit
> >> controversial...
> >
> > Sorry, this default cannot be universally correct.  You assume that
> > 'cat' is always available, which is not true on non-Posix platforms.
> > So at the very least the value should be set according to
> > executable-find.
> 
> What systems respect PAGER (and would have it presumably set to
> something like less), but would not provide cat?

PAGER is not a system-provided feature, it is provided by the programs
which honor the variable.  Any such program, when ported to a system
where there's no 'cat', will still support PAGER.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: 62958@debbugs.gnu.org
> Date: Thu, 20 Apr 2023 12:01:49 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Is this about removing the leading directories from the value of
> > executable-find?  If so, that is trivial to do, and is not the main
> > point of what I wrote.
> 
> Yes.  But anyway, the default is nil now, so it should be fine.  Unless
> you'd like the custom option of "cat" to not show up on non-UNIX
> platforms, somehow?
> 
> >> > Should this test that comint-pager is a string?
> >> 
> >> I don't think that's necessary; doing
> >> (if (stringp comint-pager) (list (format "PAGER=%s" comint-pager)))
> >> would have unexpected behavior if comint-pager was accidentally set to a
> >> non-string; doing
> >> (when comint-pager (progn (assert (stringp comint-pager))
> >>   (list (format "PAGER=%s" comint-pager))))
> >> is a bit verbose and looks weird and is probably not that important.
> >
> > So we are okay with the user setting the variable to a symbol or a
> > list or a vector?
> 
> Fair enough, I added a check:

Thanks.  This is almost ready to go, I have only 2 minor comments:


> diff --git a/lisp/comint.el b/lisp/comint.el
> index 682b555a33c..a145751565f 100644
> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -258,6 +258,49 @@ comint-input-ring-file-name
>  		 file)
>    :group 'comint)
>  
> +(defcustom comint-pager nil
> +  "If non-nil, name of the program to use as a pager.
> +
> +If non-nil, comint sets the PAGER environment variable to this
> +value before starting a subprocess.  PAGER controls the pager
> +that will be used.  If you prefer to not use a pager, you can set
> +this variable to \"cat\".
> +
> +If nil, the PAGER environment variable is not set and the default
> +pager will be used.  On Unix systems, typically this is \"less\".
> +
> +Some programs start a pager before producing output.  A pager
> +supports viewing text page by page, so that if the parent program
> +produces more output than will fit on the screen, that output can
> +be viewed incrementally.
> +
> +When a program is running under Emacs, this behavior can be
> +undesirable, since Emacs itself supports viewing text page by
> +page, and a pager requires input from the user before it will
> +show more text.  Furthermore, comint is not a full fledged
> +terminal emulator, so a pager will typically behave badly.
> +
> +However, pagers also provide backpressure: They will not consume
> +more output from the parent program than the user has actually
> +viewed, which on Unix means the output pipe will fill up and the
> +parent program will be stopped from producing unnecessary output.
> +Many programs (such as \"git log\") take advantage of this by
> +producing large amounts of output by default and relying on the
> +pager to not consume text that the user doesn't view.
> +
> +Emacs and comint do not keep track of what text the user has
> +viewed, so they can't provide backpressure like a pager does.
> +This means users who do not use a pager should be careful to not
> +run commands which produce a lot of output.  Users can avoid this
> +by limiting the amount of output (such as with \"git log -n10\")
> +or by using native Emacs interfaces instead (such as
> +`vc-print-log')."

This is too long, IMO.  There's no need for such a long doc string.  I
have tried to make it shorter without losing important details:

    "If non-nil, the program to use to disable pagination of program output.

  Some programs produce large amounts of output, and have provision for
  pagination of their output through a filter program, commonly known
  as a \"pager\".  The pager allows the user to interactively browse
  the output one page at a time.
  Some programs paginate their output by default, by always starting
  a pager.  The program they use as the pager is specified by the
  environment variable PAGER; if that variable is not defined, they
  use some fixed default, such as \"less\".

  Pagination is not needed, and gets in the way, when the output of
  the program is directed to an Emacs buffer, so in those cases
  pagination should be disabled.  To disable pagination, this
  variable's value should be a string that names a program, such
  as \"cat\", which passes through all of the output without any
  filtering or delays.  Comint will then set the PAGER variable
  to name that program, when it invokes external programs."

> +  :type '(choice (const :tag "Use default PAGER" nil)
> +                 (const :tag "Don't do paging (PAGER=cat)" "cat")
> +		 string)

Could we have a helpful :tag for the 'string' alternative?  It would
be good to explain there what kind of strings should be used: either a
full absolute file name or a basename that can be found via PATH.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: 62958@debbugs.gnu.org
>> Date: Thu, 20 Apr 2023 12:01:49 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> > Is this about removing the leading directories from the value of
>> > executable-find?  If so, that is trivial to do, and is not the main
>> > point of what I wrote.
>> 
>> Yes.  But anyway, the default is nil now, so it should be fine.  Unless
>> you'd like the custom option of "cat" to not show up on non-UNIX
>> platforms, somehow?
>> 
>> >> > Should this test that comint-pager is a string?
>> >> 
>> >> I don't think that's necessary; doing
>> >> (if (stringp comint-pager) (list (format "PAGER=%s" comint-pager)))
>> >> would have unexpected behavior if comint-pager was accidentally set to a
>> >> non-string; doing
>> >> (when comint-pager (progn (assert (stringp comint-pager))
>> >>   (list (format "PAGER=%s" comint-pager))))
>> >> is a bit verbose and looks weird and is probably not that important.
>> >
>> > So we are okay with the user setting the variable to a symbol or a
>> > list or a vector?
>> 
>> Fair enough, I added a check:
>
> Thanks.  This is almost ready to go, I have only 2 minor comments:
>
>
>> diff --git a/lisp/comint.el b/lisp/comint.el
>> index 682b555a33c..a145751565f 100644
>> --- a/lisp/comint.el
>> +++ b/lisp/comint.el
>> @@ -258,6 +258,49 @@ comint-input-ring-file-name
>>  		 file)
>>    :group 'comint)
>>  
>> +(defcustom comint-pager nil
>> +  "If non-nil, name of the program to use as a pager.
>> +
>> +If non-nil, comint sets the PAGER environment variable to this
>> +value before starting a subprocess.  PAGER controls the pager
>> +that will be used.  If you prefer to not use a pager, you can set
>> +this variable to \"cat\".
>> +
>> +If nil, the PAGER environment variable is not set and the default
>> +pager will be used.  On Unix systems, typically this is \"less\".
>> +
>> +Some programs start a pager before producing output.  A pager
>> +supports viewing text page by page, so that if the parent program
>> +produces more output than will fit on the screen, that output can
>> +be viewed incrementally.
>> +
>> +When a program is running under Emacs, this behavior can be
>> +undesirable, since Emacs itself supports viewing text page by
>> +page, and a pager requires input from the user before it will
>> +show more text.  Furthermore, comint is not a full fledged
>> +terminal emulator, so a pager will typically behave badly.
>> +
>> +However, pagers also provide backpressure: They will not consume
>> +more output from the parent program than the user has actually
>> +viewed, which on Unix means the output pipe will fill up and the
>> +parent program will be stopped from producing unnecessary output.
>> +Many programs (such as \"git log\") take advantage of this by
>> +producing large amounts of output by default and relying on the
>> +pager to not consume text that the user doesn't view.
>> +
>> +Emacs and comint do not keep track of what text the user has
>> +viewed, so they can't provide backpressure like a pager does.
>> +This means users who do not use a pager should be careful to not
>> +run commands which produce a lot of output.  Users can avoid this
>> +by limiting the amount of output (such as with \"git log -n10\")
>> +or by using native Emacs interfaces instead (such as
>> +`vc-print-log')."
>
> This is too long, IMO.  There's no need for such a long doc string.  I
> have tried to make it shorter without losing important details:
>
>     "If non-nil, the program to use to disable pagination of program output.
>
>   Some programs produce large amounts of output, and have provision for
>   pagination of their output through a filter program, commonly known
>   as a \"pager\".  The pager allows the user to interactively browse
>   the output one page at a time.
>   Some programs paginate their output by default, by always starting
>   a pager.  The program they use as the pager is specified by the
>   environment variable PAGER; if that variable is not defined, they
>   use some fixed default, such as \"less\".
>
>   Pagination is not needed, and gets in the way, when the output of
>   the program is directed to an Emacs buffer, so in those cases
>   pagination should be disabled.  To disable pagination, this
>   variable's value should be a string that names a program, such
>   as \"cat\", which passes through all of the output without any
>   filtering or delays.  Comint will then set the PAGER variable
>   to name that program, when it invokes external programs."

I think it's worth noting the fact that there are programs which depend
on pagination for their normal functionality.  How about this short
additional paragraph added after your text:

"Disabling pagination means programs will produce their entire output
immediately.  If only part of the output is useful, this can be
wasteful.  For some programs, this can be avoided by using commands (for
example, `vc-print-log') which run the program, limit the output it
produces, and let the user interactively browse the output inside
Emacs."

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: 62958@debbugs.gnu.org
> Date: Mon, 08 May 2023 15:38:12 -0400
> 
> >   Pagination is not needed, and gets in the way, when the output of
> >   the program is directed to an Emacs buffer, so in those cases
> >   pagination should be disabled.  To disable pagination, this
> >   variable's value should be a string that names a program, such
> >   as \"cat\", which passes through all of the output without any
> >   filtering or delays.  Comint will then set the PAGER variable
> >   to name that program, when it invokes external programs."
> 
> I think it's worth noting the fact that there are programs which depend
> on pagination for their normal functionality.  How about this short
> additional paragraph added after your text:
> 
> "Disabling pagination means programs will produce their entire output
> immediately.  If only part of the output is useful, this can be
> wasteful.  For some programs, this can be avoided by using commands (for
> example, `vc-print-log') which run the program, limit the output it
> produces, and let the user interactively browse the output inside
> Emacs."

Sorry, I don't understand: why would it matter that a program produces
its output immediately when that output is redirected to an Emacs
buffer?  Whatever the amount of output, the user can always page
through it interactively and conveniently using the normal movement
commands, no?  What am I missing?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[sbaugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: 62958@debbugs.gnu.org
>> Date: Mon, 08 May 2023 15:38:12 -0400
>> 
>> >   Pagination is not needed, and gets in the way, when the output of
>> >   the program is directed to an Emacs buffer, so in those cases
>> >   pagination should be disabled.  To disable pagination, this
>> >   variable's value should be a string that names a program, such
>> >   as \"cat\", which passes through all of the output without any
>> >   filtering or delays.  Comint will then set the PAGER variable
>> >   to name that program, when it invokes external programs."
>> 
>> I think it's worth noting the fact that there are programs which depend
>> on pagination for their normal functionality.  How about this short
>> additional paragraph added after your text:
>> 
>> "Disabling pagination means programs will produce their entire output
>> immediately.  If only part of the output is useful, this can be
>> wasteful.  For some programs, this can be avoided by using commands (for
>> example, `vc-print-log') which run the program, limit the output it
>> produces, and let the user interactively browse the output inside
>> Emacs."
>
> Sorry, I don't understand: why would it matter that a program produces
> its output immediately when that output is redirected to an Emacs
> buffer?  Whatever the amount of output, the user can always page
> through it interactively and conveniently using the normal movement
> commands, no?  What am I missing?

If the command produces a lot of output it can take a long time to run
and slow down Emacs.

For example, if a user disables pagination, runs M-x shell, and then
runs "git log" in the Emacs repo, it takes ~3.5 minutes to run and
produces ~1.5 million lines.  Even if all they want is to see the 10
most recent commits.  There are several negative effects of this, in
decreasing order of importance:

- The shell buffer now has way more output, which slows down navigation
and operations on that buffer forever after, including font lock.
- The user's shell is blocked during that whole time, preventing further
shell commands.
- git log producing unnecessary output wastes CPU and IO.

If "git log" is run with a pager, it can complete as soon as the user
exits the pager, and it only produces a few lines.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: sbaugh@catern.com
> Date: Tue, 09 May 2023 14:55:31 +0000 (UTC)
> Cc: Spencer Baugh <sbaugh@janestreet.com>, 62958@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Cc: 62958@debbugs.gnu.org
> >> Date: Mon, 08 May 2023 15:38:12 -0400
> >> 
> >> >   Pagination is not needed, and gets in the way, when the output of
> >> >   the program is directed to an Emacs buffer, so in those cases
> >> >   pagination should be disabled.  To disable pagination, this
> >> >   variable's value should be a string that names a program, such
> >> >   as \"cat\", which passes through all of the output without any
> >> >   filtering or delays.  Comint will then set the PAGER variable
> >> >   to name that program, when it invokes external programs."
> >> 
> >> I think it's worth noting the fact that there are programs which depend
> >> on pagination for their normal functionality.  How about this short
> >> additional paragraph added after your text:
> >> 
> >> "Disabling pagination means programs will produce their entire output
> >> immediately.  If only part of the output is useful, this can be
> >> wasteful.  For some programs, this can be avoided by using commands (for
> >> example, `vc-print-log') which run the program, limit the output it
> >> produces, and let the user interactively browse the output inside
> >> Emacs."
> >
> > Sorry, I don't understand: why would it matter that a program produces
> > its output immediately when that output is redirected to an Emacs
> > buffer?  Whatever the amount of output, the user can always page
> > through it interactively and conveniently using the normal movement
> > commands, no?  What am I missing?
> 
> If the command produces a lot of output it can take a long time to run
> and slow down Emacs.
> 
> For example, if a user disables pagination, runs M-x shell, and then
> runs "git log" in the Emacs repo, it takes ~3.5 minutes to run and
> produces ~1.5 million lines.  Even if all they want is to see the 10
> most recent commits.  There are several negative effects of this, in
> decreasing order of importance:
> 
> - The shell buffer now has way more output, which slows down navigation
> and operations on that buffer forever after, including font lock.
> - The user's shell is blocked during that whole time, preventing further
> shell commands.
> - git log producing unnecessary output wastes CPU and IO.
> 
> If "git log" is run with a pager, it can complete as soon as the user
> exits the pager, and it only produces a few lines.

But this is impossible in general, AFAIU: most comint clients aren't
prepared for interactive paging anyway.  If someone wants paging, they
should use "M-x term" or "M-x ansi-term", the terminal emulators that
Emacs provides.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Tue, 09 May 2023 14:55:31 +0000 (UTC)
>> Cc: Spencer Baugh <sbaugh@janestreet.com>, 62958@debbugs.gnu.org
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Spencer Baugh <sbaugh@janestreet.com>
>> >> Cc: 62958@debbugs.gnu.org
>> >> Date: Mon, 08 May 2023 15:38:12 -0400
>> >> 
>> >> >   Pagination is not needed, and gets in the way, when the output of
>> >> >   the program is directed to an Emacs buffer, so in those cases
>> >> >   pagination should be disabled.  To disable pagination, this
>> >> >   variable's value should be a string that names a program, such
>> >> >   as \"cat\", which passes through all of the output without any
>> >> >   filtering or delays.  Comint will then set the PAGER variable
>> >> >   to name that program, when it invokes external programs."
>> >> 
>> >> I think it's worth noting the fact that there are programs which depend
>> >> on pagination for their normal functionality.  How about this short
>> >> additional paragraph added after your text:
>> >> 
>> >> "Disabling pagination means programs will produce their entire output
>> >> immediately.  If only part of the output is useful, this can be
>> >> wasteful.  For some programs, this can be avoided by using commands (for
>> >> example, `vc-print-log') which run the program, limit the output it
>> >> produces, and let the user interactively browse the output inside
>> >> Emacs."
>> >
>> > Sorry, I don't understand: why would it matter that a program produces
>> > its output immediately when that output is redirected to an Emacs
>> > buffer?  Whatever the amount of output, the user can always page
>> > through it interactively and conveniently using the normal movement
>> > commands, no?  What am I missing?
>> 
>> If the command produces a lot of output it can take a long time to run
>> and slow down Emacs.
>> 
>> For example, if a user disables pagination, runs M-x shell, and then
>> runs "git log" in the Emacs repo, it takes ~3.5 minutes to run and
>> produces ~1.5 million lines.  Even if all they want is to see the 10
>> most recent commits.  There are several negative effects of this, in
>> decreasing order of importance:
>> 
>> - The shell buffer now has way more output, which slows down navigation
>> and operations on that buffer forever after, including font lock.
>> - The user's shell is blocked during that whole time, preventing further
>> shell commands.
>> - git log producing unnecessary output wastes CPU and IO.
>> 
>> If "git log" is run with a pager, it can complete as soon as the user
>> exits the pager, and it only produces a few lines.
>
> But this is impossible in general, AFAIU: most comint clients aren't
> prepared for interactive paging anyway.  If someone wants paging, they
> should use "M-x term" or "M-x ansi-term", the terminal emulators that
> Emacs provides.

Yes.  I of course know that, that's why I filed this bug :)

Nevertheless, today by default many programs run in comint clients will
run a pager, and even though it is a hassle and gets in the way inside
Emacs, it does prevent some programs from producing lots of output and
yielding the negative effects I mentioned previously:

>> - The shell buffer now has way more output, which slows down navigation
>> and operations on that buffer forever after, including font lock.
>> - The user's shell is blocked during that whole time, preventing further
>> shell commands.
>> - git log producing unnecessary output wastes CPU and IO.

Because of these negative effects which will start happening for some
programs when the user disables pagination, I think it's important for
users to know that there's alternatives for such programs: namely, using
native Emacs interfaces like vc-print-log.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> Date: Tue, 09 May 2023 12:30:42 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: sbaugh@catern.com
> >> Date: Tue, 09 May 2023 14:55:31 +0000 (UTC)
> >> Cc: Spencer Baugh <sbaugh@janestreet.com>, 62958@debbugs.gnu.org
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> >> Cc: 62958@debbugs.gnu.org
> >> >> Date: Mon, 08 May 2023 15:38:12 -0400
> >> >> 
> >> >> >   Pagination is not needed, and gets in the way, when the output of
> >> >> >   the program is directed to an Emacs buffer, so in those cases
> >> >> >   pagination should be disabled.  To disable pagination, this
> >> >> >   variable's value should be a string that names a program, such
> >> >> >   as \"cat\", which passes through all of the output without any
> >> >> >   filtering or delays.  Comint will then set the PAGER variable
> >> >> >   to name that program, when it invokes external programs."
> >> >> 
> >> >> I think it's worth noting the fact that there are programs which depend
> >> >> on pagination for their normal functionality.  How about this short
> >> >> additional paragraph added after your text:
> >> >> 
> >> >> "Disabling pagination means programs will produce their entire output
> >> >> immediately.  If only part of the output is useful, this can be
> >> >> wasteful.  For some programs, this can be avoided by using commands (for
> >> >> example, `vc-print-log') which run the program, limit the output it
> >> >> produces, and let the user interactively browse the output inside
> >> >> Emacs."
> >> >
> >> > Sorry, I don't understand: why would it matter that a program produces
> >> > its output immediately when that output is redirected to an Emacs
> >> > buffer?  Whatever the amount of output, the user can always page
> >> > through it interactively and conveniently using the normal movement
> >> > commands, no?  What am I missing?
> >> 
> >> If the command produces a lot of output it can take a long time to run
> >> and slow down Emacs.
> >> 
> >> For example, if a user disables pagination, runs M-x shell, and then
> >> runs "git log" in the Emacs repo, it takes ~3.5 minutes to run and
> >> produces ~1.5 million lines.  Even if all they want is to see the 10
> >> most recent commits.  There are several negative effects of this, in
> >> decreasing order of importance:
> >> 
> >> - The shell buffer now has way more output, which slows down navigation
> >> and operations on that buffer forever after, including font lock.
> >> - The user's shell is blocked during that whole time, preventing further
> >> shell commands.
> >> - git log producing unnecessary output wastes CPU and IO.
> >> 
> >> If "git log" is run with a pager, it can complete as soon as the user
> >> exits the pager, and it only produces a few lines.
> >
> > But this is impossible in general, AFAIU: most comint clients aren't
> > prepared for interactive paging anyway.  If someone wants paging, they
> > should use "M-x term" or "M-x ansi-term", the terminal emulators that
> > Emacs provides.
> 
> Yes.  I of course know that, that's why I filed this bug :)
> 
> Nevertheless, today by default many programs run in comint clients will
> run a pager, and even though it is a hassle and gets in the way inside
> Emacs, it does prevent some programs from producing lots of output and
> yielding the negative effects I mentioned previously:
> 
> >> - The shell buffer now has way more output, which slows down navigation
> >> and operations on that buffer forever after, including font lock.
> >> - The user's shell is blocked during that whole time, preventing further
> >> shell commands.
> >> - git log producing unnecessary output wastes CPU and IO.
> 
> Because of these negative effects which will start happening for some
> programs when the user disables pagination, I think it's important for
> users to know that there's alternatives for such programs: namely, using
> native Emacs interfaces like vc-print-log.

OK, but what's the purpose of the addition you were proposing?  It
basically says that sometimes paging should not be disabled, but since
this is a defcustom, and the default is not to disable paging, that
already speaks volumes about the same issue, right?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
>> Date: Tue, 09 May 2023 12:30:42 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: sbaugh@catern.com
>> >> Date: Tue, 09 May 2023 14:55:31 +0000 (UTC)
>> >> Cc: Spencer Baugh <sbaugh@janestreet.com>, 62958@debbugs.gnu.org
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> >> From: Spencer Baugh <sbaugh@janestreet.com>
>> >> >> Cc: 62958@debbugs.gnu.org
>> >> >> Date: Mon, 08 May 2023 15:38:12 -0400
>> >> >> 
>> >> >> >   Pagination is not needed, and gets in the way, when the output of
>> >> >> >   the program is directed to an Emacs buffer, so in those cases
>> >> >> >   pagination should be disabled.  To disable pagination, this
>> >> >> >   variable's value should be a string that names a program, such
>> >> >> >   as \"cat\", which passes through all of the output without any
>> >> >> >   filtering or delays.  Comint will then set the PAGER variable
>> >> >> >   to name that program, when it invokes external programs."
>> >> >> 
>> >> >> I think it's worth noting the fact that there are programs which depend
>> >> >> on pagination for their normal functionality.  How about this short
>> >> >> additional paragraph added after your text:
>> >> >> 
>> >> >> "Disabling pagination means programs will produce their entire output
>> >> >> immediately.  If only part of the output is useful, this can be
>> >> >> wasteful.  For some programs, this can be avoided by using commands (for
>> >> >> example, `vc-print-log') which run the program, limit the output it
>> >> >> produces, and let the user interactively browse the output inside
>> >> >> Emacs."
>> >> >
>> >> > Sorry, I don't understand: why would it matter that a program produces
>> >> > its output immediately when that output is redirected to an Emacs
>> >> > buffer?  Whatever the amount of output, the user can always page
>> >> > through it interactively and conveniently using the normal movement
>> >> > commands, no?  What am I missing?
>> >> 
>> >> If the command produces a lot of output it can take a long time to run
>> >> and slow down Emacs.
>> >> 
>> >> For example, if a user disables pagination, runs M-x shell, and then
>> >> runs "git log" in the Emacs repo, it takes ~3.5 minutes to run and
>> >> produces ~1.5 million lines.  Even if all they want is to see the 10
>> >> most recent commits.  There are several negative effects of this, in
>> >> decreasing order of importance:
>> >> 
>> >> - The shell buffer now has way more output, which slows down navigation
>> >> and operations on that buffer forever after, including font lock.
>> >> - The user's shell is blocked during that whole time, preventing further
>> >> shell commands.
>> >> - git log producing unnecessary output wastes CPU and IO.
>> >> 
>> >> If "git log" is run with a pager, it can complete as soon as the user
>> >> exits the pager, and it only produces a few lines.
>> >
>> > But this is impossible in general, AFAIU: most comint clients aren't
>> > prepared for interactive paging anyway.  If someone wants paging, they
>> > should use "M-x term" or "M-x ansi-term", the terminal emulators that
>> > Emacs provides.
>> 
>> Yes.  I of course know that, that's why I filed this bug :)
>> 
>> Nevertheless, today by default many programs run in comint clients will
>> run a pager, and even though it is a hassle and gets in the way inside
>> Emacs, it does prevent some programs from producing lots of output and
>> yielding the negative effects I mentioned previously:
>> 
>> >> - The shell buffer now has way more output, which slows down navigation
>> >> and operations on that buffer forever after, including font lock.
>> >> - The user's shell is blocked during that whole time, preventing further
>> >> shell commands.
>> >> - git log producing unnecessary output wastes CPU and IO.
>> 
>> Because of these negative effects which will start happening for some
>> programs when the user disables pagination, I think it's important for
>> users to know that there's alternatives for such programs: namely, using
>> native Emacs interfaces like vc-print-log.
>
> OK, but what's the purpose of the addition you were proposing?  It
> basically says that sometimes paging should not be disabled, but since
> this is a defcustom, and the default is not to disable paging, that
> already speaks volumes about the same issue, right?

My addition says:
- sometimes paging should not be disabled (for programs like "git log")
- but if you're going to disable it anyway, commands like `vc-print-log'
  become more useful (because it's an alternative to "git log")

The purpose of the addition is to communicate the second part.  (And
since I think the primary way most people will encounter this is through
"git log", I think it's worth mentioning `vc-print-log' specifically)

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> Date: Tue, 09 May 2023 12:53:24 -0400
> 
> > OK, but what's the purpose of the addition you were proposing?  It
> > basically says that sometimes paging should not be disabled, but since
> > this is a defcustom, and the default is not to disable paging, that
> > already speaks volumes about the same issue, right?
> 
> My addition says:
> - sometimes paging should not be disabled (for programs like "git log")
> - but if you're going to disable it anyway, commands like `vc-print-log'
>   become more useful (because it's an alternative to "git log")
> 
> The purpose of the addition is to communicate the second part.  (And
> since I think the primary way most people will encounter this is through
> "git log", I think it's worth mentioning `vc-print-log' specifically)

I'm asking why this is not already obvious from the fact that this is
a defcustom, by default off.  All your addition says is "don't disable
paging in all cases, sometimes it can be useful".  But we already said
that, in effect, by making the paging enabled by default.  So there's
no reason to have that text, as it isn't instrumental, and doesn't add
anything.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Tue, May 9, 2023, 12:58 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Spencer Baugh <sbaugh@janestreet.com>
> > Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> > Date: Tue, 09 May 2023 12:53:24 -0400
> >
> > > OK, but what's the purpose of the addition you were proposing?  It
> > > basically says that sometimes paging should not be disabled, but since
> > > this is a defcustom, and the default is not to disable paging, that
> > > already speaks volumes about the same issue, right?
> >
> > My addition says:
> > - sometimes paging should not be disabled (for programs like "git log")
> > - but if you're going to disable it anyway, commands like `vc-print-log'
> >   become more useful (because it's an alternative to "git log")
> >
> > The purpose of the addition is to communicate the second part.  (And
> > since I think the primary way most people will encounter this is through
> > "git log", I think it's worth mentioning `vc-print-log' specifically)
>
> I'm asking why this is not already obvious from the fact that this is
> a defcustom, by default off.  All your addition says is "don't disable
> paging in all cases, sometimes it can be useful".  But we already said
> that, in effect, by making the paging enabled by default.  So there's
> no reason to have that text, as it isn't instrumental, and doesn't add
> anything.
>

How does "this is a defcustom, by default off" communicate "you should use
vc-print-log if you turn this on"?

>

[-- Attachment #2: Type: text/html, Size: 2411 bytes --]

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
> Date: Tue, 09 May 2023 19:59:35 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> I'm asking why this is not already obvious from the fact that this is
> a defcustom, by default off.  All your addition says is "don't disable
> paging in all cases, sometimes it can be useful".  But we already said
> that, in effect, by making the paging enabled by default.  So there's
> no reason to have that text, as it isn't instrumental, and doesn't add
> anything.

Or let me turn the table and ask whether the following minor
rephrasing of what I proposed will satisfy you:

  Pagination is normally not needed, and might get in the way, when
  the output of the program is directed to an Emacs buffer, so in
  those cases pagination might need to be disabled.  To disable
  pagination, this variable's value should be a string that names a
  program, such as \"cat\", which passes through all of the output
  without any filtering or delays.  Comint will then set the PAGER
  variable to name that program, when it invokes external programs."

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Tue, 9 May 2023 13:01:07 -0400
> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
> 
>  I'm asking why this is not already obvious from the fact that this is
>  a defcustom, by default off.  All your addition says is "don't disable
>  paging in all cases, sometimes it can be useful".  But we already said
>  that, in effect, by making the paging enabled by default.  So there's
>  no reason to have that text, as it isn't instrumental, and doesn't add
>  anything.
> 
> How does "this is a defcustom, by default off" communicate "you should use vc-print-log if you turn this
> on"?

Why are we suddenly talking about vc-print-log when discussing a much
more general feature?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Tue, 9 May 2023 13:01:07 -0400
>> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
>> 
>>  I'm asking why this is not already obvious from the fact that this is
>>  a defcustom, by default off.  All your addition says is "don't disable
>>  paging in all cases, sometimes it can be useful".  But we already said
>>  that, in effect, by making the paging enabled by default.  So there's
>>  no reason to have that text, as it isn't instrumental, and doesn't add
>>  anything.
>> 
>> How does "this is a defcustom, by default off" communicate "you should use vc-print-log if you turn this
>> on"?
>
> Why are we suddenly talking about vc-print-log when discussing a much
> more general feature?

Because disabling pagination effectively breaks the shell command "git
log" and vc-print-log is one alternative to running "git log", and I
don't know how else the user would realize that.  (In my experience with
talking to users about this, they usually don't)

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> Date: Tue, 09 May 2023 13:13:56 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Date: Tue, 9 May 2023 13:01:07 -0400
> >> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
> >> 
> >>  I'm asking why this is not already obvious from the fact that this is
> >>  a defcustom, by default off.  All your addition says is "don't disable
> >>  paging in all cases, sometimes it can be useful".  But we already said
> >>  that, in effect, by making the paging enabled by default.  So there's
> >>  no reason to have that text, as it isn't instrumental, and doesn't add
> >>  anything.
> >> 
> >> How does "this is a defcustom, by default off" communicate "you should use vc-print-log if you turn this
> >> on"?
> >
> > Why are we suddenly talking about vc-print-log when discussing a much
> > more general feature?
> 
> Because disabling pagination effectively breaks the shell command "git
> log" and vc-print-log is one alternative to running "git log", and I
> don't know how else the user would realize that.  (In my experience with
> talking to users about this, they usually don't)

A doc string of a general option is not the proper place for talking
about some very specific case when that option is used.  If the main
problem is vc-print-log, we should find another way of dealing with
it.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Juri Linkov]

>> Sorry, I don't understand: why would it matter that a program produces
>> its output immediately when that output is redirected to an Emacs
>> buffer?  Whatever the amount of output, the user can always page
>> through it interactively and conveniently using the normal movement
>> commands, no?  What am I missing?
>
> If the command produces a lot of output it can take a long time to run
> and slow down Emacs.

I will definitely customize the new option 'comint-pager' to "cat"
as soon as it arrives to master.  But for such cases when some
commands produce too long output, would it be also possible
to provide an additional option to set the number of lines
to output before stopping?

It seems not possible just to set the ENV variable 'LINES'.
From 'man less':

  LINES  Sets the number of lines on the screen.  Takes precedence over the number  of
         lines  specified  by  the TERM variable.  (But if you have a windowing system
         which supports TIOCGWINSZ or WIOCGETD, the window system's idea of the screen
         size takes precedence over the LINES and COLUMNS environment variables.)

And indeed it has no effect when I tried.  Maybe because window size
adjustments are explicitly coded in 'window--adjust-process-windows'.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Spencer Baugh <sbaugh@janestreet.com>,
>   62958@debbugs.gnu.org
> Date: Wed, 10 May 2023 19:39:12 +0300
> 
> It seems not possible just to set the ENV variable 'LINES'.
> >From 'man less':
> 
>   LINES  Sets the number of lines on the screen.  Takes precedence over the number  of
>          lines  specified  by  the TERM variable.  (But if you have a windowing system
>          which supports TIOCGWINSZ or WIOCGETD, the window system's idea of the screen
>          size takes precedence over the LINES and COLUMNS environment variables.)
> 
> And indeed it has no effect when I tried.  Maybe because window size
> adjustments are explicitly coded in 'window--adjust-process-windows'.

Almost definitely because of that.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Gregory Heytings]

>
> I will definitely customize the new option 'comint-pager' to "cat" as 
> soon as it arrives to master.  But for such cases when some commands 
> produce too long output, would it be also possible to provide an 
> additional option to set the number of lines to output before stopping?
>

Did you try "cat | head -N"?

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Juri Linkov]

>> I will definitely customize the new option 'comint-pager' to "cat" as
>> soon as it arrives to master.  But for such cases when some commands
>> produce too long output, would it be also possible to provide an
>> additional option to set the number of lines to output before stopping?
>
> Did you try "cat | head -N"?

Now I tried out, and it works as expected.  Only there is no indication
that the output is truncated.  But probably such indication is not important
with overly long output.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Gregory Heytings]

>>> I will definitely customize the new option 'comint-pager' to "cat" as 
>>> soon as it arrives to master.  But for such cases when some commands 
>>> produce too long output, would it be also possible to provide an 
>>> additional option to set the number of lines to output before 
>>> stopping?
>>
>> Did you try "cat | head -N"?
>
> Now I tried out, and it works as expected.  Only there is no indication 
> that the output is truncated.  But probably such indication is not 
> important with overly long output.
>

If you want such an indication, I suggest using sed instead of head:

cat | sed -e "<N>aOUTPUT TRUNCATED" -e "<N>q"

(Of course, replace <N> with the number of lines of output you want.  And 
of course you can replace the string "OUTPUT TRUNCATED" with whatever you 
want.)

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
>> Date: Tue, 09 May 2023 13:13:56 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Spencer Baugh <sbaugh@janestreet.com>
>> >> Date: Tue, 9 May 2023 13:01:07 -0400
>> >> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
>> >> 
>> >>  I'm asking why this is not already obvious from the fact that this is
>> >>  a defcustom, by default off.  All your addition says is "don't disable
>> >>  paging in all cases, sometimes it can be useful".  But we already said
>> >>  that, in effect, by making the paging enabled by default.  So there's
>> >>  no reason to have that text, as it isn't instrumental, and doesn't add
>> >>  anything.
>> >> 
>> >> How does "this is a defcustom, by default off" communicate "you should use vc-print-log if you turn this
>> >> on"?
>> >
>> > Why are we suddenly talking about vc-print-log when discussing a much
>> > more general feature?
>> 
>> Because disabling pagination effectively breaks the shell command "git
>> log" and vc-print-log is one alternative to running "git log", and I
>> don't know how else the user would realize that.  (In my experience with
>> talking to users about this, they usually don't)
>
> A doc string of a general option is not the proper place for talking
> about some very specific case when that option is used.  If the main
> problem is vc-print-log, we should find another way of dealing with
> it.

OK, how about this doc string?

  "If non-nil, the program to use for pagination of program output.

Some programs produce large amounts of output, and have provision for
pagination of their output through a filter program, commonly known as
a \"pager\".  The pager limits the amount of output produced and
allows the user to interactively browse the output one page at a time.
Some programs paginate their output by default, by always starting a
pager.  The program they use as the pager is specified by the
environment variable PAGER; if that variable is not defined, they use
some fixed default, such as \"less\".

The interactive browsing aspects of pagination are not needed, and get
in the way, when the output of the program is directed to an Emacs
buffer, so in those cases pagination might need to be disabled.
Disabling pagination means that some programs will produce large
amounts of output, but most such programs have other ways to limit
their output, such as additional arguments or Emacs interfaces.
To disable pagination, this variable's value should be a string that
names a program, such as \"cat\", which passes through all of the
output without any filtering or delays.  Comint will then set the
PAGER variable to name that program, when it invokes external
programs."

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> Date: Tue, 16 May 2023 15:49:21 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> OK, how about this doc string?
> 
>   "If non-nil, the program to use for pagination of program output.
> 
> Some programs produce large amounts of output, and have provision for
> pagination of their output through a filter program, commonly known as
> a \"pager\".  The pager limits the amount of output produced and
> allows the user to interactively browse the output one page at a time.
> Some programs paginate their output by default, by always starting a
> pager.  The program they use as the pager is specified by the
> environment variable PAGER; if that variable is not defined, they use
> some fixed default, such as \"less\".
> 
> The interactive browsing aspects of pagination are not needed, and get
> in the way, when the output of the program is directed to an Emacs
> buffer, so in those cases pagination might need to be disabled.
> Disabling pagination means that some programs will produce large
> amounts of output, but most such programs have other ways to limit
> their output, such as additional arguments or Emacs interfaces.
> To disable pagination, this variable's value should be a string that
> names a program, such as \"cat\", which passes through all of the
> output without any filtering or delays.  Comint will then set the
> PAGER variable to name that program, when it invokes external
> programs."

SGTM, thanks.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Spencer Baugh]

[-- Attachment #1: Type: text/plain, Size: 17 bytes --]


Updated patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-setting-PAGER-cat-in-comint.el-bug-62958.patch --]
[-- Type: text/x-patch, Size: 3203 bytes --]

From 94db0faf148960eeace9279fea9e173b1d1ea516 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Wed, 19 Apr 2023 17:44:54 -0400
Subject: [PATCH] Support setting PAGER=cat in comint.el (bug#62958)

Paging can be undesirable in comint-derived commands such as
async-shell-command and M-x shell.  It is a frequent footgun for new
Emacs users when they try to run commands which start a pager in such
modes.

Simply adding (setenv "PAGER" "cat") globally is not correct, since
that will break modes like term, which support paging quite well.
It's only and exactly the comint-derived modes which don't need
paging.

* lisp/comint.el (comint-pager): Add. (bug#62958)
(comint-exec-1): Use comint-pager to set PAGER.
---
 lisp/comint.el | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/lisp/comint.el b/lisp/comint.el
index 682b555a33c..b956bd6f697 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -258,6 +258,35 @@ comint-input-ring-file-name
 		 file)
   :group 'comint)
 
+(defcustom comint-pager nil
+  "If non-nil, the program to use for pagination of program output.
+
+Some programs produce large amounts of output, and have provision for
+pagination of their output through a filter program, commonly known as
+a \"pager\".  The pager limits the amount of output produced and
+allows the user to interactively browse the output one page at a time.
+Some programs paginate their output by default, by always starting a
+pager.  The program they use as the pager is specified by the
+environment variable PAGER; if that variable is not defined, they use
+some fixed default, such as \"less\".
+
+The interactive browsing aspects of pagination are not needed, and get
+in the way, when the output of the program is directed to an Emacs
+buffer, so in those cases pagination might need to be disabled.
+Disabling pagination means that some programs will produce large
+amounts of output, but most such programs have other ways to limit
+their output, such as additional arguments or Emacs interfaces.
+To disable pagination, this variable's value should be a string that
+names a program, such as \"cat\", which passes through all of the
+output without any filtering or delays.  Comint will then set the
+PAGER variable to name that program, when it invokes external
+programs."
+  :version "30.1"
+  :type '(choice (const :tag "Use default PAGER" nil)
+                 (const :tag "Don't do paging (PAGER=cat)" "cat")
+                 (string :tag "Program name or absolute path of pager"))
+  :group 'comint)
+
 (defvar comint-input-ring-file-prefix nil
   "The prefix to skip when parsing the input ring file.
 This is useful in Zsh when the extended_history option is on.")
@@ -864,6 +893,10 @@ comint-exec-1
 	 (nconc
           (comint-term-environment)
 	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
+          (when comint-pager
+            (if (stringp comint-pager)
+                (list (format "PAGER=%s" comint-pager))
+              (error "comint-pager should be a string: %s" comint-pager)))
 	  process-environment))
 	(default-directory
 	  (if (file-accessible-directory-p default-directory)
-- 
2.30.2

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> Date: Wed, 17 May 2023 10:55:16 -0400
> 
> Updated patch.

LGTM, thanks.

bug#62958: [PATCH] Set PAGER=cat in comint.el

[Eli Zaretskii]

> Cc: sbaugh@catern.com, 62958@debbugs.gnu.org
> Date: Fri, 19 May 2023 09:09:54 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Spencer Baugh <sbaugh@janestreet.com>
> > Cc: sbaugh@catern.com,  62958@debbugs.gnu.org
> > Date: Wed, 17 May 2023 10:55:16 -0400
> > 
> > Updated patch.
> 
> LGTM, thanks.

No further comment, so I've now installed this on the master branch,
and I'm therefore closing this bug.

Thanks.