Re: guix-package --search

[ludo@gnu.org (Ludovic Courtès)]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> Instead, you can directly build the list of matching packages, like:
>
>>   (fold-packages (lambda (package result)
>>                    (if (or (regexp-exec rx (package-synopsis package))
>>                            (regexp-exec rx (package-description package)))
>>                        (cons package result)
>>                        result))
>>                  '())
>
>> This way, only one traversal is done.
>
> 'regexp-exec' will raise an error if either 'synopsis' or 'description'
> is not a string.  That's why I wrapped it in 'false-if-exception'.

OK.

>> For i18n, we should actually use (gettext (package-description
>> package)), likewise for synopsis.  This way, that will search through
>> text in the user’s native language.
>
> I added it, but I haven't properly tested it.  Also, there is
> 'remove-duplicates' that works in the REPL.

Yes, see below.

> +(define (find-packages-by-description rx)
> +  "Search in SYNOPSIS and DESCRIPTION using RX.  Return a list of
> +matching packages."
> +  (define (remove-duplicates pred lst)
> +    ;; Remove duplicates from sorted LST using PRED.
> +    (cond ((null-list? lst)   lst)
> +          ((= (length lst) 1) lst)
> +          ((= (length lst) 2)
> +           (if (pred (first lst) (second lst)) (cdr lst) lst))
> +          ((pred (first lst) (second lst))
> +           (remove-duplicates pred (cdr lst)))
> +          (else (cons (first lst) (remove-duplicates pred (cdr lst))))))

First, can you use ‘delete-duplicates’ from (srfi srfi-1) instead?

> +  (define (same-location? p1 p2)
> +    ;; Compare locations of two packages.
> +    (eq? (package-location p1) (package-location p2)))

Here you need to use ‘equal?’, not ‘eq?’: ‘equal?’ checks for structural
equality, whereas ‘eq?’ checks for pointer equality (info "(guile) Equality").

> +  (remove-duplicates
> +   same-location?
> +   (stable-sort
> +    (fold-packages
> +     (lambda (package result)
> +       (if (or (false-if-exception
> +                (regexp-exec rx (gettext (package-synopsis package))))
> +               (false-if-exception
> +                (regexp-exec rx (gettext (package-description package)))))
> +           (cons package result)
> +           result))

Instead of ‘false-if-exception’, I’d prefer:

  (lambda (package result)
    (define matches?
      (cut regexp-exec rx <>))

    (if (or (and=> (package-synopsis package)
                   (compose matches? gettext))
            ...

Other than that it looks good to me!

Could you please also add a couple of tests in tests/guix-package.sh,
and the corresponding doc in guix.texi?

Thanks!

Ludo’.