guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] guix package: add a "show" option.


From: Ludovic Courtès
Subject: Re: [PATCH] guix package: add a "show" option.
Date: Tue, 15 Jul 2014 23:23:15 +0200
User-agent: Gnus/5.130009 (Ma Gnus v0.9) Emacs/24.3 (gnu/linux)

Cyril Roelandt <address@hidden> skribis:

> * guix/packages.scm (package-direct-inputs): New procedure.
> * guix/scripts/package.scm: Add a "show" option.
> * tests/guix-package.sh: Add a test for the "show" option.

Given that there is popular demand ;-), here’s a better review.

> +(define (package-direct-inputs package)
> +  (sort (append (package-inputs package)
> +                (package-native-inputs package)
> +                (package-propagated-inputs package))
> +        (lambda (p1 p2)
> +          (string<? (car p1) (car p2)))))

Please add a docstring and at least one test in tests/packages.scm.

However, please remove ‘sort’ from here.  The labels we use in package
inputs don’t have any special meaning, so there’s no reason to sort
things here, and they’ll probably vanish once we’ve fully integrated
gexps.

> +  (display (_ "
> +  --show=PACKAGE         show details about PACKAGE"))

Please also add it to guix.texi, mentioning that the output is in
recutils format.

> +        (('show-package requested-name)
> +         (let* ((available (fold-packages
> +                            (lambda (p r)
> +                              (let ((name (package-name p))
> +                                    (full-name (package-full-name p)))
> +                                (if (or (string=? requested-name name)
> +                                        (string=? requested-name full-name))
> +                                    (cons p r)
> +                                    r)))
> +                            '())))

Use ‘specification->package+output’ (and ignore the second return value)
instead of ‘fold-packages’.

> +           (leave-on-EPIPE
> +            (for-each (lambda (p)
> +                        (format #t "Package: ~a\n\
> +Version: ~a\n\
> +Description: ~a\n\
> +Depends: ~a\n\
> +Homepage: ~a\n\
> +License: ~a\n~%"
> +                                (package-name p)
> +                                (package-version p)
> +                                (package-description p)
> +                                (string-join (map car (package-direct-inputs 
> p)) ", ")
> +                                (package-home-page p)
> +                                (license-name (package-license p))))

Use ‘package->recutils’ instead of this.

In a separate patch, you can augment it to add the ‘depends’ field.

> +                      (sort available
> +                            (lambda (p1 p2)
> +                              (version>? (package-version p2) 
> (package-version p1))))))

Sort by ‘package-full-name’ instead of ‘package-version’.

Thoughts?

Thanks!

Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]