From patchwork Thu Jun 16 21:43:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ludovic_Court=C3=A8s?= X-Patchwork-Id: 40056 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 1DA0227BBEA; Thu, 16 Jun 2022 22:44:26 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 5023327BBE9 for ; Thu, 16 Jun 2022 22:44:25 +0100 (BST) Received: from localhost ([::1]:46962 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o1xHs-0002wU-BN for patchwork@mira.cbaines.net; Thu, 16 Jun 2022 17:44:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49220) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1xHX-0002wK-C3 for guix-patches@gnu.org; Thu, 16 Jun 2022 17:44:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:49373) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o1xHX-0006v7-1p for guix-patches@gnu.org; Thu, 16 Jun 2022 17:44:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o1xHX-0005MS-0C for guix-patches@gnu.org; Thu, 16 Jun 2022 17:44:03 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#55845] [PATCH 0/1] Improve pager selection logic when less is not installed Resent-From: Ludovic =?utf-8?q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 16 Jun 2022 21:44:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55845 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Taiju HIGASHI Cc: 55845@debbugs.gnu.org, me@tobias.gr, Maxime Devos Received: via spool by 55845-submit@debbugs.gnu.org id=B55845.165541584020577 (code B ref 55845); Thu, 16 Jun 2022 21:44:02 +0000 Received: (at 55845) by debbugs.gnu.org; 16 Jun 2022 21:44:00 +0000 Received: from localhost ([127.0.0.1]:43268 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1xHU-0005Lo-0G for submit@debbugs.gnu.org; Thu, 16 Jun 2022 17:44:00 -0400 Received: from eggs.gnu.org ([209.51.188.92]:36000) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o1xHP-0005LL-Ko for 55845@debbugs.gnu.org; Thu, 16 Jun 2022 17:43:59 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:50868) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1xHI-0006uO-Dy; Thu, 16 Jun 2022 17:43:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=acDZ8K3m5coi27/xZ75qXywsHJ97JIPutWBfSy/SmAg=; b=XW4tZOL4NC+EiUWBlYAb Clx4KIFZ+KdOf0H1G3hO0bH4o3tzUeni7306Axi30R/xdr8Hle8V7bINRBdXa7wXp8KgB+44JxEwU paS640uzhXdcxzMPN2VH2siUlht7s5Wc6AyWyxBWZ4KkwlAyf2iQNcFk8HUR+totkt7KOPKnR5R0a gIqczIhZRPYNlhk5L11mYkHO7N6owQEHDE7XtNA9uwOtSqg1DjxqBVw/y2710JcP8aI0shpud5NaY gflWJ25MGkTzvPJ2rjLcvttWRZI2tR+qKvIvWEYFVvvDC5XIY4kevfQuiCbJ73XCYTEHE4U0I22Lx DGw8PGLVL4aW7w==; Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=42600 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o1xHI-0000Ex-1Q; Thu, 16 Jun 2022 17:43:48 -0400 From: Ludovic =?utf-8?q?Court=C3=A8s?= References: <20220608102124.14865-1-higashi@taiju.info> <20220608102257.15042-1-higashi@taiju.info> <87leu72sbo.fsf@taiju.info> <55fe6cdbed891707aca419ff4eedd7c37ef3eb03.camel@telenet.be> <87fskexiyc.fsf@taiju.info> <874k0uxhif.fsf@taiju.info> <5ccb68a4dc80a02baaf6d19fd782145b6b62e7b5.camel@telenet.be> <87a6alwdvn.fsf@taiju.info> <87r13wvrlz.fsf@taiju.info> <8250aad90dcf563aba4103127df95084a25fbd5d.camel@telenet.be> <87bkuzsap7.fsf@taiju.info> Date: Thu, 16 Jun 2022 23:43:46 +0200 In-Reply-To: <87bkuzsap7.fsf@taiju.info> (Taiju HIGASHI's message of "Sat, 11 Jun 2022 20:26:12 +0900") Message-ID: <87a6acjncd.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Hi, Taiju HIGASHI skribis: >>From bf557600c549e22a06ccfb288b89b1a0736b0500 Mon Sep 17 00:00:00 2001 > From: Taiju HIGASHI > Date: Wed, 8 Jun 2022 18:50:28 +0900 > Subject: [PATCH v4] ui: Improve pager selection logic when less is not > installed. > > * guix/ui.scm (find-available-pager): New procedure. Return a available pager. > (call-with-paginated-output-port): Change to use find-available-pager to > select pager. > * guix/utils.scm (call-with-environment-variables): Allow clearing of > specified environment variables. > * tests/ui.scm: Add tests for find-available-pager. Applied with the cosmetic changes below, mostly aiming to visually simplify the code and make it consistent with the rest. It’s great that you went to great lengths to implement tests for this, as Maxime had suggested. To me, the complexity of a test must be justified by its “bug-finding performance”; in this particular case, I think we’re borderline: the tests are a little bit complex and unlikely to find new bugs. Thanks for all the work and for your feedback on your experience programming with Guile! Ludo’. diff --git a/guix/ui.scm b/guix/ui.scm index 93707a7a4b..a7acd41440 100644 --- a/guix/ui.scm +++ b/guix/ui.scm @@ -1674,15 +1674,13 @@ (define* (pager-wrapped-port #:optional (port (current-output-port))) #f))) (define (find-available-pager) - "Returns the program name or path of an available pager. -If neither less nor more is installed, return an empty string so that -call-with-paginated-output-port will not call pager." + "Return the program name of an available pager or the empty string if none is +available." (or (getenv "GUIX_PAGER") (getenv "PAGER") (which "less") (which "more") - "" ;; Returns an empty string so that call-with-paginated-output-port does not call pager. - )) + "")) (define* (call-with-paginated-output-port proc #:key (less-options "FrX")) diff --git a/tests/ui.scm b/tests/ui.scm index ff83e66a7e..6a25a204ca 100644 --- a/tests/ui.scm +++ b/tests/ui.scm @@ -294,13 +294,12 @@ (define guile-2.0.9 (>0 (package-relevance libb2 (map rx '("crypto" "library"))))))) -(define make-dummy-file - (compose - close-port - open-output-file - (cut in-vicinity <> <>))) +(define (make-empty-file directory file) + ;; Create FILE in DIRECTORY. + (close-port (open-output-file (in-vicinity directory file)))) (define (assert-equals-find-available-pager expected) + ;; Use 'with-paginated-output-port' and return true if it invoked EXPECTED. (define used-command "") (mock ((ice-9 popen) open-pipe* (lambda (mode command . args) @@ -314,56 +313,51 @@ (define used-command "") (string=? expected used-command))))) -(test-assert "find-available-pager, All environment variables are specified and both less and more are installed" +(test-assert "find-available-pager, GUIX_PAGER takes precedence" (call-with-temporary-directory (lambda (dir) - (with-environment-variables - `(("PATH" ,dir) - ("GUIX_PAGER" "guix-pager") - ("PAGER" "pager")) - (make-dummy-file dir "less") - (make-dummy-file dir "more") + (with-environment-variables `(("PATH" ,dir) + ("GUIX_PAGER" "guix-pager") + ("PAGER" "pager")) + (make-empty-file dir "less") + (make-empty-file dir "more") (assert-equals-find-available-pager "guix-pager"))))) -(test-assert "find-available-pager, GUIX_PAGER is not specified" +(test-assert "find-available-pager, PAGER takes precedence" (call-with-temporary-directory (lambda (dir) - (with-environment-variables - `(("PATH" ,dir) - ("GUIX_PAGER" #false) - ("PAGER" "pager")) - (make-dummy-file dir "less") - (make-dummy-file dir "more") + (with-environment-variables `(("PATH" ,dir) + ("GUIX_PAGER" #false) + ("PAGER" "pager")) + (make-empty-file dir "less") + (make-empty-file dir "more") (assert-equals-find-available-pager "pager"))))) -(test-assert "find-available-pager, All environment variables are not specified and both less and more are installed" +(test-assert "find-available-pager, 'less' takes precedence" (call-with-temporary-directory (lambda (dir) - (with-environment-variables - `(("PATH" ,dir) - ("GUIX_PAGER" #false) - ("PAGER" #false)) - (make-dummy-file dir "less") - (make-dummy-file dir "more") + (with-environment-variables `(("PATH" ,dir) + ("GUIX_PAGER" #false) + ("PAGER" #false)) + (make-empty-file dir "less") + (make-empty-file dir "more") (assert-equals-find-available-pager (in-vicinity dir "less")))))) -(test-assert "find-available-pager, All environment variables are not specified and more is installed" +(test-assert "find-available-pager, 'more' takes precedence" (call-with-temporary-directory (lambda (dir) - (with-environment-variables - `(("PATH" ,dir) - ("GUIX_PAGER" #false) - ("PAGER" #false)) - (make-dummy-file dir "more") + (with-environment-variables `(("PATH" ,dir) + ("GUIX_PAGER" #false) + ("PAGER" #false)) + (make-empty-file dir "more") (assert-equals-find-available-pager (in-vicinity dir "more")))))) -(test-assert "find-available-pager, All environment variables are not specified and both less and more are not installed" +(test-assert "find-available-pager, no pager" (call-with-temporary-directory (lambda (dir) - (with-environment-variables - `(("PATH" ,dir) - ("GUIX_PAGER" #false) - ("PAGER" #false)) + (with-environment-variables `(("PATH" ,dir) + ("GUIX_PAGER" #false) + ("PAGER" #false)) (assert-equals-find-available-pager ""))))) (test-end "ui")