Re: [PATCH -v2 2/2] guix: profiles: create fonts.dir/scale for all fonts directories

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

Huang Ying <huang.ying.caritas@gmail.com> skribis:

> * guix/profiles.scm (fonts-dir-file): Create fonts.dir/scale files for all
>   fonts directories.

Looks cool, modulo Danny’s suggestions and minor issues below.

BTW, could you explain (probably as a comment in the code) why we need
to do all this?  What is it fixing?  :-)

> +                (union-build fonts-dir fonts-dirs
> +                             #:log-port (%make-void-port "w")
> +                             #:create-all-directories? #t)
> +                (ftw fonts-dir
> +                     (lambda (dir statinfo flag)

‘ftw’ is not great IMO and not used in Guix.

I would suggest using ‘find-files’ from (guix build utils).  You could
write something like:

  (let ((directories (find-files fonts-dir
                                 (lambda (file stat)
                                   (eq? 'directory (stat:type stat)))
                                 #:directories? #t)))
    (for-each do-something directories))

This also has the advantage of separating the generation of the
directory list from the actual action.

> +                       (and (eq? flag 'directory)
> +                            (with-directory-excursion dir
> +                              (and (file-exists? fonts-scale-file)
> +                                   (delete-file fonts-scale-file))

Here, since ‘delete-file’ has a side-effect and a meaningless return
value, we conventionally avoid ‘and’ and instead write:

  (when (file-exists? fonts-scale-file)
    (delete-file fonts-scale-file))

for clarity.

> +                              (and (file-exists? fonts-dir-file)
> +                                   (delete-file fonts-dir-file))
> +                              (system* mkfontscale)
> +                              (system* mkfontdir)
> +                              (and (empty-file? fonts-scale-file)
> +                                   (delete-file fonts-scale-file))
> +                              (and (empty-file? fonts-dir-file)
> +                                   (delete-file fonts-dir-file))))

Same as above.

Thank you!

Ludo’.