GNU bug report logs - #57963
[PATCH 0/1] Support user's fontconfig.

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Taiju HIGASHI <higashi <at> taiju.info>
Cc: 57963 <at> debbugs.gnu.org, liliana.prikler <at> gmail.com, andrew <at> trop.in
Subject: [bug#57963] [PATCH v4 2/2] home: fontutils: Support user's fontconfig.
Date: Sat, 01 Oct 2022 23:57:18 +0200

Taiju HIGASHI <higashi <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.