Hi,
Nikita Karetnikov <nik...@karetnikov.org> skribis:
> 1. 'psd-list' is a potential bottleneck. It will cause problems when we
> have more packages. If it's possible to evaluate it lazily, I'll
> rewrite it. (I haven't checked yet.)
See below.
[...]
> 4. I've noticed that 'fold-packages' returns duplicates. Should it be
> fixed? Should I filter the output instead?
These are not really duplicates, actually: these are variants of the
same packages, created using ‘inherit’, and keeping the original
package’s source location info (which could be fixed).
That said, it would make sense to ‘delete-duplicates’ any two packages
whose ‘package-location’ are ‘eq?’. But that can be left as a separate
commit.
The patch looks good overall. A few comments:
> +(define (sd-search rx)
Rather ‘find-packages-by-description’ or similar.
> + "Search in SYNOPSIS and DESCRIPTION using RX. Return a list of
> +matching packages."
> + (define psd-list
> + ;; Return a list of lists (each inner list contains PACKAGE-NAME,
> + ;; SYNOPSIS, and DESCRIPTION of every package).
> + (map (lambda (x)
> + (list x (package-synopsis x) (package-description x)))
> + (fold-packages cons '())))
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.
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.
> + (('search regexp)
> + (let ((regexp (and regexp (make-regexp regexp))))
> + (for-each (lambda (p)
> + (format #t "~a\t~a\t~a~%"
> + (package-name p)
> + (package-version p)
> + (location->string (package-location p))))
> + (sd-search regexp))
> + #t))
Perfect.
Thanks!
Ludo’.
