| Message ID | 20201115184726.29944-1-me@tobias.gr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#44663] ui: Launch $PAGER through the shell. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Hi, Tobias Geerinckx-Rice <me@tobias.gr> skribis: > This is the convention elsewhere and sounds like the right thing to do. > > * guix/ui.scm (call-with-paginated-output-port): Substitute OPEN-PIPE > for OPEN-PIPE*. > > Reported by Daniel Brooks <db48x@db48x.net>. What’s the rationale though? Are there cases where this makes a practical difference? I’m all for avoiding the shell if there’s no need for it. Thanks, Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > What’s the rationale though? Are there cases where this makes a > practical difference? The error I hit was effectively running PAGER=less -FXRS guix search foo. Everything else that uses PAGER handles this case just fine. db48x
Daniel Brooks <db48x@db48x.net> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> What’s the rationale though? Are there cases where this makes a >> practical difference? > > The error I hit was effectively running PAGER=less -FXRS guix search > foo. Everything else that uses PAGER handles this case just fine. Oh, I see, hmm. I feel that going through the shell makes things more brittle, but you describe a valid use case, so maybe we should just go ahead and apply the patch Tobias posted. Ludo’.
> I feel that going through the shell makes things more brittle
I don't think it's brittle but it's icky. Using OPEN-PIPE is just
the easiest way to get the shell to do free parsing for us. It
also allows using pipes and for-loops in PAGER so I agree it's not
ideal.
Here's a v2 that does more work for less result, but feels less
dirty :-)
Kind regards,
T G-R
diff --git a/guix/ui.scm b/guix/ui.scm index 4e686297e8..2b7d9dd64b 100644 --- a/guix/ui.scm +++ b/guix/ui.scm @@ -72,7 +72,7 @@ #:use-module (ice-9 match) #:use-module (ice-9 format) #:use-module (ice-9 regex) - #:autoload (ice-9 popen) (open-pipe* close-pipe) + #:autoload (ice-9 popen) (open-pipe close-pipe) #:autoload (system base compile) (compile-file) #:autoload (system repl repl) (start-repl) #:autoload (system repl debug) (make-debug stack->vector) @@ -1673,9 +1673,9 @@ 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* OPEN_WRITE - (or (getenv "GUIX_PAGER") (getenv "PAGER") - "less"))))) + (open-pipe (or (getenv "GUIX_PAGER") (getenv "PAGER") + "less") + OPEN_WRITE)))) (dynamic-wind (const #t) (lambda () (proc pager))