[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#55845] [PATCH 0/1] Improve pager selection logic when less is not i
From: |
Tobias Geerinckx-Rice |
Subject: |
[bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed |
Date: |
Wed, 08 Jun 2022 14:14:34 +0200 |
Hi!
Taiju HIGASHI 写道:
The problem rarely occurs, but when we run guix commands in an
environment
where "less" is not installed we get an error.
True. Odd that it's gone unreported(?) for so long.
I am concerned about performance degradation due to more
unnecessary
processing.
Since you asked… :-)
One way that this is ‘expensive’ is that it always calls WHICH at
least once, no matter what Guix was invoked to do.
If you're familiar with Haskell or Nix: Scheme is not that, it's
not ‘lazy’ and will evaluate the (if (which "less") …) even when
the value is never used. Turning AVAILABLE-PAGER into a procedure
would avoid that.
Also, you're looking up the final pager in $PATH twice: you call
WHICH, but then discard its work by returning the relative string
"less".
The final OPEN-PIPE* invokes a shell which will search $PATH
again. We could save it the trouble by returning an absolute file
name: the result of WHICH.
And since WHICH returns #f on failure, you can replace the nested
IFs with a single OR:
(define (available-pager)
(or (which "less")
(which "more")))
And well, as you probably noticed by now, it's actually more clear
and concise if we just in-line what little is left:
(let ((pager-command-line (or (getenv "GUIX_PAGER")
(getenv "PAGER")
(which "less")
(which "more")
"")))
…
Your original patch returns #f if no pages could be found. I
don't think that is handled, but "" is, so return that instead.
Now I think that's 100% equivalent to your original; let me know
if I missed a spot.
Also, if you feel that this is a minor issue and not worth
addressing, please
feel free to dismiss it. (Still, a fix to make the error message
more friendly
might be a good idea.)
It *is* minor, but then so is the fix, and as written above it
doesn't add ‘overhead’. I think it's a good idea to check for
"more" (but no more) and silently disable paging otherwise.
Thanks!
T G-R
signature.asc
Description: PGP signature
- [bug#55845] [PATCH 1/1] ui: Improve pager selection logic when less is not installed., (continued)
- [bug#55845] [PATCH 1/1] ui: Improve pager selection logic when less is not installed., Maxime Devos, 2022/06/15
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Ludovic Courtès, 2022/06/16
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Taiju HIGASHI, 2022/06/16
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Ludovic Courtès, 2022/06/17
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Taiju HIGASHI, 2022/06/17
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Ludovic Courtès, 2022/06/17
- [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed, Taiju HIGASHI, 2022/06/18
- [bug#55845] [PATCH 1/1] ui: Improve pager selection logic when less is not installed., Taiju HIGASHI, 2022/06/09
- [bug#55845] [PATCH 1/1] ui: Improve pager selection logic when less is not installed., Maxime Devos, 2022/06/10
- [bug#55845] [PATCH 1/1] ui: Improve pager selection logic when less is not installed., Taiju HIGASHI, 2022/06/10
[bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed,
Tobias Geerinckx-Rice <=