GNU bug report logs - #55845
[PATCH 0/1] Improve pager selection logic when less is not installed

Previous Next

Full log

View this message in rfc822 format

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Taiju HIGASHI <higashi <at> taiju.info>
Cc: 55845 <at> debbugs.gnu.org
Subject: [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed
Date: Wed, 08 Jun 2022 14:14:34 +0200

[Message part 1 (text/plain, inline)]

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 (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.