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?