Dear, The first patch fixes the unexpected behaviour of "guix lint": guix lint -c description -n vs guix lint -n -c description Now, if '--no-network' and any checkers are provided using '--checkers' then the ones requiring Internet access are turned off. The second patch adds the '--no-checkers' option discussed some time ago. I am not convinced by the 'option-checker' helper function. What could be better? Instead of '--no-checkers' maybe '--exclude-checkers' is a better name. Last, note that '--list-checkers' operates as a dry-run: --8<---------------cut here---------------start------------->8--- ./pre-inst-env guix lint -c description,formatting,synopsis -n -x description -l Available checkers: - formatting: Look for formatting issues in the source --8<---------------cut here---------------end--------------->8--- All the best, simon zimoun (2): lint: Fix '--no-network' option. lint: Add '--no-checkers' option. doc/guix.texi | 9 +++++++ guix/scripts/lint.scm | 55 +++++++++++++++++++++++++++---------------- 2 files changed, 44 insertions(+), 20 deletions(-) base-commit: 89e1e4481382d18033a9773b90c09345fa33d6cb -- 2.28.0
* guix/scripts/lint.scm: (show-help): Add '--no-network' option message. (%options, parse-options): Fix argument order. * doc/guix.texi: Document it. --- doc/guix.texi | 4 ++++ guix/scripts/lint.scm | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index f224e356bc..ea2aa1581e 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -10632,6 +10632,10 @@ and exit. Only enable the checkers specified in a comma-separated list using the names returned by @option{--list-checkers}. +@item --no-network +@itemx -n +Only enable the checkers which do not dependent on Internet access. + @item --load-path=@var{directory} @itemx -L @var{directory} Add @var{directory} to the front of the package module search path diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 5168a1ca17..c56576fcbd 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -9,7 +9,7 @@ ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr> ;;; Copyright © 2017, 2018 Efraim Flashner <efraim@flashner.co.il> ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> -;;; Copyright © 2019 Simon Tournier <zimon.toutoune@gmail.com> +;;; Copyright © 2019, 2020 Simon Tournier <zimon.toutoune@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -98,6 +98,9 @@ run the checkers on all packages.\n")) (display (G_ " -c, --checkers=CHECKER1,CHECKER2... only run the specified checkers")) + (display (G_ " + -n, --no-network only run checkers which do not access to network")) + (display (G_ " -L, --load-path=DIR prepend DIR to the package module search path")) (newline) @@ -132,10 +135,7 @@ run the checkers on all packages.\n")) result)))) (option '(#\n "no-network") #f #f (lambda (opt name arg result) - (alist-cons 'checkers - %local-checkers - (alist-delete 'checkers - result)))) + (alist-cons 'no-network? #t result))) (find (lambda (option) (member "load-path" (option-names option))) %standard-build-options) @@ -169,7 +169,13 @@ run the checkers on all packages.\n")) value) (_ #f)) (reverse opts))) - (checkers (or (assoc-ref opts 'checkers) %all-checkers))) + (the-checkers (or (assoc-ref opts 'checkers) %all-checkers)) + (checkers + (if (assoc-ref opts 'no-network?) + (filter (lambda (checker) + (member checker %local-checkers)) + the-checkers) + the-checkers))) (when (assoc-ref opts 'list?) (list-checkers-and-exit checkers)) -- 2.28.0
* guix/scripts/lint.scm (%options, parse-options): Add '--no-checkers' option. * doc/guix.texi: Document it. --- doc/guix.texi | 5 +++++ guix/scripts/lint.scm | 39 ++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index ea2aa1581e..bcc6fe8324 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -10632,6 +10632,11 @@ and exit. Only enable the checkers specified in a comma-separated list using the names returned by @option{--list-checkers}. +@item --no-checkers +@itemx -x +Only disable the checkers specified in a comma-separated list using the +names returned by @option{--list-checkers}. + @item --no-network @itemx -n Only enable the checkers which do not dependent on Internet access. diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index c56576fcbd..0007e18bcd 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -99,6 +99,9 @@ run the checkers on all packages.\n")) -c, --checkers=CHECKER1,CHECKER2... only run the specified checkers")) (display (G_ " + -x, --no-checkers=CHECKER1,CHECKER2... + exclude the specified checkers")) + (display (G_ " -n, --no-network only run checkers which do not access to network")) (display (G_ " @@ -113,26 +116,29 @@ run the checkers on all packages.\n")) (newline) (show-bug-report-information)) +(define (option-checker short-long) + (option short-long #t #f + (lambda (opt name arg result) + (let ((names (map string->symbol (string-split arg #\,))) + (checker-names (map lint-checker-name %all-checkers))) + (for-each (lambda (c) + (unless (memq c checker-names) + (leave (G_ "~a: invalid checker~%") c))) + names) + (alist-cons (string->symbol (cadr short-long)) + (filter (lambda (checker) + (member (lint-checker-name checker) + names)) + %all-checkers) + result))))) (define %options ;; Specification of the command-line options. ;; TODO: add some options: ;; * --certainty=[low,medium,high]: only run checkers that have at least this ;; 'certainty'. - (list (option '(#\c "checkers") #t #f - (lambda (opt name arg result) - (let ((names (map string->symbol (string-split arg #\,))) - (checker-names (map lint-checker-name %all-checkers))) - (for-each (lambda (c) - (unless (memq c checker-names) - (leave (G_ "~a: invalid checker~%") c))) - names) - (alist-cons 'checkers - (filter (lambda (checker) - (member (lint-checker-name checker) - names)) - %all-checkers) - result)))) + (list (option-checker '(#\c "checkers")) + (option-checker '(#\x "no-checkers")) (option '(#\n "no-network") #f #f (lambda (opt name arg result) (alist-cons 'no-network? #t result))) @@ -169,7 +175,10 @@ run the checkers on all packages.\n")) value) (_ #f)) (reverse opts))) - (the-checkers (or (assoc-ref opts 'checkers) %all-checkers)) + (no-checkers (or (assoc-ref opts 'no-checkers) '())) + (the-checkers (filter (lambda (checker) + (not (member checker no-checkers))) + (or (assoc-ref opts 'checkers) %all-checkers))) (checkers (if (assoc-ref opts 'no-network?) (filter (lambda (checker) -- 2.28.0
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --] On Mon, Sep 07, 2020 at 08:02:29PM +0200, zimoun wrote: > Dear, > > The first patch fixes the unexpected behaviour of "guix lint": > > guix lint -c description -n > vs > guix lint -n -c description > > Now, if '--no-network' and any checkers are provided using '--checkers' then > the ones requiring Internet access are turned off. > I was going to say I didn't like the '-n' flag but I see it's already there, just not documented in the help message. > > The second patch adds the '--no-checkers' option discussed some time ago. I > am not convinced by the 'option-checker' helper function. What could be > better? > > Instead of '--no-checkers' maybe '--exclude-checkers' is a better name. > how about '--skip' > > Last, note that '--list-checkers' operates as a dry-run: > > --8<---------------cut here---------------start------------->8--- > ./pre-inst-env guix lint -c description,formatting,synopsis -n -x description -l > Available checkers: > - formatting: Look for formatting issues in the source > --8<---------------cut here---------------end--------------->8--- > > > All the best, > simon > > zimoun (2): > lint: Fix '--no-network' option. > lint: Add '--no-checkers' option. > > doc/guix.texi | 9 +++++++ > guix/scripts/lint.scm | 55 +++++++++++++++++++++++++++---------------- > 2 files changed, 44 insertions(+), 20 deletions(-) > > > base-commit: 89e1e4481382d18033a9773b90c09345fa33d6cb > -- > 2.28.0 > > > > -- Efraim Flashner <efraim@flashner.co.il> אפרים פלשנר GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --]
HI Efraim, Thank you for the feedback. On Tue, 8 Sep 2020 at 09:57, Efraim Flashner <efraim@flashner.co.il> wrote: > I was going to say I didn't like the '-n' flag but I see it's already > there, just not documented in the help message. Me neither. Since it is still undocumented, maybe we could change the short name. Well, I do not have a strong opinion. > > The second patch adds the '--no-checkers' option discussed some time ago. I > > am not convinced by the 'option-checker' helper function. What could be > > better? > > > > Instead of '--no-checkers' maybe '--exclude-checkers' is a better name. > > how about '--skip' With the short name '-s' or still '-x' ? Cheers, simon
friendly ping
On Mon, 07 Sep 2020 at 20:02, zimoun <zimon.toutoune@gmail.com> wrote:
> Dear,
>
> The first patch fixes the unexpected behaviour of "guix lint":
>
> guix lint -c description -n
> vs
> guix lint -n -c description
>
> Now, if '--no-network' and any checkers are provided using '--checkers' then
> the ones requiring Internet access are turned off.
>
>
> The second patch adds the '--no-checkers' option discussed some time ago. I
> am not convinced by the 'option-checker' helper function. What could be
> better?
>
> Instead of '--no-checkers' maybe '--exclude-checkers' is a better name.
>
>
> Last, note that '--list-checkers' operates as a dry-run:
>
> ./pre-inst-env guix lint -c description,formatting,synopsis -n -x description -l
> Available checkers:
> - formatting: Look for formatting issues in the source
>
>
> All the best,
> simon
>
> zimoun (2):
> lint: Fix '--no-network' option.
> lint: Add '--no-checkers' option.
>
> doc/guix.texi | 9 +++++++
> guix/scripts/lint.scm | 55 +++++++++++++++++++++++++++----------------
> 2 files changed, 44 insertions(+), 20 deletions(-)
>
>
> base-commit: 89e1e4481382d18033a9773b90c09345fa33d6cb
Hi, Efraim Flashner <efraim@flashner.co.il> skribis: > On Mon, Sep 07, 2020 at 08:02:29PM +0200, zimoun wrote: [...] >> The second patch adds the '--no-checkers' option discussed some time ago. I >> am not convinced by the 'option-checker' helper function. What could be >> better? >> >> Instead of '--no-checkers' maybe '--exclude-checkers' is a better name. >> > > how about '--skip' I’d suggest ‘--exclude’ + ‘-x’, which is similar to what ‘guix hash’ does for instance. Ludo’.
Hi, zimoun <zimon.toutoune@gmail.com> skribis: > * guix/scripts/lint.scm (%options, parse-options): Add '--no-checkers' option. > * doc/guix.texi: Document it. Please mention the section name (in the manual) and variable names. > +(define (option-checker short-long) > + (option short-long #t #f Indentation is off. Also please add a docstring. > + (lambda (opt name arg result) > + (let ((names (map string->symbol (string-split arg #\,))) > + (checker-names (map lint-checker-name %all-checkers))) > + (for-each (lambda (c) > + (unless (memq c checker-names) > + (leave (G_ "~a: invalid checker~%") c))) > + names) > + (alist-cons (string->symbol (cadr short-long)) Use ‘match’ instead of ‘cadr’, or maybe make it a parameter of this procedure? Thanks! Ludo’.
* guix/scripts/lint.scm: (show-help): Add '--no-network' option message. (%options, parse-options): Fix argument order. * doc/guix.texi (Invoking guix lint): Document it. --- doc/guix.texi | 4 ++++ guix/scripts/lint.scm | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 22bddf10e3..19cf26572c 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -11528,6 +11528,10 @@ and exit. Only enable the checkers specified in a comma-separated list using the names returned by @option{--list-checkers}. +@item --no-network +@itemx -n +Only enable the checkers which do not dependent on Internet access. + @item --load-path=@var{directory} @itemx -L @var{directory} Add @var{directory} to the front of the package module search path diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 979d4f8363..1ab563a3fa 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -9,7 +9,7 @@ ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr> ;;; Copyright © 2017, 2018 Efraim Flashner <efraim@flashner.co.il> ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac@systemreboot.net> -;;; Copyright © 2019 Simon Tournier <zimon.toutoune@gmail.com> +;;; Copyright © 2019, 2020 Simon Tournier <zimon.toutoune@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -98,6 +98,9 @@ run the checkers on all packages.\n")) (display (G_ " -c, --checkers=CHECKER1,CHECKER2... only run the specified checkers")) + (display (G_ " + -n, --no-network only run checkers which do not access to network")) + (display (G_ " -L, --load-path=DIR prepend DIR to the package module search path")) (newline) @@ -132,10 +135,7 @@ run the checkers on all packages.\n")) result)))) (option '(#\n "no-network") #f #f (lambda (opt name arg result) - (alist-cons 'checkers - %local-checkers - (alist-delete 'checkers - result)))) + (alist-cons 'no-network? #t result))) (find (lambda (option) (member "load-path" (option-names option))) %standard-build-options) @@ -172,7 +172,13 @@ run the checkers on all packages.\n")) value) (_ #f)) (reverse opts))) - (checkers (or (assoc-ref opts 'checkers) %all-checkers))) + (the-checkers (or (assoc-ref opts 'checkers) %all-checkers)) + (checkers + (if (assoc-ref opts 'no-network?) + (filter (lambda (checker) + (member checker %local-checkers)) + the-checkers) + the-checkers))) (when (assoc-ref opts 'list?) (list-checkers-and-exit checkers)) base-commit: d22d129da903cf6c3382cff5226d81a881fed2aa -- 2.28.0
* guix/scripts/lint.scm (%options, parse-options): Add '--exclude' option. (option-checker): New helper function. * doc/guix.texi (Invoking guix lint): Document it. --- doc/guix.texi | 5 +++++ guix/scripts/lint.scm | 44 ++++++++++++++++++++++++++++--------------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 19cf26572c..1c146026fd 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -11528,6 +11528,11 @@ and exit. Only enable the checkers specified in a comma-separated list using the names returned by @option{--list-checkers}. +@item --exclude +@itemx -x +Only disable the checkers specified in a comma-separated list using the +names returned by @option{--list-checkers}. + @item --no-network @itemx -n Only enable the checkers which do not dependent on Internet access. diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 1ab563a3fa..cfe1a41211 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -99,6 +99,9 @@ run the checkers on all packages.\n")) -c, --checkers=CHECKER1,CHECKER2... only run the specified checkers")) (display (G_ " + -x, --exclude=CHECKER1,CHECKER2... + exclude the specified checkers")) + (display (G_ " -n, --no-network only run checkers which do not access to network")) (display (G_ " @@ -113,26 +116,34 @@ run the checkers on all packages.\n")) (newline) (show-bug-report-information)) +(define (option-checker short-long) + ;; Factorize the creation of the two options -c/--checkers and -x/--exclude, + ;; see %options. The parameter SHORT-LONG is the list containing the short + ;; and long name. The alist uses the long name as symbol. + (option short-long #t #f + (lambda (opt name arg result) + (let ((names (map string->symbol (string-split arg #\,))) + (checker-names (map lint-checker-name %all-checkers)) + (option-name (string->symbol (match short-long + ((short long) long))))) + (for-each (lambda (c) + (unless (memq c checker-names) + (leave (G_ "~a: invalid checker~%") c))) + names) + (alist-cons option-name + (filter (lambda (checker) + (member (lint-checker-name checker) + names)) + %all-checkers) + result))))) (define %options ;; Specification of the command-line options. ;; TODO: add some options: ;; * --certainty=[low,medium,high]: only run checkers that have at least this ;; 'certainty'. - (list (option '(#\c "checkers") #t #f - (lambda (opt name arg result) - (let ((names (map string->symbol (string-split arg #\,))) - (checker-names (map lint-checker-name %all-checkers))) - (for-each (lambda (c) - (unless (memq c checker-names) - (leave (G_ "~a: invalid checker~%") c))) - names) - (alist-cons 'checkers - (filter (lambda (checker) - (member (lint-checker-name checker) - names)) - %all-checkers) - result)))) + (list (option-checker '(#\c "checkers")) + (option-checker '(#\x "exclude")) (option '(#\n "no-network") #f #f (lambda (opt name arg result) (alist-cons 'no-network? #t result))) @@ -172,7 +183,10 @@ run the checkers on all packages.\n")) value) (_ #f)) (reverse opts))) - (the-checkers (or (assoc-ref opts 'checkers) %all-checkers)) + (no-checkers (or (assoc-ref opts 'exclude) '())) + (the-checkers (filter (lambda (checker) + (not (member checker no-checkers))) + (or (assoc-ref opts 'checkers) %all-checkers))) (checkers (if (assoc-ref opts 'no-network?) (filter (lambda (checker) -- 2.28.0
Hi,
Thank you for the review. All the comments are included in v2.
On Wed, 28 Oct 2020 at 16:18, Ludovic Courtès <ludo@gnu.org> wrote:
> Use ‘match’ instead of ‘cadr’, or maybe make it a parameter of this
> procedure?
Well, it is a matter of taste. :-) Just to be sure, that’s the point:
--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(ice-9 match)
scheme@(guile-user)> (define foo '(hello world))
scheme@(guile-user)> (cadr foo)
$2 = world
scheme@(guile-user)> (match foo ((hi all) all))
$3 = world
--8<---------------cut here---------------end--------------->8---
right?
Cheers,
simon
[-- Attachment #1: Type: text/plain, Size: 260 bytes --] Hi! zimoun <zimon.toutoune@gmail.com> skribis: > * guix/scripts/lint.scm: (show-help): Add '--no-network' option message. > (%options, parse-options): Fix argument order. > * doc/guix.texi (Invoking guix lint): Document it. Applied with the changes below. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Type: text/x-patch, Size: 995 bytes --] diff --git a/doc/guix.texi b/doc/guix.texi index 9e276c547d..e3b92d86f9 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -11530,7 +11530,7 @@ names returned by @option{--list-checkers}. @item --no-network @itemx -n -Only enable the checkers which do not dependent on Internet access. +Only enable the checkers that do not depend on Internet access. @item --load-path=@var{directory} @itemx -L @var{directory} diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm index 1ab563a3fa..6833c60741 100644 --- a/guix/scripts/lint.scm +++ b/guix/scripts/lint.scm @@ -99,7 +99,7 @@ run the checkers on all packages.\n")) -c, --checkers=CHECKER1,CHECKER2... only run the specified checkers")) (display (G_ " - -n, --no-network only run checkers which do not access to network")) + -n, --no-network only run checkers that do not access the network")) (display (G_ " -L, --load-path=DIR prepend DIR to the package module search path"))
zimoun <zimon.toutoune@gmail.com> skribis:
> * guix/scripts/lint.scm (%options, parse-options): Add '--exclude' option.
> (option-checker): New helper function.
> * doc/guix.texi (Invoking guix lint): Document it.
Applied, thanks!
Ludo’.