[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#57963] [PATCH v4 2/2] home: fontutils: Support user's fontconfig.
From: |
Ludovic Courtès |
Subject: |
[bug#57963] [PATCH v4 2/2] home: fontutils: Support user's fontconfig. |
Date: |
Sat, 01 Oct 2022 23:57:18 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) |
Taiju HIGASHI <higashi@taiju.info> skribis:
> * gnu/home/services/fontutils.scm: Support user's fontconfig.
I’m nitpicking a bit, but here we would describe all the
variables/procedures added, removed, or modified. Please check ‘git
log’ for examples.
Regarding code, there’s a convention to add a docstring to each
top-level procedure:
https://guix.gnu.org/manual/devel/en/html_node/Formatting-Code.html
It would be nice to follow it here.
> +(define (default-font-sanitizer type)
> + (lambda (value)
> + (if (null? value)
> + value
> + `(alias
> + (family ,type)
> + (prefer
> + (family ,value))))))
Giving '() special meaning here looks quite unusual. As Liliana wrote,
we’d usually use #f as the value denoting “nothing”.
> +(define (sxml->xmlstring sxml)
> + (if (null? sxml)
> + ""
> + (call-with-output-string
> + (lambda (port)
> + (sxml->xml sxml port)))))
Same here. Also, “xml-string” rather than “xmlstring”.
> +(define font-directories? list?)
Is it really needed?
> +(define (serialize-font-directories field-name value)
> + (sxml->xmlstring
> + (append
> + '((dir "~/.guix-home/profile/share/fonts"))
> + (map
> + (lambda (path)
> + `(dir ,path))
> + value))))
The indentation would rather be:
(append '((dir …))
(map (lambda (directory)
`(dir ,directory))
value))
> + (map (match-lambda
> + ((? pair? sxml) sxml)
> + ((? string? xml) (xml->sxml xml))
> + (_ (error "extra-config value must be xml string or sxml list.")))
Instead of ‘error’, which would lead to an ugly backtrace and an
untranslated error message, write:
(raise (formatted-message (G_ "'extra-config' …")))
without a trailing dot in the message.
The rest LGTM! Like I wrote, could you please add documentation in
‘doc/guix.texi’, with a configuration example like the one you gave?
Thanks for all the work!
Ludo’.