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

From: Spencer Baugh <[email-masked]>
Date: Mon, 08 May 2023 15:38:12 -0400

Eli Zaretskii <[email-masked]> writes:

>> From: Spencer Baugh <[email-masked]>
>> Cc: [email-masked]
>> Date: Thu, 20 Apr 2023 12:01:49 -0400
>> 
>> Eli Zaretskii <[email-masked]> 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."