Nikita Karetnikov <nik...@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’.
