Re: [PATCH] gnu: Add font-adobe-source-han-sans.

[Andreas Enge <andreas@enge.fr>]

Great, thanks for your work!

On Fri, Feb 27, 2015 at 02:02:56PM +0800, 宋文武 wrote:
> +    (outputs '("out" "cn" "jp" "kr" "tw"))

I wondered if that was needed, but after downloading the source, I agree it
is the right thing to do!

> +    (build-system gnu-build-system)

I would go for trivial-build-system here. Or the unpack-build-system
that has not yet been written :-)

> +        (lambda* (#:key outputs #:allow-other-keys)
> +          (define (install-opentype-font font out)
> +            (let* ((font-dir (string-append out "/share/fonts/opentype"))
> +                   (newfont (string-append font-dir "/" (basename font))))
> +              (mkdir-p font-dir)
> +              (copy-file font newfont)))

I think it would be more elegant to use
  (let ((install-opentype-font (lambda (font out) ...
here; it defines a variable install-opentype-font for the expression inside
the body of "let", and this variable contains a function.

> +          ;; Install OpenType/CFF collection (OTC).
> +          (for-each (cut install-opentype-font <> (assoc-ref outputs "out"))
> +                    (find-files "OTC" "\\.ttc"))

If I see correctly, this copies each file separately, but also calls
"(mkdir-p font-dir)" each time, right? The output directories should be
created only once. You can use "copy-recursively" from (guix build utils),
see a few examples in the distribution.

Maybe add a comment what is in "out" (truetype fonts?) and in the other
outputs (open type fonts?).

Andreas