Subject: Re: bug#63778: [PATCH] Use comint-pager in eshell

From: Morgan Smith <[email-masked]>
Date: Mon, 29 May 2023 02:23:31 -0400

Jim Porter <[email-masked]> writes:

>
> Ah, in that case, then I think you'd want to change the logic in esh-var.el so
> that, when 'comint-pager' is nil, $PAGER returns the real value of PAGER from
> the environment. Since this behavior is opt-in, I think it would be enough to
> just make this fix (and ignore the visual command stuff), though special
> handling for visual commands would still be nice to have.
>

I decided to work on this instead of sleeping so I apologize if these
patches are of poor quality.

I still haven't looked into how everything interacts with the visual
commands but it took some real effort (maybe because I'm tired :P) to
get what I got so far and I think it's good enough.

My main pain point was trying to figure out how to maintain the ability
to set/unset the PAGER variable.  These current patches allow you to
set/unset the PAGER variable iff you don't set comint-pager.  It even
allows you to do stuff like 'PAGER=cat git log' (see
eshell-handle-local-variables) without modifying buffer state (as it
should).  Maintaining those capabilities when comint-pager is set seems
very difficult so I gave up.

Trying to setq-local comint-pager in the set function might honestly be
a better user experience for those that set comint-pager but then doing
'PAGER=cat git log' would cause a permanent buffer local change.

So currently everything works 100% great and as expected if comint-pager
is nil.  If comint-pager is not-nil then you cannot set PAGER in a way
that will take any affect.

>From 506849752916ac016457d891f4378d7d2116b0df Mon Sep 17 00:00:00 2001
From: Morgan Smith <[email-masked]>
Date: Mon, 29 May 2023 02:04:42 -0400
Subject: [PATCH 1/2] Fix infinite loop in eshell-{get|set}-variable

* lisp/eshell/esh-var.el (eshell-get-variable, eshell-set-variable):
Short circuit if the name and target are the same.
---
 lisp/eshell/esh-var.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 7dcaff1e24f..ecb0dfe274b 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -682,6 +682,8 @@ If QUOTED is non-nil, this was invoked inside double-quotes."
                 (funcall target indices)))))
          ((symbolp target)
           (eshell-apply-indices (symbol-value target) indices quoted))
+         ((and (equal name target))
+          (getenv target))
          (t
           (eshell-get-variable target indices quoted))))
     (unless (stringp name)
@@ -724,6 +726,8 @@ to a Lisp variable)."
          ((and eshell-prefer-lisp-variables
                (stringp target))
           (eshell-set-variable (intern target) value))
+         ((and (equal name target))
+          (setenv target value))
          (t
           (eshell-set-variable target value))))
     (cond
-- 
2.40.1

>From 01a9f036988bfa3d00b9d837dbacb2666918bc2e Mon Sep 17 00:00:00 2001
From: Morgan Smith <[email-masked]>
Date: Mon, 29 May 2023 02:06:48 -0400
Subject: [PATCH 2/2] Use comint-pager in eshell

* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Set $PAGER
environment variable from comint-pager variable
---
 lisp/eshell/esh-var.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index ecb0dfe274b..b036024f065 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -162,6 +162,7 @@ if they are quoted with a backslash."
     ("COLUMNS" ,(lambda () (window-body-width nil 'remap)) t t)
     ("LINES" ,(lambda () (window-body-height nil 'remap)) t t)
     ("INSIDE_EMACS" eshell-inside-emacs t)
+    ("PAGER" (,(lambda () (or comint-pager (getenv "PAGER"))) . "PAGER") t t)
     ("UID" ,(lambda () (file-user-uid)) nil t)
 
     ;; for esh-ext.el
-- 
2.40.1

Thanks,

Morgan