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

[Eli Zaretskii <eliz@gnu.org>]

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