[bug#44663] [PATCH] ui: Launch $PAGER through the shell.

[Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org>]

[-- Attachment #1.1: Type: text/plain, Size: 702 bytes --]

Guix,

Tobias Geerinckx-Rice via Guix-patches via 写道：
> This is the convention elsewhere and sounds like the right thing 
> to do.

Before this patch, using PAGER= failed:

  $ PAGER= guix search e
  In execvp of : No such file or directory
  $

With it, it fails in a slightly worse way:

  $ PAGER= guix search e
  $ # nothing, because we spawn the shell that swallows all

Attached are two possible solutions.  One falls back to ‘less’, 
the other to no paging.

I think I prefer the former (‘Ignore empty $PAGER variables’) 
because the concept of ‘unset but empty’ could confuse ‘users’. 
Is that too patronising?  Do tell.

Kind regards,

T G-R


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-ui-Ignore-empty-PAGER-variables.patch --]
[-- Type: text/x-patch, Size: 1336 bytes --]

From dc64aadd9b124df37fcdf2f6dc057b61cf05a473 Mon Sep 17 00:00:00 2001
From: Tobias Geerinckx-Rice <me@tobias.gr>
Date: Sun, 15 Nov 2020 20:36:32 +0100
Subject: [PATCH] ui: Ignore empty $PAGER variables.

* guix/ui.scm (call-with-paginated-output-port): Treat "" as unset.
---
 guix/ui.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/guix/ui.scm b/guix/ui.scm
index 66614eef7c..584afc65dd 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -1673,8 +1673,11 @@ zero means that PACKAGE does not match any of REGEXPS."
       ;; instead of 'r': this strips hyperlinks but allows 'less' to make a
       ;; good estimate of the line length.
       (let ((pager (with-environment-variables `(("LESS" ,less-options))
-                     (open-pipe (or (getenv "GUIX_PAGER") (getenv "PAGER")
-                                    "less")
+                     ;; Ignore environment variables set to "" as if unset.
+                     (open-pipe (find (lambda (s) (and s (not (string=? "" s))))
+                                      (list (getenv "GUIX_PAGER")
+                                            (getenv "PAGER")
+                                            "less"))
                                 OPEN_WRITE))))
         (dynamic-wind
           (const #t)
-- 
2.29.2


[-- Attachment #1.3: 0001-ui-Disable-paging-if-PAGER-is-set-to-the-empty-strin.patch --]
[-- Type: text/x-patch, Size: 2961 bytes --]

From e1cf7e852c4a4c0cfce8c0de5625d026229dd71b Mon Sep 17 00:00:00 2001
From: Tobias Geerinckx-Rice <me@tobias.gr>
Date: Sun, 15 Nov 2020 20:26:54 +0100
Subject: [PATCH] ui: Disable paging if $PAGER is set to the empty string.

* guix/ui.scm (call-with-paginated-output-port): Don't open a pipe if $PAGER is "".
---
 guix/ui.scm | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/guix/ui.scm b/guix/ui.scm
index 66614eef7c..bb03e06759 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -1663,24 +1663,27 @@ zero means that PACKAGE does not match any of REGEXPS."
 
 (define* (call-with-paginated-output-port proc
                                           #:key (less-options "FrX"))
-  (if (isatty?* (current-output-port))
-      ;; Set 'LESS' so that 'less' exits if everything fits on the screen (F),
-      ;; lets ANSI escapes through (r), does not send the termcap
-      ;; initialization string (X).  Set it unconditionally because some
-      ;; distros set it to something that doesn't work here.
-      ;;
-      ;; For things that produce long lines, such as 'guix processes', use 'R'
-      ;; instead of 'r': this strips hyperlinks but allows 'less' to make a
-      ;; good estimate of the line length.
-      (let ((pager (with-environment-variables `(("LESS" ,less-options))
-                     (open-pipe (or (getenv "GUIX_PAGER") (getenv "PAGER")
-                                    "less")
-                                OPEN_WRITE))))
-        (dynamic-wind
-          (const #t)
-          (lambda () (proc pager))
-          (lambda () (close-pipe pager))))
-      (proc (current-output-port))))
+  (let ((command (or (getenv "GUIX_PAGER") (getenv "PAGER")
+                     "less")))
+    ;; If a user types ‘PAGER= guix foo’ their intention is probably to disable
+    ;; paging entirely, not to use Guix's default pager.
+    (if (and (not (string=? "" command))
+             (isatty?* (current-output-port)))
+        ;; Set 'LESS' so that 'less' exits if everything fits on the screen (F),
+        ;; lets ANSI escapes through (r), does not send the termcap
+        ;; initialization string (X).  Set it unconditionally because some
+        ;; distros set it to something that doesn't work here.
+        ;;
+        ;; For things that produce long lines, such as 'guix processes', use 'R'
+        ;; instead of 'r': this strips hyperlinks but allows 'less' to make a
+        ;; good estimate of the line length.
+        (let ((pager (with-environment-variables `(("LESS" ,less-options))
+                       (open-pipe command OPEN_WRITE))))
+          (dynamic-wind
+            (const #t)
+            (lambda () (proc pager))
+            (lambda () (close-pipe pager))))
+        (proc (current-output-port)))))
 
 (define-syntax with-paginated-output-port
   (syntax-rules ()
-- 
2.29.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]