diff mbox series

[bug#57963,v5,2/2] home: services: Support user's fontconfig configuration.

Message ID 20221002131535.9972-2-higashi@taiju.info
State New
Headers show
Series [bug#57963,v5,1/2] home: services: Add base. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Taiju HIGASHI Oct. 2, 2022, 1:15 p.m. UTC 
* gnu/home/services/fontutils.scm (add-fontconfig-config-file): Support user's
fontconfig configuration.
(home-fontconfig-configuration): New configuration for it.
(string-list, maybe-string, maybe-extra-config-list): New types for it.
(string-list?, extra-config-list?): New predicate procedures for it.
(serialize-string-list, serialize-string, serialize-extra-config-list): New
serialize procedures for it.
(guix-home-font-dir): New variable.
---
 gnu/home/services/fontutils.scm | 89 ++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 6 deletions(-)

Comments

Andrew Tropin Oct. 10, 2022, 6:40 a.m. UTC | #1 
On 2022-10-02 22:15, Taiju HIGASHI wrote:

> * gnu/home/services/fontutils.scm (add-fontconfig-config-file): Support user's
> fontconfig configuration.
> (home-fontconfig-configuration): New configuration for it.
> (string-list, maybe-string, maybe-extra-config-list): New types for it.
> (string-list?, extra-config-list?): New predicate procedures for it.
> (serialize-string-list, serialize-string, serialize-extra-config-list): New
> serialize procedures for it.
> (guix-home-font-dir): New variable.
> ---
>  gnu/home/services/fontutils.scm | 89 ++++++++++++++++++++++++++++++---
>  1 file changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/gnu/home/services/fontutils.scm b/gnu/home/services/fontutils.scm
> index 6062eaed6a..4b3caf3985 100644
> --- a/gnu/home/services/fontutils.scm
> +++ b/gnu/home/services/fontutils.scm
> @@ -1,6 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
>  ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
> +;;; Copyright © 2022 Taiju HIGASHI <higashi@taiju.info>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -20,9 +21,17 @@
>  (define-module (gnu home services fontutils)
>    #:use-module (gnu home services)
>    #:use-module (gnu packages fontutils)
> +  #:use-module (gnu services configuration)
> +  #:use-module (guix diagnostics)
>    #:use-module (guix gexp)
> +  #:use-module (guix i18n)
> +  #:use-module (guix records)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (sxml simple)
> +  #:use-module (ice-9 match)
>  
> -  #:export (home-fontconfig-service-type))
> +  #:export (home-fontconfig-service-type
> +            home-fontconfig-configuration))
>  
>  ;;; Commentary:
>  ;;;
> @@ -33,15 +42,83 @@ (define-module (gnu home services fontutils)
>  ;;;
>  ;;; Code:
>  
> -(define (add-fontconfig-config-file he-symlink-path)
> +(define (sxml->xml-string sxml)
> +  "Serialize the sxml tree @var{tree} as XML. The output will be string."
> +  (call-with-output-string
> +    (lambda (port)
> +      (sxml->xml sxml port))))
> +
> +(define guix-home-font-dir "~/.guix-home/profile/share/fonts")
> +
> +(define (string-list? value)
> +  (and (pair? value) (every string? value)))

Better to use list? here and in the other places, at least for the
consistency, but also for semantic meaning.

> +
> +(define (serialize-string-list field-name value)
> +  (sxml->xml-string
> +   (map
> +    (lambda (path) `(dir ,path))
> +    (if (member guix-home-font-dir value)
> +        value
> +        (append (list guix-home-font-dir) value)))))
> +
> +(define (serialize-string field-name value)
> +  (define (serialize type value)
> +    (sxml->xml-string
> +     `(alias
> +       (family ,type)
> +       (prefer
> +        (family ,value)))))
> +  (match (list field-name value)
> +    (('default-font-serif-family family)
> +     (serialize 'serif family))
> +    (('default-font-sans-serif-family family)
> +     (serialize 'sans-serif family))
> +    (('default-font-monospace-family family)
> +     (serialize 'monospace family))))
> +
> +(define-maybe string)
> +
> +(define extra-config-list? list?)
> +
> +(define-maybe extra-config-list)
> +
> +(define (serialize-extra-config-list field-name value)
> +  (sxml->xml-string
> +   (map (match-lambda
> +          ((? pair? sxml) sxml)

Other branches would never be visited because it will fail earlier by
define-configuration predicate check for extra-config-list? (which is
basically list?).

Also, making multi-type fields is debatable, but isn't great IMO.

If serialization would support G-exps, we could write 

(list #~"RAW_XML_HERE")

or even something like this:

(list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))

> +          ((? string? xml) (xml->sxml xml))
> +          (else
> +           (raise (formatted-message
> +                   (G_ "'extra-config' type must be xml string or sxml list, was given: ~a")
> +                   value))))
> +        value)))
> +
> +(define-configuration home-fontconfig-configuration
> +  (font-directories
> +   (string-list (list guix-home-font-dir))

It's not a generic string-list, but a specific font-directories-list
with extra logic inside.

Also, because guix-home-font-dir always added to the list, the default
value should '() and field should be called additional-font-directories
instead.  Otherwise it will be confusing, why guix-home-font-dir is not
removed from the final configuration, when this field is set to a
different value.

I skimmed previous messages, but sorry, if I missed any already
mentioned points.

> +   "The directory list that provides fonts.")
> +  (default-font-serif-family
> +    maybe-string
> +    "The preffered default fonts of serif.")
> +  (default-font-sans-serif-family
> +    maybe-string
> +    "The preffered default fonts of sans-serif.")
> +  (default-font-monospace-family
> +    maybe-string
> +    "The preffered default fonts of monospace.")
> +  (extra-config
> +   maybe-extra-config-list
> +   "Extra configuration values to append to the fonts.conf."))
> +
> +(define (add-fontconfig-config-file user-config)
>    `(("fontconfig/fonts.conf"
>       ,(mixed-text-file
>         "fonts.conf"
>         "<?xml version='1.0'?>
>  <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
> -<fontconfig>
> -  <dir>~/.guix-home/profile/share/fonts</dir>
> -</fontconfig>"))))
> +<fontconfig>"
> +       (serialize-configuration user-config home-fontconfig-configuration-fields)

Just a thought for the future and a point for configuration module
improvements: It would be cool if serialize-configuration and all other
serialize- functions returned a G-exps, this way we could write
something like that:

(home-fontconfig-configuration
 (font-directories (list (file-append font-iosevka "/share/fonts"))))

> +       "</fontconfig>\n"))))
>  
>  (define (regenerate-font-cache-gexp _)
>    `(("profile/share/fonts"
> @@ -59,7 +136,7 @@ (define home-fontconfig-service-type
>                         (service-extension
>                          home-profile-service-type
>                          (const (list fontconfig)))))
> -                (default-value #f)
> +                (default-value (home-fontconfig-configuration))
>                  (description
>                   "Provides configuration file for fontconfig and make
>  fc-* utilities aware of font packages installed in Guix Home's profile.")))
Liliana Marie Prikler Oct. 10, 2022, 4:15 p.m. UTC | #2 
Am Montag, dem 10.10.2022 um 10:40 +0400 schrieb Andrew Tropin:
> Also, because guix-home-font-dir always added to the list, the
> default value should '() and field should be called
> additional-font-directories instead.  Otherwise it will be confusing,
> why guix-home-font-dir is not removed from the final configuration,
> when this field is set to a different value.
Actually, I think the default value should (if possible) explicitly
contain the one being added by Guix Home.  I also think it shouldn't be
added when the user explicitly removed it.

Cheers
Taiju HIGASHI Oct. 11, 2022, 3:54 a.m. UTC | #3 
Hi Andrew,

Thank you for your review!

>> +(define (string-list? value)
>> +  (and (pair? value) (every string? value)))
>
> Better to use list? here and in the other places, at least for the
> consistency, but also for semantic meaning.

OK. I'll rewrite it to below.

--8<---------------cut here---------------start------------->8---
(define (string-list? value)
  (and (list? value) (every string? value)))
--8<---------------cut here---------------end--------------->8---

>> +
>> +(define (serialize-string-list field-name value)
>> +  (sxml->xml-string
>> +   (map
>> +    (lambda (path) `(dir ,path))
>> +    (if (member guix-home-font-dir value)
>> +        value
>> +        (append (list guix-home-font-dir) value)))))
>> +
>> +(define (serialize-string field-name value)
>> +  (define (serialize type value)
>> +    (sxml->xml-string
>> +     `(alias
>> +       (family ,type)
>> +       (prefer
>> +        (family ,value)))))
>> +  (match (list field-name value)
>> +    (('default-font-serif-family family)
>> +     (serialize 'serif family))
>> +    (('default-font-sans-serif-family family)
>> +     (serialize 'sans-serif family))
>> +    (('default-font-monospace-family family)
>> +     (serialize 'monospace family))))
>> +
>> +(define-maybe string)
>> +
>> +(define extra-config-list? list?)
>> +
>> +(define-maybe extra-config-list)
>> +
>> +(define (serialize-extra-config-list field-name value)
>> +  (sxml->xml-string
>> +   (map (match-lambda
>> +          ((? pair? sxml) sxml)
>
> Other branches would never be visited because it will fail earlier by
> define-configuration predicate check for extra-config-list? (which is
> basically list?).

We can specify invalid value such as (list "foo" '(foo bar) 123).

> Also, making multi-type fields is debatable, but isn't great IMO.

I see. If we had to choose one or the other, I would prefer the
string-type field.

> If serialization would support G-exps, we could write
>
> (list #~"RAW_XML_HERE")
>
> or even something like this:
>
> (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))

Does it mean that the specification does not allow it now? Or does it
mean that it is not possible with my implementation?

>> +          ((? string? xml) (xml->sxml xml))
>> +          (else
>> +           (raise (formatted-message
>> + (G_ "'extra-config' type must be xml string or sxml list, was
>> given: ~a")
>> +                   value))))
>> +        value)))
>> +
>> +(define-configuration home-fontconfig-configuration
>> +  (font-directories
>> +   (string-list (list guix-home-font-dir))
>
> It's not a generic string-list, but a specific font-directories-list
> with extra logic inside.
>
> Also, because guix-home-font-dir always added to the list, the default
> value should '() and field should be called additional-font-directories
> instead.  Otherwise it will be confusing, why guix-home-font-dir is not
> removed from the final configuration, when this field is set to a
> different value.
>
> I skimmed previous messages, but sorry, if I missed any already
> mentioned points.

Sure, It is more appropriate that the field type is to
font-directories-list field name is to additional-font-directories.

>> +   "The directory list that provides fonts.")
>> +  (default-font-serif-family
>> +    maybe-string
>> +    "The preffered default fonts of serif.")
>> +  (default-font-sans-serif-family
>> +    maybe-string
>> +    "The preffered default fonts of sans-serif.")
>> +  (default-font-monospace-family
>> +    maybe-string
>> +    "The preffered default fonts of monospace.")
>> +  (extra-config
>> +   maybe-extra-config-list
>> +   "Extra configuration values to append to the fonts.conf."))
>> +
>> +(define (add-fontconfig-config-file user-config)
>>    `(("fontconfig/fonts.conf"
>>       ,(mixed-text-file
>>         "fonts.conf"
>>         "<?xml version='1.0'?>
>>  <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
>> -<fontconfig>
>> -  <dir>~/.guix-home/profile/share/fonts</dir>
>> -</fontconfig>"))))
>> +<fontconfig>"
>> +       (serialize-configuration user-config home-fontconfig-configuration-fields)
>
> Just a thought for the future and a point for configuration module
> improvements: It would be cool if serialize-configuration and all other
> serialize- functions returned a G-exps, this way we could write
> something like that:
>
> (home-fontconfig-configuration
>  (font-directories (list (file-append font-iosevka "/share/fonts"))))

Nice.

Thanks,
--
Taiju
Liliana Marie Prikler Oct. 11, 2022, 4:21 a.m. UTC | #4 
Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
> We can specify invalid value such as (list "foo" '(foo bar) 123).
It will be sanitized before that.

> > Also, making multi-type fields is debatable, but isn't great IMO.
> 
> I see. If we had to choose one or the other, I would prefer the
> string-type field.
Prefer sexp-type.

> > If serialization would support G-exps, we could write
> > 
> > (list #~"RAW_XML_HERE")
> > 
> > or even something like this:
> > 
> > (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
> 
> Does it mean that the specification does not allow it now? Or does it
> mean that it is not possible with my implementation?
I think your serialize would have to unpack the G-Expressions.  You can
test that with some example configs of your own.

> > 
Cheers
Taiju HIGASHI Oct. 11, 2022, 8:09 a.m. UTC | #5 
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
>> We can specify invalid value such as (list "foo" '(foo bar) 123).
> It will be sanitized before that.

I'm sorry, I may not be getting it.

When I reconfigure with the following settings:

--8<---------------cut here---------------start------------->8---
(home-environment
 (packages (list font-google-noto))
 (services
  (append
      (list
       (service home-bash-service-type))
      (modify-services %home-base-services
        (home-fontconfig-service-type
         config => (home-fontconfig-configuration
                    (extra-config
                     (list "<dir>foo</dir>" 123))))))))
--8<---------------cut here---------------end--------------->8---

The following error occurs.

--8<---------------cut here---------------start------------->8---
./pre-inst-env guix home container home-fontconfig-config.scm
Backtrace:
In guix/monads.scm:
    487:9 19 (_ _)
In gnu/services.scm:
  1137:16 18 (_ _)
In guix/monads.scm:
    487:9 17 (_ _)
In gnu/services.scm:
  1140:36 16 (_ _)
In srfi/srfi-1.scm:
   586:17 15 (map1 (#<<service> type: #<service-type home-fontconfig 7f1926abf…>))
In ice-9/eval.scm:
    155:9 14 (_ #(#(#<directory (gnu home services fontutils) 7f1926df8780>) #))
    159:9 13 (_ #(#(#<directory (gnu home services fontutils) 7f1926df8780>) #))
   173:55 12 (_ #(#(#<directory (gnu home services fontutils) 7f1926df8780>) #))
In gnu/services/configuration.scm:
    124:8 11 (serialize-configuration _ _)
In srfi/srfi-1.scm:
   586:29 10 (map1 (#<<configuration-field> name: font-directories type: str…> …))
   586:29  9 (map1 (#<<configuration-field> name: default-font-serif-family …> …))
   586:29  8 (map1 (#<<configuration-field> name: default-font-sans-serif-fa…> …))
   586:29  7 (map1 (#<<configuration-field> name: default-font-monospace-fam…> …))
   586:17  6 (map1 (#<<configuration-field> name: extra-config type: maybe-ext…>))
In ice-9/eval.scm:
    155:9  5 (_ #(#(#<directory (gnu home services fontutils) 7f1926df8780>) # …))
In srfi/srfi-1.scm:
   586:29  4 (map1 ("<dir>foo</dir>" 123))
   586:17  3 (map1 (123))
In unknown file:
           2 (raise #<&formatted-message format: "'extra-config' type must be x…>)
In ice-9/boot-9.scm:
  1685:16  1 (raise-exception _ #:continuable? _)
  1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Wrong type (expecting exact integer): #<&formatted-message format: "'extra-config' type must be xml string or sxml list, was given: ~a\n" arguments: (("<dir>foo</dir>" 123))>
--8<---------------cut here---------------end--------------->8---

Is it sanitized before?

>> > Also, making multi-type fields is debatable, but isn't great IMO.
>>
>> I see. If we had to choose one or the other, I would prefer the
>> string-type field.
> Prefer sexp-type.

I too would like to write my settings in S-expression, but for users who
know the XML format of fontconfig but do not know how to use SXML, I
believe the effort of converting XML to SXML in their head and writing
it cannot be ignored.
Still, users can write settings in SXML and convert them to
strings. That is a choice the user prefers to make; someone who doesn't
know SXML writing strings and converting them to SXML is not a choice
the user prefers to make.

>> > If serialization would support G-exps, we could write
>> >
>> > (list #~"RAW_XML_HERE")
>> >
>> > or even something like this:
>> >
>> > (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>>
>> Does it mean that the specification does not allow it now? Or does it
>> mean that it is not possible with my implementation?
> I think your serialize would have to unpack the G-Expressions.  You can
> test that with some example configs of your own.

Thank you. I'll give it a try.

Thanks,
Liliana Marie Prikler Oct. 11, 2022, 6:24 p.m. UTC | #6 
Am Dienstag, dem 11.10.2022 um 17:09 +0900 schrieb Taiju HIGASHI:
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> 
> > Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
> > > We can specify invalid value such as (list "foo" '(foo bar) 123).
> > It will be sanitized before that.
> 
> I'm sorry, I may not be getting it.
> 
> When I reconfigure with the following settings:
> 
> --8<---------------cut here---------------start------------->8---
> (home-environment
>  (packages (list font-google-noto))
>  (services
>   (append
>       (list
>        (service home-bash-service-type))
>       (modify-services %home-base-services
>         (home-fontconfig-service-type
>          config => (home-fontconfig-configuration
>                     (extra-config
>                      (list "<dir>foo</dir>" 123))))))))
> --8<---------------cut here---------------end--------------->8---
> 
> The following error occurs.
> 
> --8<---------------cut here---------------start------------->8---
> ./pre-inst-env guix home container home-fontconfig-config.scm
> Backtrace:
> In guix/monads.scm:
>     487:9 19 (_ _)
> In gnu/services.scm:
>   1137:16 18 (_ _)
> In guix/monads.scm:
>     487:9 17 (_ _)
> In gnu/services.scm:
>   1140:36 16 (_ _)
> In srfi/srfi-1.scm:
>    586:17 15 (map1 (#<<service> type: #<service-type home-fontconfig
> 7f1926abf…>))
> In ice-9/eval.scm:
>     155:9 14 (_ #(#(#<directory (gnu home services fontutils)
> 7f1926df8780>) #))
>     159:9 13 (_ #(#(#<directory (gnu home services fontutils)
> 7f1926df8780>) #))
>    173:55 12 (_ #(#(#<directory (gnu home services fontutils)
> 7f1926df8780>) #))
> In gnu/services/configuration.scm:
>     124:8 11 (serialize-configuration _ _)
> In srfi/srfi-1.scm:
>    586:29 10 (map1 (#<<configuration-field> name: font-directories
> type: str…> …))
>    586:29  9 (map1 (#<<configuration-field> name: default-font-serif-
> family …> …))
>    586:29  8 (map1 (#<<configuration-field> name: default-font-sans-
> serif-fa…> …))
>    586:29  7 (map1 (#<<configuration-field> name: default-font-
> monospace-fam…> …))
>    586:17  6 (map1 (#<<configuration-field> name: extra-config type:
> maybe-ext…>))
> In ice-9/eval.scm:
>     155:9  5 (_ #(#(#<directory (gnu home services fontutils)
> 7f1926df8780>) # …))
> In srfi/srfi-1.scm:
>    586:29  4 (map1 ("<dir>foo</dir>" 123))
>    586:17  3 (map1 (123))
> In unknown file:
>            2 (raise #<&formatted-message format: "'extra-config' type
> must be x…>)
> In ice-9/boot-9.scm:
>   1685:16  1 (raise-exception _ #:continuable? _)
>   1685:16  0 (raise-exception _ #:continuable? _)
> 
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> Wrong type (expecting exact integer): #<&formatted-message format:
> "'extra-config' type must be xml string or sxml list, was given:
> ~a\n" arguments: (("<dir>foo</dir>" 123))>
> --8<---------------cut here---------------end--------------->8---
> 
> Is it sanitized before?
That error seems to be coming from your sanitizer if I read this
correctly.

> > > > Also, making multi-type fields is debatable, but isn't great
> > > > IMO.
> > > 
> > > I see. If we had to choose one or the other, I would prefer the
> > > string-type field.
> > Prefer sexp-type.
> 
> I too would like to write my settings in S-expression, but for users
> who know the XML format of fontconfig but do not know how to use
> SXML, I believe the effort of converting XML to SXML in their head
> and writing it cannot be ignored.
> Still, users can write settings in SXML and convert them to
> strings. That is a choice the user prefers to make; someone who
> doesn't know SXML writing strings and converting them to SXML is not
> a choice the user prefers to make.
You can likewise convert xml->sxml explicitly, there's not really any
difference here.  Providing this in a sanitizer just makes it more
user-friendly.

Cheers
Taiju HIGASHI Oct. 12, 2022, 3:59 a.m. UTC | #7 
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Dienstag, dem 11.10.2022 um 17:09 +0900 schrieb Taiju HIGASHI:
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> > Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
>> > > We can specify invalid value such as (list "foo" '(foo bar) 123).
>> > It will be sanitized before that.
>>
>> I'm sorry, I may not be getting it.
>>
>> When I reconfigure with the following settings:
>>
>> --8<---------------cut here---------------start------------->8---
>> (home-environment
>>  (packages (list font-google-noto))
>>  (services
>>   (append
>>       (list
>>        (service home-bash-service-type))
>>       (modify-services %home-base-services
>>         (home-fontconfig-service-type
>>          config => (home-fontconfig-configuration
>>                     (extra-config
>>                      (list "<dir>foo</dir>" 123))))))))
>> --8<---------------cut here---------------end--------------->8---
>>
>> The following error occurs.
>>
>> --8<---------------cut here---------------start------------->8---
>> ./pre-inst-env guix home container home-fontconfig-config.scm
>> Backtrace:
>> In guix/monads.scm:
>>     487:9 19 (_ _)
>> In gnu/services.scm:
>>   1137:16 18 (_ _)
>> In guix/monads.scm:
>>     487:9 17 (_ _)
>> In gnu/services.scm:
>>   1140:36 16 (_ _)
>> In srfi/srfi-1.scm:
>>    586:17 15 (map1 (#<<service> type: #<service-type home-fontconfig
>> 7f1926abf…>))
>> In ice-9/eval.scm:
>>     155:9 14 (_ #(#(#<directory (gnu home services fontutils)
>> 7f1926df8780>) #))
>>     159:9 13 (_ #(#(#<directory (gnu home services fontutils)
>> 7f1926df8780>) #))
>>    173:55 12 (_ #(#(#<directory (gnu home services fontutils)
>> 7f1926df8780>) #))
>> In gnu/services/configuration.scm:
>>     124:8 11 (serialize-configuration _ _)
>> In srfi/srfi-1.scm:
>>    586:29 10 (map1 (#<<configuration-field> name: font-directories
>> type: str…> …))
>>    586:29  9 (map1 (#<<configuration-field> name: default-font-serif-
>> family …> …))
>>    586:29  8 (map1 (#<<configuration-field> name: default-font-sans-
>> serif-fa…> …))
>>    586:29  7 (map1 (#<<configuration-field> name: default-font-
>> monospace-fam…> …))
>>    586:17  6 (map1 (#<<configuration-field> name: extra-config type:
>> maybe-ext…>))
>> In ice-9/eval.scm:
>>     155:9  5 (_ #(#(#<directory (gnu home services fontutils)
>> 7f1926df8780>) # …))
>> In srfi/srfi-1.scm:
>>    586:29  4 (map1 ("<dir>foo</dir>" 123))
>>    586:17  3 (map1 (123))
>> In unknown file:
>>            2 (raise #<&formatted-message format: "'extra-config' type
>> must be x…>)
>> In ice-9/boot-9.scm:
>>   1685:16  1 (raise-exception _ #:continuable? _)
>>   1685:16  0 (raise-exception _ #:continuable? _)
>>
>> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
>> Wrong type (expecting exact integer): #<&formatted-message format:
>> "'extra-config' type must be xml string or sxml list, was given:
>> ~a\n" arguments: (("<dir>foo</dir>" 123))>
>> --8<---------------cut here---------------end--------------->8---
>>
>> Is it sanitized before?
> That error seems to be coming from your sanitizer if I read this
> correctly.

Yes, I think so.  So I do not know what he meant when he said "Other
branches would never be visited."

    Other branches would never be visited because it will fail earlier
    by define-configuration predicate check for extra-config-list?
    (which is basically list?).

I may have misunderstood the location of the code to which his comment
refers.

>> > > > Also, making multi-type fields is debatable, but isn't great
>> > > > IMO.
>> > >
>> > > I see. If we had to choose one or the other, I would prefer the
>> > > string-type field.
>> > Prefer sexp-type.
>>
>> I too would like to write my settings in S-expression, but for users
>> who know the XML format of fontconfig but do not know how to use
>> SXML, I believe the effort of converting XML to SXML in their head
>> and writing it cannot be ignored.
>> Still, users can write settings in SXML and convert them to
>> strings. That is a choice the user prefers to make; someone who
>> doesn't know SXML writing strings and converting them to SXML is not
>> a choice the user prefers to make.
> You can likewise convert xml->sxml explicitly, there's not really any
> difference here.  Providing this in a sanitizer just makes it more
> user-friendly.

I believe the v5 patch currently does that. Do you think a multi-type
field is acceptable? Or do you think it is better to keep only the
SXML-type field?

Cheers,
Liliana Marie Prikler Oct. 12, 2022, 4:21 a.m. UTC | #8 
Am Mittwoch, dem 12.10.2022 um 12:59 +0900 schrieb Taiju HIGASHI:
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> 
> > Am Dienstag, dem 11.10.2022 um 17:09 +0900 schrieb Taiju HIGASHI:
> > > Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > > 
> > > > Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju
> > > > HIGASHI:
> > > > > We can specify invalid value such as (list "foo" '(foo bar)
> > > > > 123).
> > > > It will be sanitized before that.
> > > 
> > > I'm sorry, I may not be getting it.
> > > 
> > > When I reconfigure with the following settings:
> > > 
> > > --8<---------------cut here---------------start------------->8---
> > > (home-environment
> > >  (packages (list font-google-noto))
> > >  (services
> > >   (append
> > >       (list
> > >        (service home-bash-service-type))
> > >       (modify-services %home-base-services
> > >         (home-fontconfig-service-type
> > >          config => (home-fontconfig-configuration
> > >                     (extra-config
> > >                      (list "<dir>foo</dir>" 123))))))))
> > > --8<---------------cut here---------------end--------------->8---
> > > 
> > > The following error occurs.
> > > 
> > > --8<---------------cut here---------------start------------->8---
> > > ./pre-inst-env guix home container home-fontconfig-config.scm
> > > Backtrace:
> > > In guix/monads.scm:
> > >     487:9 19 (_ _)
> > > In gnu/services.scm:
> > >   1137:16 18 (_ _)
> > > In guix/monads.scm:
> > >     487:9 17 (_ _)
> > > In gnu/services.scm:
> > >   1140:36 16 (_ _)
> > > In srfi/srfi-1.scm:
> > >    586:17 15 (map1 (#<<service> type: #<service-type home-
> > > fontconfig
> > > 7f1926abf…>))
> > > In ice-9/eval.scm:
> > >     155:9 14 (_ #(#(#<directory (gnu home services fontutils)
> > > 7f1926df8780>) #))
> > >     159:9 13 (_ #(#(#<directory (gnu home services fontutils)
> > > 7f1926df8780>) #))
> > >    173:55 12 (_ #(#(#<directory (gnu home services fontutils)
> > > 7f1926df8780>) #))
> > > In gnu/services/configuration.scm:
> > >     124:8 11 (serialize-configuration _ _)
> > > In srfi/srfi-1.scm:
> > >    586:29 10 (map1 (#<<configuration-field> name: font-
> > > directories
> > > type: str…> …))
> > >    586:29  9 (map1 (#<<configuration-field> name: default-font-
> > > serif-
> > > family …> …))
> > >    586:29  8 (map1 (#<<configuration-field> name: default-font-
> > > sans-
> > > serif-fa…> …))
> > >    586:29  7 (map1 (#<<configuration-field> name: default-font-
> > > monospace-fam…> …))
> > >    586:17  6 (map1 (#<<configuration-field> name: extra-config
> > > type:
> > > maybe-ext…>))
> > > In ice-9/eval.scm:
> > >     155:9  5 (_ #(#(#<directory (gnu home services fontutils)
> > > 7f1926df8780>) # …))
> > > In srfi/srfi-1.scm:
> > >    586:29  4 (map1 ("<dir>foo</dir>" 123))
> > >    586:17  3 (map1 (123))
> > > In unknown file:
> > >            2 (raise #<&formatted-message format: "'extra-config'
> > > type
> > > must be x…>)
> > > In ice-9/boot-9.scm:
> > >   1685:16  1 (raise-exception _ #:continuable? _)
> > >   1685:16  0 (raise-exception _ #:continuable? _)
> > > 
> > > ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> > > Wrong type (expecting exact integer): #<&formatted-message
> > > format:
> > > "'extra-config' type must be xml string or sxml list, was given:
> > > ~a\n" arguments: (("<dir>foo</dir>" 123))>
> > > --8<---------------cut here---------------end--------------->8---
> > > 
> > > Is it sanitized before?
> > That error seems to be coming from your sanitizer if I read this
> > correctly.
> 
> Yes, I think so.  So I do not know what he meant when he said "Other
> branches would never be visited."
> 
>     Other branches would never be visited because it will fail
> earlier
>     by define-configuration predicate check for extra-config-list?
>     (which is basically list?).
> 
> I may have misunderstood the location of the code to which his
> comment refers.
I think this basically means that you can't have a raw string, but only
a list of strings, which conflicts with how you distinguish xml and
sxml?

> > > > > > Also, making multi-type fields is debatable, but isn't
> > > > > > great
> > > > > > IMO.
> > > > > 
> > > > > I see. If we had to choose one or the other, I would prefer
> > > > > the
> > > > > string-type field.
> > > > Prefer sexp-type.
> > > 
> > > I too would like to write my settings in S-expression, but for
> > > users
> > > who know the XML format of fontconfig but do not know how to use
> > > SXML, I believe the effort of converting XML to SXML in their
> > > head
> > > and writing it cannot be ignored.
> > > Still, users can write settings in SXML and convert them to
> > > strings. That is a choice the user prefers to make; someone who
> > > doesn't know SXML writing strings and converting them to SXML is
> > > not
> > > a choice the user prefers to make.
> > You can likewise convert xml->sxml explicitly, there's not really
> > any
> > difference here.  Providing this in a sanitizer just makes it more
> > user-friendly.
> 
> I believe the v5 patch currently does that. Do you think a multi-type
> field is acceptable? Or do you think it is better to keep only the
> SXML-type field?
I think the field, once sanitized, should be SXML.  I care little about
what happens before.

Cheers
Andrew Tropin Oct. 12, 2022, 6:05 a.m. UTC | #9 
On 2022-10-10 18:15, Liliana Marie Prikler wrote:

> Am Montag, dem 10.10.2022 um 10:40 +0400 schrieb Andrew Tropin:
>> Also, because guix-home-font-dir always added to the list, the
>> default value should '() and field should be called
>> additional-font-directories instead.  Otherwise it will be confusing,
>> why guix-home-font-dir is not removed from the final configuration,
>> when this field is set to a different value.
> Actually, I think the default value should (if possible) explicitly
> contain the one being added by Guix Home.  I also think it shouldn't be
> added when the user explicitly removed it.
>

Completely agree.  Probably I had to be more explicit on the implication
of my comment.
Andrew Tropin Oct. 12, 2022, 6:43 a.m. UTC | #10 
On 2022-10-11 12:54, Taiju HIGASHI wrote:

> Hi Andrew,
>
> Thank you for your review!
>
>>> +(define (string-list? value)
>>> +  (and (pair? value) (every string? value)))
>>
>> Better to use list? here and in the other places, at least for the
>> consistency, but also for semantic meaning.
>
> OK. I'll rewrite it to below.
>
> --8<---------------cut here---------------start------------->8---
> (define (string-list? value)
>   (and (list? value) (every string? value)))
> --8<---------------cut here---------------end--------------->8---
>
>>> +
>>> +(define (serialize-string-list field-name value)
>>> +  (sxml->xml-string
>>> +   (map
>>> +    (lambda (path) `(dir ,path))
>>> +    (if (member guix-home-font-dir value)
>>> +        value
>>> +        (append (list guix-home-font-dir) value)))))
>>> +
>>> +(define (serialize-string field-name value)
>>> +  (define (serialize type value)
>>> +    (sxml->xml-string
>>> +     `(alias
>>> +       (family ,type)
>>> +       (prefer
>>> +        (family ,value)))))
>>> +  (match (list field-name value)
>>> +    (('default-font-serif-family family)
>>> +     (serialize 'serif family))
>>> +    (('default-font-sans-serif-family family)
>>> +     (serialize 'sans-serif family))
>>> +    (('default-font-monospace-family family)
>>> +     (serialize 'monospace family))))
>>> +
>>> +(define-maybe string)
>>> +
>>> +(define extra-config-list? list?)
>>> +
>>> +(define-maybe extra-config-list)
>>> +
>>> +(define (serialize-extra-config-list field-name value)
>>> +  (sxml->xml-string
>>> +   (map (match-lambda
>>> +          ((? pair? sxml) sxml)
>>
>> Other branches would never be visited because it will fail earlier by
>> define-configuration predicate check for extra-config-list? (which is
>> basically list?).

Oh, I missed the map over the list elements and slightly missread the
code.  I thought (according to my incorrect perception of
implementation) extra-config have to be either sxml or string, that's is
why I said that it will fail earlier because plan string value won't
satisfy list predicate attached to extra-config field, but in a fact
extra-config is always a list, but can be a list of sxml's and strings
mixed together.

Thus, some of my comments are invalid.  Sorry for the confusion.  I'll
rephrase and elaborate in the later message.

>
> We can specify invalid value such as (list "foo" '(foo bar) 123).
>
>> Also, making multi-type fields is debatable, but isn't great IMO.
>
> I see. If we had to choose one or the other, I would prefer the
> string-type field.
>
>> If serialization would support G-exps, we could write
>>
>> (list #~"RAW_XML_HERE")
>>
>> or even something like this:
>>
>> (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>
> Does it mean that the specification does not allow it now? Or does it
> mean that it is not possible with my implementation?
>

It's not possible with the current implementation.

>>> + ((? string? xml) (xml->sxml xml)) + (else + (raise
>>> (formatted-message + (G_ "'extra-config' type must be xml string or
>>> sxml list, was given: ~a") + value)))) + value))) +
>>> +(define-configuration home-fontconfig-configuration +
>>> (font-directories + (string-list (list guix-home-font-dir))
>>
>> It's not a generic string-list, but a specific font-directories-list
>> with extra logic inside.
>>
>> Also, because guix-home-font-dir always added to the list, the default
>> value should '() and field should be called additional-font-directories
>> instead.  Otherwise it will be confusing, why guix-home-font-dir is not
>> removed from the final configuration, when this field is set to a
>> different value.
>>
>> I skimmed previous messages, but sorry, if I missed any already
>> mentioned points.
>
> Sure, It is more appropriate that the field type is to
> font-directories-list field name is to additional-font-directories.
>

As Liliana mentioned in the other message, it's better not to set
anything implicitly.  It's better to keep the name, but change the
implementation and remove implicitly and unconditionally added
directory.

>>> +   "The directory list that provides fonts.")
>>> +  (default-font-serif-family
>>> +    maybe-string
>>> +    "The preffered default fonts of serif.")
>>> +  (default-font-sans-serif-family
>>> +    maybe-string
>>> +    "The preffered default fonts of sans-serif.")
>>> +  (default-font-monospace-family
>>> +    maybe-string
>>> +    "The preffered default fonts of monospace.")
>>> +  (extra-config
>>> +   maybe-extra-config-list
>>> +   "Extra configuration values to append to the fonts.conf."))
>>> +
>>> +(define (add-fontconfig-config-file user-config)
>>>    `(("fontconfig/fonts.conf"
>>>       ,(mixed-text-file
>>>         "fonts.conf"
>>>         "<?xml version='1.0'?>
>>>  <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
>>> -<fontconfig>
>>> -  <dir>~/.guix-home/profile/share/fonts</dir>
>>> -</fontconfig>"))))
>>> +<fontconfig>"
>>> +       (serialize-configuration user-config home-fontconfig-configuration-fields)
>>
>> Just a thought for the future and a point for configuration module
>> improvements: It would be cool if serialize-configuration and all other
>> serialize- functions returned a G-exps, this way we could write
>> something like that:
>>
>> (home-fontconfig-configuration
>>  (font-directories (list (file-append font-iosevka "/share/fonts"))))
>
> Nice.
>
> Thanks,
> --
> Taiju
Andrew Tropin Oct. 12, 2022, 7:07 a.m. UTC | #11 
On 2022-10-11 06:21, Liliana Marie Prikler wrote:

> Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
>> We can specify invalid value such as (list "foo" '(foo bar) 123).
> It will be sanitized before that.
>
>> > Also, making multi-type fields is debatable, but isn't great IMO.
>> 
>> I see. If we had to choose one or the other, I would prefer the
>> string-type field.
> Prefer sexp-type.
>

Current (v5) extra-config has a list type.  This list can contain strings
and nested lists, string elements are for raw XML, and list
elements are for SXML.

This is done I guess to support following use case:

--8<---------------cut here---------------start------------->8---
(list "<tag>Already existing XML copied from existing .xml file, which
we don't want to rewrite to SXML.</tag>"
      '((tag (@ (attr1 "value1")
                (attr2 "value2"))
             (nested "Part of the configuration defined with SXML")
             (empty)))
      "<another-tag>Maybe some other part of raw XML</another-tag>")
--8<---------------cut here---------------end--------------->8---

This way we can combine SXML with already existing raw XML.

Am I right?

>> > If serialization would support G-exps, we could write
>> > 
>> > (list #~"RAW_XML_HERE")
>> > 
>> > or even something like this:
>> > 
>> > (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>> 
>> Does it mean that the specification does not allow it now? Or does it
>> mean that it is not possible with my implementation?
> I think your serialize would have to unpack the G-Expressions.  You can
> test that with some example configs of your own.
>
>> > 
> Cheers
Taiju HIGASHI Oct. 12, 2022, 11:38 a.m. UTC | #12 
Andrew Tropin <andrew@trop.in> writes:

> On 2022-10-11 12:54, Taiju HIGASHI wrote:
>
>> Hi Andrew,
>>
>> Thank you for your review!
>>
>>>> +(define (string-list? value)
>>>> +  (and (pair? value) (every string? value)))
>>>
>>> Better to use list? here and in the other places, at least for the
>>> consistency, but also for semantic meaning.
>>
>> OK. I'll rewrite it to below.
>>
>> --8<---------------cut here---------------start------------->8---
>> (define (string-list? value)
>>   (and (list? value) (every string? value)))
>> --8<---------------cut here---------------end--------------->8---
>>
>>>> +
>>>> +(define (serialize-string-list field-name value)
>>>> +  (sxml->xml-string
>>>> +   (map
>>>> +    (lambda (path) `(dir ,path))
>>>> +    (if (member guix-home-font-dir value)
>>>> +        value
>>>> +        (append (list guix-home-font-dir) value)))))
>>>> +
>>>> +(define (serialize-string field-name value)
>>>> +  (define (serialize type value)
>>>> +    (sxml->xml-string
>>>> +     `(alias
>>>> +       (family ,type)
>>>> +       (prefer
>>>> +        (family ,value)))))
>>>> +  (match (list field-name value)
>>>> +    (('default-font-serif-family family)
>>>> +     (serialize 'serif family))
>>>> +    (('default-font-sans-serif-family family)
>>>> +     (serialize 'sans-serif family))
>>>> +    (('default-font-monospace-family family)
>>>> +     (serialize 'monospace family))))
>>>> +
>>>> +(define-maybe string)
>>>> +
>>>> +(define extra-config-list? list?)
>>>> +
>>>> +(define-maybe extra-config-list)
>>>> +
>>>> +(define (serialize-extra-config-list field-name value)
>>>> +  (sxml->xml-string
>>>> +   (map (match-lambda
>>>> +          ((? pair? sxml) sxml)
>>>
>>> Other branches would never be visited because it will fail earlier by
>>> define-configuration predicate check for extra-config-list? (which is
>>> basically list?).
>
> Oh, I missed the map over the list elements and slightly missread the
> code.  I thought (according to my incorrect perception of
> implementation) extra-config have to be either sxml or string, that's is
> why I said that it will fail earlier because plan string value won't
> satisfy list predicate attached to extra-config field, but in a fact
> extra-config is always a list, but can be a list of sxml's and strings
> mixed together.
>
> Thus, some of my comments are invalid.  Sorry for the confusion.  I'll
> rephrase and elaborate in the later message.

I was worried that I was the only one who did not understand the code I
wrote, but I've relieved to hear that it was a misunderstanding :)

Is it OK to have multiple data types (XML string and SXML list) in a
list?

>> We can specify invalid value such as (list "foo" '(foo bar) 123).
>>
>>> Also, making multi-type fields is debatable, but isn't great IMO.
>>
>> I see. If we had to choose one or the other, I would prefer the
>> string-type field.
>>
>>> If serialization would support G-exps, we could write
>>>
>>> (list #~"RAW_XML_HERE")
>>>
>>> or even something like this:
>>>
>>> (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>>
>> Does it mean that the specification does not allow it now? Or does it
>> mean that it is not possible with my implementation?
>>
>
> It's not possible with the current implementation.

I'll try to modify it so that it can support G-exps.

>>>> + ((? string? xml) (xml->sxml xml)) + (else + (raise
>>>> (formatted-message + (G_ "'extra-config' type must be xml string or
>>>> sxml list, was given: ~a") + value)))) + value))) +
>>>> +(define-configuration home-fontconfig-configuration +
>>>> (font-directories + (string-list (list guix-home-font-dir))
>>>
>>> It's not a generic string-list, but a specific font-directories-list
>>> with extra logic inside.
>>>
>>> Also, because guix-home-font-dir always added to the list, the default
>>> value should '() and field should be called additional-font-directories
>>> instead.  Otherwise it will be confusing, why guix-home-font-dir is not
>>> removed from the final configuration, when this field is set to a
>>> different value.
>>>
>>> I skimmed previous messages, but sorry, if I missed any already
>>> mentioned points.
>>
>> Sure, It is more appropriate that the field type is to
>> font-directories-list field name is to additional-font-directories.
>>
>
> As Liliana mentioned in the other message, it's better not to set
> anything implicitly.  It's better to keep the name, but change the
> implementation and remove implicitly and unconditionally added
> directory.

OK.  I'll modify the default value to an empty list and include
~/.guix-home/profile/share/fonts in the sample code in the
documentation.

>>>> +   "The directory list that provides fonts.")
>>>> +  (default-font-serif-family
>>>> +    maybe-string
>>>> +    "The preffered default fonts of serif.")
>>>> +  (default-font-sans-serif-family
>>>> +    maybe-string
>>>> +    "The preffered default fonts of sans-serif.")
>>>> +  (default-font-monospace-family
>>>> +    maybe-string
>>>> +    "The preffered default fonts of monospace.")
>>>> +  (extra-config
>>>> +   maybe-extra-config-list
>>>> +   "Extra configuration values to append to the fonts.conf."))
>>>> +
>>>> +(define (add-fontconfig-config-file user-config)
>>>>    `(("fontconfig/fonts.conf"
>>>>       ,(mixed-text-file
>>>>         "fonts.conf"
>>>>         "<?xml version='1.0'?>
>>>>  <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
>>>> -<fontconfig>
>>>> -  <dir>~/.guix-home/profile/share/fonts</dir>
>>>> -</fontconfig>"))))
>>>> +<fontconfig>"
>>>> +       (serialize-configuration user-config home-fontconfig-configuration-fields)
>>>
>>> Just a thought for the future and a point for configuration module
>>> improvements: It would be cool if serialize-configuration and all other
>>> serialize- functions returned a G-exps, this way we could write
>>> something like that:
>>>
>>> (home-fontconfig-configuration
>>>  (font-directories (list (file-append font-iosevka "/share/fonts"))))
>>
>> Nice.
>>
>> Thanks,
>> --
>> Taiju

Thanks,
--
Taiju
Taiju HIGASHI Oct. 12, 2022, 11:42 a.m. UTC | #13 
Andrew Tropin <andrew@trop.in> writes:

> On 2022-10-11 06:21, Liliana Marie Prikler wrote:
>
>> Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
>>> We can specify invalid value such as (list "foo" '(foo bar) 123).
>> It will be sanitized before that.
>>
>>> > Also, making multi-type fields is debatable, but isn't great IMO.
>>>
>>> I see. If we had to choose one or the other, I would prefer the
>>> string-type field.
>> Prefer sexp-type.
>>
>
> Current (v5) extra-config has a list type.  This list can contain strings
> and nested lists, string elements are for raw XML, and list
> elements are for SXML.
>
> This is done I guess to support following use case:
>
> (list "<tag>Already existing XML copied from existing .xml file, which
> we don't want to rewrite to SXML.</tag>"
>       '((tag (@ (attr1 "value1")
>                 (attr2 "value2"))
>              (nested "Part of the configuration defined with SXML")
>              (empty)))
>       "<another-tag>Maybe some other part of raw XML</another-tag>")
>
> This way we can combine SXML with already existing raw XML.
>
> Am I right?

You're right.  The current implementation allows XML string and SXML
list in the list.  Also, it can mix those.

>>> > If serialization would support G-exps, we could write
>>> >
>>> > (list #~"RAW_XML_HERE")
>>> >
>>> > or even something like this:
>>> >
>>> > (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>>>
>>> Does it mean that the specification does not allow it now? Or does it
>>> mean that it is not possible with my implementation?
>> I think your serialize would have to unpack the G-Expressions.  You can
>> test that with some example configs of your own.
>>
>>> >
>> Cheers

Thanks,
Andrew Tropin Oct. 12, 2022, 12:41 p.m. UTC | #14 
On 2022-10-12 20:38, Taiju HIGASHI wrote:

> Andrew Tropin <andrew@trop.in> writes:
>
>> On 2022-10-11 12:54, Taiju HIGASHI wrote:
>>
>>> Hi Andrew,
>>>
>>> Thank you for your review!
>>>
>>>>> +(define (string-list? value)
>>>>> +  (and (pair? value) (every string? value)))
>>>>
>>>> Better to use list? here and in the other places, at least for the
>>>> consistency, but also for semantic meaning.
>>>
>>> OK. I'll rewrite it to below.
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> (define (string-list? value)
>>>   (and (list? value) (every string? value)))
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>>>> +
>>>>> +(define (serialize-string-list field-name value)
>>>>> +  (sxml->xml-string
>>>>> +   (map
>>>>> +    (lambda (path) `(dir ,path))
>>>>> +    (if (member guix-home-font-dir value)
>>>>> +        value
>>>>> +        (append (list guix-home-font-dir) value)))))
>>>>> +
>>>>> +(define (serialize-string field-name value)
>>>>> +  (define (serialize type value)
>>>>> +    (sxml->xml-string
>>>>> +     `(alias
>>>>> +       (family ,type)
>>>>> +       (prefer
>>>>> +        (family ,value)))))
>>>>> +  (match (list field-name value)
>>>>> +    (('default-font-serif-family family)
>>>>> +     (serialize 'serif family))
>>>>> +    (('default-font-sans-serif-family family)
>>>>> +     (serialize 'sans-serif family))
>>>>> +    (('default-font-monospace-family family)
>>>>> +     (serialize 'monospace family))))
>>>>> +
>>>>> +(define-maybe string)
>>>>> +
>>>>> +(define extra-config-list? list?)
>>>>> +
>>>>> +(define-maybe extra-config-list)
>>>>> +
>>>>> +(define (serialize-extra-config-list field-name value)
>>>>> +  (sxml->xml-string
>>>>> +   (map (match-lambda
>>>>> +          ((? pair? sxml) sxml)
>>>>
>>>> Other branches would never be visited because it will fail earlier by
>>>> define-configuration predicate check for extra-config-list? (which is
>>>> basically list?).
>>
>> Oh, I missed the map over the list elements and slightly missread the
>> code.  I thought (according to my incorrect perception of
>> implementation) extra-config have to be either sxml or string, that's is
>> why I said that it will fail earlier because plan string value won't
>> satisfy list predicate attached to extra-config field, but in a fact
>> extra-config is always a list, but can be a list of sxml's and strings
>> mixed together.
>>
>> Thus, some of my comments are invalid.  Sorry for the confusion.  I'll
>> rephrase and elaborate in the later message.
>
> I was worried that I was the only one who did not understand the code I
> wrote, but I've relieved to hear that it was a misunderstanding :)
>
> Is it OK to have multiple data types (XML string and SXML list) in a
> list?
>

I think it's not a great practice, I'll describe an alternative approach
in the other message.

>>> We can specify invalid value such as (list "foo" '(foo bar) 123).
>>>
>>>> Also, making multi-type fields is debatable, but isn't great IMO.
>>>
>>> I see. If we had to choose one or the other, I would prefer the
>>> string-type field.
>>>
>>>> If serialization would support G-exps, we could write
>>>>
>>>> (list #~"RAW_XML_HERE")
>>>>
>>>> or even something like this:
>>>>
>>>> (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>>>
>>> Does it mean that the specification does not allow it now? Or does it
>>> mean that it is not possible with my implementation?
>>>
>>
>> It's not possible with the current implementation.
>
> I'll try to modify it so that it can support G-exps.
>
>>>>> + ((? string? xml) (xml->sxml xml)) + (else + (raise
>>>>> (formatted-message + (G_ "'extra-config' type must be xml string or
>>>>> sxml list, was given: ~a") + value)))) + value))) +
>>>>> +(define-configuration home-fontconfig-configuration +
>>>>> (font-directories + (string-list (list guix-home-font-dir))
>>>>
>>>> It's not a generic string-list, but a specific font-directories-list
>>>> with extra logic inside.
>>>>
>>>> Also, because guix-home-font-dir always added to the list, the default
>>>> value should '() and field should be called additional-font-directories
>>>> instead.  Otherwise it will be confusing, why guix-home-font-dir is not
>>>> removed from the final configuration, when this field is set to a
>>>> different value.
>>>>
>>>> I skimmed previous messages, but sorry, if I missed any already
>>>> mentioned points.
>>>
>>> Sure, It is more appropriate that the field type is to
>>> font-directories-list field name is to additional-font-directories.
>>>
>>
>> As Liliana mentioned in the other message, it's better not to set
>> anything implicitly.  It's better to keep the name, but change the
>> implementation and remove implicitly and unconditionally added
>> directory.
>
> OK.  I'll modify the default value to an empty list and include
> ~/.guix-home/profile/share/fonts in the sample code in the
> documentation.
>

The default value is good, but the code, which always adds
~/.guix-home/profile/share/fonts to fontdirs is not.

--8<---------------cut here---------------start------------->8---
+    (if (member guix-home-font-dir value)
+        value
+        (append (list guix-home-font-dir) value))
--8<---------------cut here---------------end--------------->8---


>>>>> +   "The directory list that provides fonts.")
>>>>> +  (default-font-serif-family
>>>>> +    maybe-string
>>>>> +    "The preffered default fonts of serif.")
>>>>> +  (default-font-sans-serif-family
>>>>> +    maybe-string
>>>>> +    "The preffered default fonts of sans-serif.")
>>>>> +  (default-font-monospace-family
>>>>> +    maybe-string
>>>>> +    "The preffered default fonts of monospace.")
>>>>> +  (extra-config
>>>>> +   maybe-extra-config-list
>>>>> +   "Extra configuration values to append to the fonts.conf."))
>>>>> +
>>>>> +(define (add-fontconfig-config-file user-config)
>>>>>    `(("fontconfig/fonts.conf"
>>>>>       ,(mixed-text-file
>>>>>         "fonts.conf"
>>>>>         "<?xml version='1.0'?>
>>>>>  <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
>>>>> -<fontconfig>
>>>>> -  <dir>~/.guix-home/profile/share/fonts</dir>
>>>>> -</fontconfig>"))))
>>>>> +<fontconfig>"
>>>>> +       (serialize-configuration user-config home-fontconfig-configuration-fields)
>>>>
>>>> Just a thought for the future and a point for configuration module
>>>> improvements: It would be cool if serialize-configuration and all other
>>>> serialize- functions returned a G-exps, this way we could write
>>>> something like that:
>>>>
>>>> (home-fontconfig-configuration
>>>>  (font-directories (list (file-append font-iosevka "/share/fonts"))))
>>>
>>> Nice.
>>>
>>> Thanks,
>>> --
>>> Taiju
>
> Thanks,
> --
> Taiju
Andrew Tropin Oct. 12, 2022, 1:03 p.m. UTC | #15 
On 2022-10-12 20:42, Taiju HIGASHI wrote:

> Andrew Tropin <andrew@trop.in> writes:
>
>> On 2022-10-11 06:21, Liliana Marie Prikler wrote:
>>
>>> Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju HIGASHI:
>>>> We can specify invalid value such as (list "foo" '(foo bar) 123).
>>> It will be sanitized before that.
>>>
>>>> > Also, making multi-type fields is debatable, but isn't great IMO.
>>>>
>>>> I see. If we had to choose one or the other, I would prefer the
>>>> string-type field.
>>> Prefer sexp-type.
>>>
>>
>> Current (v5) extra-config has a list type.  This list can contain strings
>> and nested lists, string elements are for raw XML, and list
>> elements are for SXML.
>>
>> This is done I guess to support following use case:
>>
>> (list "<tag>Already existing XML copied from existing .xml file, which
>> we don't want to rewrite to SXML.</tag>"
>>       '((tag (@ (attr1 "value1")
>>                 (attr2 "value2"))
>>              (nested "Part of the configuration defined with SXML")
>>              (empty)))
>>       "<another-tag>Maybe some other part of raw XML</another-tag>")
>>
>> This way we can combine SXML with already existing raw XML.
>>
>> Am I right?
>
> You're right.  The current implementation allows XML string and SXML
> list in the list.  Also, it can mix those.
>

Ok, that means we can cover this use case, but at the same time have
more functionality, clarity and consistency.

We can make extra-config to be SXML only (with G-exps support), this way
we will achieve not only the same functionality, but will get more
advanced features like referencing files/directiories in the /gnu/store
or generating parts of configuration using full-fledged scheme (the
simpliest example is just reading the content of the existing file-like
object or using format to generate "raw XML" and insert it in arbitrary
place of SXML tree).

--8<---------------cut here---------------start------------->8---
(list #~"<tag>Already existing XML copied from existing .xml file, which
we don't want to rewrite to SXML.</tag>"
      `((tag (@ (attr1 "value1")
                (attr2 "value2"))
             (nested "Part of the configuration defined with SXML")
             ,#~(format #f "    <nested-tag>~a</nested-tag>" #$variable)
             (fontdirs
              (dirs ,(file-append font-iosevka "/share/fonts")))
             (empty)))
      #~(call-with-input-file #$(local-file "old.xml") get-string-all)
      #~"<another-tag>Maybe some other part of raw XML</another-tag>")
--8<---------------cut here---------------end--------------->8---

Liliana, Ludo what do you think?

>>>> > If serialization would support G-exps, we could write
>>>> >
>>>> > (list #~"RAW_XML_HERE")
>>>> >
>>>> > or even something like this:
>>>> >
>>>> > (list #~(READ-THE-WHOLE-FILE #$(local-file "our-old.xml")))
>>>>
>>>> Does it mean that the specification does not allow it now? Or does it
>>>> mean that it is not possible with my implementation?
>>> I think your serialize would have to unpack the G-Expressions.  You can
>>> test that with some example configs of your own.
>>>
>>>> >
>>> Cheers
>
> Thanks,
Liliana Marie Prikler Oct. 12, 2022, 6:23 p.m. UTC | #16 
Am Mittwoch, dem 12.10.2022 um 17:03 +0400 schrieb Andrew Tropin:
> On 2022-10-12 20:42, Taiju HIGASHI wrote:
> 
> > Andrew Tropin <andrew@trop.in> writes:
> > 
> > > On 2022-10-11 06:21, Liliana Marie Prikler wrote:
> > > 
> > > > Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju
> > > > HIGASHI:
> > > > > We can specify invalid value such as (list "foo" '(foo bar)
> > > > > 123).
> > > > It will be sanitized before that.
> > > > 
> > > > > > Also, making multi-type fields is debatable, but isn't
> > > > > > great IMO.
> > > > > 
> > > > > I see. If we had to choose one or the other, I would prefer
> > > > > the
> > > > > string-type field.
> > > > Prefer sexp-type.
> > > > 
> > > 
> > > Current (v5) extra-config has a list type.  This list can contain
> > > strings
> > > and nested lists, string elements are for raw XML, and list
> > > elements are for SXML.
> > > 
> > > This is done I guess to support following use case:
> > > 
> > > (list "<tag>Already existing XML copied from existing .xml file,
> > > which
> > > we don't want to rewrite to SXML.</tag>"
> > >       '((tag (@ (attr1 "value1")
> > >                 (attr2 "value2"))
> > >              (nested "Part of the configuration defined with
> > > SXML")
> > >              (empty)))
> > >       "<another-tag>Maybe some other part of raw XML</another-
> > > tag>")
> > > 
> > > This way we can combine SXML with already existing raw XML.
> > > 
> > > Am I right?
> > 
> > You're right.  The current implementation allows XML string and
> > SXML
> > list in the list.  Also, it can mix those.
> > 
> 
> Ok, that means we can cover this use case, but at the same time have
> more functionality, clarity and consistency.
> 
> We can make extra-config to be SXML only (with G-exps support), this
> way we will achieve not only the same functionality, but will get
> more advanced features like referencing files/directiories in the
> /gnu/store or generating parts of configuration using full-fledged
> scheme (the simpliest example is just reading the content of the
> existing file-like object or using format to generate "raw XML" and
> insert it in arbitrary place of SXML tree).
> 
> --8<---------------cut here---------------start------------->8---
> (list #~"<tag>Already existing XML copied from existing .xml file,
> which
> we don't want to rewrite to SXML.</tag>"
>       `((tag (@ (attr1 "value1")
>                 (attr2 "value2"))
>              (nested "Part of the configuration defined with SXML")
>              ,#~(format #f "    <nested-tag>~a</nested-tag>"
> #$variable)
>              (fontdirs
>               (dirs ,(file-append font-iosevka "/share/fonts")))
>              (empty)))
>       #~(call-with-input-file #$(local-file "old.xml") get-string-
> all)
>       #~"<another-tag>Maybe some other part of raw XML</another-
> tag>")
> --8<---------------cut here---------------end--------------->8---
> 
> Liliana, Ludo what do you think?
I think the mockup implementation is a little unclear.  Do you mean
that G-Expressions should imply a string that is to be parsed?  Because
note that gexp->sexp exists and you could likewise #~(sxml->xml #$some-
file-in-the-store) imho.

Cheers
Andrew Tropin Oct. 13, 2022, 3:51 a.m. UTC | #17 
On 2022-10-12 20:23, Liliana Marie Prikler wrote:

> Am Mittwoch, dem 12.10.2022 um 17:03 +0400 schrieb Andrew Tropin:
>> On 2022-10-12 20:42, Taiju HIGASHI wrote:
>> 
>> > Andrew Tropin <andrew@trop.in> writes:
>> > 
>> > > On 2022-10-11 06:21, Liliana Marie Prikler wrote:
>> > > 
>> > > > Am Dienstag, dem 11.10.2022 um 12:54 +0900 schrieb Taiju
>> > > > HIGASHI:
>> > > > > We can specify invalid value such as (list "foo" '(foo bar)
>> > > > > 123).
>> > > > It will be sanitized before that.
>> > > > 
>> > > > > > Also, making multi-type fields is debatable, but isn't
>> > > > > > great IMO.
>> > > > > 
>> > > > > I see. If we had to choose one or the other, I would prefer
>> > > > > the
>> > > > > string-type field.
>> > > > Prefer sexp-type.
>> > > > 
>> > > 
>> > > Current (v5) extra-config has a list type.  This list can contain
>> > > strings
>> > > and nested lists, string elements are for raw XML, and list
>> > > elements are for SXML.
>> > > 
>> > > This is done I guess to support following use case:
>> > > 
>> > > (list "<tag>Already existing XML copied from existing .xml file,
>> > > which
>> > > we don't want to rewrite to SXML.</tag>"
>> > >       '((tag (@ (attr1 "value1")
>> > >                 (attr2 "value2"))
>> > >              (nested "Part of the configuration defined with
>> > > SXML")
>> > >              (empty)))
>> > >       "<another-tag>Maybe some other part of raw XML</another-
>> > > tag>")
>> > > 
>> > > This way we can combine SXML with already existing raw XML.
>> > > 
>> > > Am I right?
>> > 
>> > You're right.  The current implementation allows XML string and
>> > SXML
>> > list in the list.  Also, it can mix those.
>> > 
>> 
>> Ok, that means we can cover this use case, but at the same time have
>> more functionality, clarity and consistency.
>> 
>> We can make extra-config to be SXML only (with G-exps support), this
>> way we will achieve not only the same functionality, but will get
>> more advanced features like referencing files/directiories in the
>> /gnu/store or generating parts of configuration using full-fledged
>> scheme (the simpliest example is just reading the content of the
>> existing file-like object or using format to generate "raw XML" and
>> insert it in arbitrary place of SXML tree).
>> 
>> --8<---------------cut here---------------start------------->8---
>> (list #~"<tag>Already existing XML copied from existing .xml file,
>> which
>> we don't want to rewrite to SXML.</tag>"
>>       `((tag (@ (attr1 "value1")
>>                 (attr2 "value2"))
>>              (nested "Part of the configuration defined with SXML")
>>              ,#~(format #f "    <nested-tag>~a</nested-tag>"
>> #$variable)
>>              (fontdirs
>>               (dirs ,(file-append font-iosevka "/share/fonts")))
>>              (empty)))
>>       #~(call-with-input-file #$(local-file "old.xml") get-string-
>> all)
>>       #~"<another-tag>Maybe some other part of raw XML</another-
>> tag>")
>> --8<---------------cut here---------------end--------------->8---
>> 
>> Liliana, Ludo what do you think?
> I think the mockup implementation is a little unclear. 

The file generated from definition above should look like:
--8<---------------cut here---------------start------------->8---
<tag>Already existing XML copied from existing .xml file, which we don't want to rewrite to SXML.</tag>
<tag attr1="value1"
     attr2="value2">
  <nested>Text node</nested>
    <nested-tag>variable value here</nested-tag>
  <fontdirs>
    <dirs>
      /gnu/store/w2wvg2229lj3qba0r44pmd786mkllvjl-font-iosevka-15.2.0/share/fonts
    </dirs>
  </fontdirs>
  <empty/>
</tag>
<!-- the literal content of old.xml file here -->
<another-tag>Maybe some other part of raw XML</another-tag>
--8<---------------cut here---------------end--------------->8---

Hope it helps, let me know if you want me to rephrase or clarify
something.

> Do you mean that G-Expressions should imply a string that is to be
> parsed?  Because note that gexp->sexp exists and you could likewise
> #~(sxml->xml #$some- file-in-the-store) imho.

Not sure what you mean. Can you elaborate, please?
Ludovic Courtès Oct. 13, 2022, 12:37 p.m. UTC | #18 
Hello,

Andrew Tropin <andrew@trop.in> skribis:

> If serialization would support G-exps, we could write 
>
> (list #~"RAW_XML_HERE")

There’s a one-to-one lossless mapping between XML and SXML, so I don’t
think it makes sense to support XML-in-strings when we have SXML.

The only thing it would give us, as I see it, is the ability to generate
syntactically-invalid XML.  Maybe we can live without it?  :-)

Thanks,
Ludo’.
Andrew Tropin Oct. 14, 2022, 5:06 a.m. UTC | #19 
On 2022-10-13 14:37, Ludovic Courtès wrote:

> Hello,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> If serialization would support G-exps, we could write 
>>
>> (list #~"RAW_XML_HERE")
>
> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
> think it makes sense to support XML-in-strings when we have SXML.
>
> The only thing it would give us, as I see it, is the ability to generate
> syntactically-invalid XML.  Maybe we can live without it?  :-)

Of course we can :), but we won't be able:

1. To take already existing xml config and use it without rewriting.

2. Use full path to gnu store of file-like objects.
Taiju HIGASHI Oct. 15, 2022, 11:13 a.m. UTC | #20 
Andrew Tropin <andrew@trop.in> writes:

> On 2022-10-13 14:37, Ludovic Courtès wrote:
>
>> Hello,
>>
>> Andrew Tropin <andrew@trop.in> skribis:
>>
>>> If serialization would support G-exps, we could write
>>>
>>> (list #~"RAW_XML_HERE")
>>
>> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
>> think it makes sense to support XML-in-strings when we have SXML.
>>
>> The only thing it would give us, as I see it, is the ability to generate
>> syntactically-invalid XML.  Maybe we can live without it?  :-)
>
> Of course we can :), but we won't be able:
>
> 1. To take already existing xml config and use it without rewriting.

I find it surprisingly important to be able to simply copy and paste
settings without having to rewrite existing settings or those listed on
a web page somewhere.  I know we can easily convert from XML to SXML,
but those unfamiliar with SXML may find it a bothering task.

> 2. Use full path to gnu store of file-like objects.

Thanks,
Ludovic Courtès Oct. 17, 2022, 4:28 p.m. UTC | #21 
Hi,

Taiju HIGASHI <higashi@taiju.info> skribis:

> Andrew Tropin <andrew@trop.in> writes:

[...]

>>> Andrew Tropin <andrew@trop.in> skribis:
>>>
>>>> If serialization would support G-exps, we could write
>>>>
>>>> (list #~"RAW_XML_HERE")
>>>
>>> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
>>> think it makes sense to support XML-in-strings when we have SXML.
>>>
>>> The only thing it would give us, as I see it, is the ability to generate
>>> syntactically-invalid XML.  Maybe we can live without it?  :-)
>>
>> Of course we can :), but we won't be able:
>>
>> 1. To take already existing xml config and use it without rewriting.
>
> I find it surprisingly important to be able to simply copy and paste
> settings without having to rewrite existing settings or those listed on
> a web page somewhere.  I know we can easily convert from XML to SXML,
> but those unfamiliar with SXML may find it a bothering task.

OK, that makes sense.

But then, let’s not allow users to intersperse XML-in-strings in the
middle of XML.  It should be either a user-provided file/string or the
generated config, but not a mixture of both; that’d be a recipe for
confusion.

How about this: the service takes either a <fontconfig-configuration>
record or a file-like object?

(We can even have a “gexp compiler” for <fontconfig-configuration> to
make that transparent.)

Thanks,
Ludo’.
Taiju HIGASHI Oct. 18, 2022, 12:41 p.m. UTC | #22 
Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Taiju HIGASHI <higashi@taiju.info> skribis:
>
>> Andrew Tropin <andrew@trop.in> writes:
>
> [...]
>
>>>> Andrew Tropin <andrew@trop.in> skribis:
>>>>
>>>>> If serialization would support G-exps, we could write
>>>>>
>>>>> (list #~"RAW_XML_HERE")
>>>>
>>>> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
>>>> think it makes sense to support XML-in-strings when we have SXML.
>>>>
>>>> The only thing it would give us, as I see it, is the ability to generate
>>>> syntactically-invalid XML.  Maybe we can live without it?  :-)
>>>
>>> Of course we can :), but we won't be able:
>>>
>>> 1. To take already existing xml config and use it without rewriting.
>>
>> I find it surprisingly important to be able to simply copy and paste
>> settings without having to rewrite existing settings or those listed on
>> a web page somewhere.  I know we can easily convert from XML to SXML,
>> but those unfamiliar with SXML may find it a bothering task.
>
> OK, that makes sense.
>
> But then, let’s not allow users to intersperse XML-in-strings in the
> middle of XML.  It should be either a user-provided file/string or the
> generated config, but not a mixture of both; that’d be a recipe for
> confusion.
>
> How about this: the service takes either a <fontconfig-configuration>
> record or a file-like object?
>
> (We can even have a “gexp compiler” for <fontconfig-configuration> to
> make that transparent.)

Thank you for your consideration.

That idea sounds good.  I don't know if I can successfully implement
this, but I will consider it and give it a try.

Thanks,
Taiju HIGASHI Oct. 19, 2022, 9:42 p.m. UTC | #23 
Taiju HIGASHI <higashi@taiju.info> writes:

> Hi,
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi,
>>
>> Taiju HIGASHI <higashi@taiju.info> skribis:
>>
>>> Andrew Tropin <andrew@trop.in> writes:
>>
>> [...]
>>
>>>>> Andrew Tropin <andrew@trop.in> skribis:
>>>>>
>>>>>> If serialization would support G-exps, we could write
>>>>>>
>>>>>> (list #~"RAW_XML_HERE")
>>>>>
>>>>> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
>>>>> think it makes sense to support XML-in-strings when we have SXML.
>>>>>
>>>>> The only thing it would give us, as I see it, is the ability to generate
>>>>> syntactically-invalid XML.  Maybe we can live without it?  :-)
>>>>
>>>> Of course we can :), but we won't be able:
>>>>
>>>> 1. To take already existing xml config and use it without rewriting.
>>>
>>> I find it surprisingly important to be able to simply copy and paste
>>> settings without having to rewrite existing settings or those listed on
>>> a web page somewhere.  I know we can easily convert from XML to SXML,
>>> but those unfamiliar with SXML may find it a bothering task.
>>
>> OK, that makes sense.
>>
>> But then, let’s not allow users to intersperse XML-in-strings in the
>> middle of XML.  It should be either a user-provided file/string or the
>> generated config, but not a mixture of both; that’d be a recipe for
>> confusion.
>>
>> How about this: the service takes either a <fontconfig-configuration>
>> record or a file-like object?
>>
>> (We can even have a “gexp compiler” for <fontconfig-configuration> to
>> make that transparent.)
>
> Thank you for your consideration.
>
> That idea sounds good.  I don't know if I can successfully implement
> this, but I will consider it and give it a try.
>
> Thanks,

I'm trying to implement the following, is it consistent with the intent
of what you suggested?

--8<---------------cut here---------------start------------->8---
(define (add-fontconfig-config-file user-config)
  `(("fontconfig/fonts.conf"
     ,(if (home-fontconfig-configuration? user-config)
          (mixed-text-file
           "fonts.conf"
           "<?xml version='1.0'?>
<!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
<fontconfig>"
           (serialize-configuration user-config home-fontconfig-configuration-fields)
           "</fontconfig>\n")
          user-config))))
--8<---------------cut here---------------end--------------->8---

It is assumed that configurations can be specified in one of the
following ways.

* fontconfig-configuration:

--8<---------------cut here---------------start------------->8---
(home-environment
 (packages (list font-google-noto))
 (services
  (append
      (list
       (service home-bash-service-type))
      (modify-services %home-base-services
        (home-fontconfig-service-type
         config => (home-fontconfig-configuration
                    (font-directories
                     (cons* "~/fonts" %home-fontconfig-base-font-directories))
                    (default-font-serif-family "Noto Serif CJK JP")
                    (default-font-sans-serif-family "Noto Sans Serif CJK JP")
                    (default-font-monospace-family "PlemolJP Console")
                    (extra-config
                     '(foo "bar"))))))))
--8<---------------cut here---------------end--------------->8---

Note:
%home-fontconfig-base-font-directories is the new variable I plan to
export as the default value, based on Andrew's and Liliana's point.

* file-like objects:

--8<---------------cut here---------------start------------->8---
(home-environment
 (packages (list font-google-noto))
 (services
  (append
      (list
       (service home-bash-service-type))
      (modify-services %home-base-services
        (home-fontconfig-service-type
         config => (local-file "/path/to/your/fonts.conf"))))))
--8<---------------cut here---------------end--------------->8---

Thanks,
Declan Tsien Oct. 20, 2022, 1:23 a.m. UTC | #24 
Taiju HIGASHI <higashi@taiju.info> writes:

>                     (default-font-serif-family "Noto Serif CJK JP")
>                     (default-font-sans-serif-family "Noto Sans Serif CJK JP")
>                     (default-font-monospace-family "PlemolJP Console")

Does this take a list as value? Because I have specified some fallback fonts in my configuration.
I directly use sxml to serialize the config file right now. Below is a portion of it.

It would be great if I could use this home-service without writing extra sxml code once it gets merged.

#+begin_src scheme
  (alias (@ (binding "strong"))
	 (family "sans-serif")
	 (prefer
	  (family "WenQuanYi Micro Hei")
	  (family "Noto Sans")))

  (alias (@ (binding "strong"))
	 (family "monospace")
	 (prefer
	  (family "Sarasa Mono CL")
	  (family "Inconsolata")
	  (family "Noto Mono")))
#+end_src
Taiju HIGASHI Oct. 20, 2022, 1:37 a.m. UTC | #25 
Hi Declan,

Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>>                     (default-font-serif-family "Noto Serif CJK JP")
>>                     (default-font-sans-serif-family "Noto Sans Serif CJK JP")
>>                     (default-font-monospace-family "PlemolJP Console")
>
> Does this take a list as value? Because I have specified some fallback fonts in my configuration.
> I directly use sxml to serialize the config file right now. Below is a portion of it.
>
> It would be great if I could use this home-service without writing extra sxml code once it gets merged.
>
> #+begin_src scheme
>   (alias (@ (binding "strong"))
> 	 (family "sans-serif")
> 	 (prefer
> 	  (family "WenQuanYi Micro Hei")
> 	  (family "Noto Sans")))
>
>   (alias (@ (binding "strong"))
> 	 (family "monospace")
> 	 (prefer
> 	  (family "Sarasa Mono CL")
> 	  (family "Inconsolata")
> 	  (family "Noto Mono")))
> #+end_src
>

That makes sense.
I thought that being able to specify one preferred font would be
sufficient, but since actual fontconfig allows multiple specification, I
thought it would certainly be better to be able to specify more than one
in this setting as well.
By the way, should we be able to specify the binding attribute as well?

Best Regards,
Declan Tsien Oct. 20, 2022, 2:03 a.m. UTC | #26 
Taiju HIGASHI <higashi@taiju.info> writes:

>
> By the way, should we be able to specify the binding attribute as well?
>

I checked the fontconfig doc.
https://www.freedesktop.org/software/fontconfig/fontconfig-user.html
Here is the relevant portation:

> There is one special case to this rule; family names are split into
> two bindings; strong and weak. Strong family names are given greater
> precedence in the match than lang elements while weak family names are
> given lower precedence than lang elements. This permits the document
> language to drive font selection when any document specified font is
> unavailable.

I guess it's ok to ignore or set a default =strong= when serializing?
Taiju HIGASHI Oct. 20, 2022, 3:44 a.m. UTC | #27 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>>
>> By the way, should we be able to specify the binding attribute as well?
>>
>
> I checked the fontconfig doc.
> https://www.freedesktop.org/software/fontconfig/fontconfig-user.html
> Here is the relevant portation:
>
>> There is one special case to this rule; family names are split into
>> two bindings; strong and weak. Strong family names are given greater
>> precedence in the match than lang elements while weak family names are
>> given lower precedence than lang elements. This permits the document
>> language to drive font selection when any document specified font is
>> unavailable.
>
> I guess it's ok to ignore or set a default =strong= when serializing?
>

If you put the setting below,

--8<---------------cut here---------------start------------->8---
(home-fontconfig-configuration
  (default-font-serif-family "Noto Serif CJK JP")
  (default-font-sans-serif-family "Noto Sans Serif CJK JP")
  (default-font-monospace-family "PlemolJP Console"))
--8<---------------cut here---------------end--------------->8---

The current implementation serializes below.

--8<---------------cut here---------------start------------->8---
<!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
<fontconfig>
  <dir>~/.guix-home/profile/share/fonts</dir>
  <alias>
    <family>serif</family>
    <prefer>
      <family>Noto Serif CJK JP</family>
    </prefer>
  </alias>
  <alias>
    <family>sans-serif</family>
    <prefer>
      <family>Noto Sans Serif CJK JP</family>
    </prefer>
  </alias>
  <alias>
    <family>monospace</family>
    <prefer>
      <family>PlemolJP Console</family>
    </prefer>
  </alias>
</fontconfig>
--8<---------------cut here---------------end--------------->8---

Since the binding attribute is omitted, it would be interpreted as the
default weak.
ref: https://github.com/behdad/fontconfig/blob/5b41ded2b0ddb98a07ac86264b94403cb7a0fd82/fonts.dtd#L127-L128

I would like the default-font-* fields to cover only typical settings.
Instead, we provide extra-config field to be used for settings that are
not typical.

You can also configure the settings you want by specifying them in
extra-config.

--8<---------------cut here---------------start------------->8---
(home-fontconfig-configuration
 (extra-config
  '((alias (@ (binding "strong"))
	   (family "sans-serif")
	   (prefer
	    (family "WenQuanYi Micro Hei")
	    (family "Noto Sans")))
    (alias (@ (binding "strong"))
	   (family "monospace")
	   (prefer
	    (family "Sarasa Mono CL")
	    (family "Inconsolata")
	    (family "Noto Mono"))))))
--8<---------------cut here---------------end--------------->8---

I don't see clearly what the typical configuration of alias should be,
but I believe the current specification is sufficient for our needs.

Do you still think it is preferable to change the default-font-* field
interface, even knowing that you can configure it in the extra-config
field?  Please give me your frank opinion :)

Thanks,
Declan Tsien Oct. 20, 2022, 5:06 a.m. UTC | #28 
Taiju HIGASHI <higashi@taiju.info> writes:

>
> You can also configure the settings you want by specifying them in
> extra-config.
>

Oh, nice. So my use case is covered. Didn't realize that. Nice work.

>
> I don't see clearly what the typical configuration of alias should be,
> but I believe the current specification is sufficient for our needs.
>
> Do you still think it is preferable to change the default-font-* field
> interface, even knowing that you can configure it in the extra-config
> field?  Please give me your frank opinion :)
>

Let's stick to the current specification, no changes needed. Cheers.

Thanks
Declan Tsien Oct. 20, 2022, 5:40 a.m. UTC | #29 
Taiju HIGASHI <higashi@taiju.info> writes:

> @@ -59,7 +136,7 @@ (define home-fontconfig-service-type
>                         (service-extension
>                          home-profile-service-type
>                          (const (list fontconfig)))))
> -                (default-value #f)
> +                (default-value (home-fontconfig-configuration))
>                  (description
>                   "Provides configuration file for fontconfig and make
>  fc-* utilities aware of font packages installed in Guix Home's profile.")))
> -- 
> 2.37.3

Do we also have support service extension here? like this.

#+begin_src scheme
  (compose identity)
  (extend home-fontconfig-extensions)
  (default-value (home-fontconfig-configuration))
#+end_src
Taiju HIGASHI Oct. 21, 2022, 1:02 a.m. UTC | #30 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>>
>> You can also configure the settings you want by specifying them in
>> extra-config.
>>
>
> Oh, nice. So my use case is covered. Didn't realize that. Nice work.
>
>>
>> I don't see clearly what the typical configuration of alias should be,
>> but I believe the current specification is sufficient for our needs.
>>
>> Do you still think it is preferable to change the default-font-* field
>> interface, even knowing that you can configure it in the extra-config
>> field?  Please give me your frank opinion :)
>>
>
> Let's stick to the current specification, no changes needed. Cheers.

OK. Thank you for your input!

Thanks,
Taiju HIGASHI Oct. 21, 2022, 4:03 a.m. UTC | #31 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>> @@ -59,7 +136,7 @@ (define home-fontconfig-service-type
>>                         (service-extension
>>                          home-profile-service-type
>>                          (const (list fontconfig)))))
>> -                (default-value #f)
>> +                (default-value (home-fontconfig-configuration))
>>                  (description
>>                   "Provides configuration file for fontconfig and make
>>  fc-* utilities aware of font packages installed in Guix Home's profile.")))
>> --
>> 2.37.3
>
> Do we also have support service extension here? like this.
>
> #+begin_src scheme
>   (compose identity)
>   (extend home-fontconfig-extensions)
>   (default-value (home-fontconfig-configuration))
> #+end_src
>

Sorry, I didn't understand your question. Could you give me more
specific needs?

Thanks,
Declan Tsien Oct. 21, 2022, 5:02 a.m. UTC | #32 
Taiju HIGASHI <higashi@taiju.info> writes:

>
> Sorry, I didn't understand your question. Could you give me more
> specific needs?
>

My apologies. I should be more clear.

I think I saw we are using =modify-services= (somewhere in this
thread) to configure =home-fontconfig-service-type=. But wouldn't be nice
if user can just use =simple-service= to extend it?

Like this in my guix-config:

https://git.sr.ht/~declantsien/guix-config/tree/master/item/home-conf/appearance/font.scm#L30-55
https://git.sr.ht/~declantsien/guix-config/tree/master/item/guix/gnu/home/services/fontutils.scm#L94-96
Taiju HIGASHI Oct. 21, 2022, 8:01 a.m. UTC | #33 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>>
>> Sorry, I didn't understand your question. Could you give me more
>> specific needs?
>>
>
> My apologies. I should be more clear.
>
> I think I saw we are using =modify-services= (somewhere in this
> thread) to configure =home-fontconfig-service-type=. But wouldn't be nice
> if user can just use =simple-service= to extend it?
>
> Like this in my guix-config:
>
> https://git.sr.ht/~declantsien/guix-config/tree/master/item/home-conf/appearance/font.scm#L30-55
> https://git.sr.ht/~declantsien/guix-config/tree/master/item/guix/gnu/home/services/fontutils.scm#L94-96
>

Thank you for detail information.
Currently, It not support.  The interface is not suitable for extension,
so we decided to forgot it.
See: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#71

We have to come up with a merge strategy if we allow to extend, how
would you like to extend it?

Perhaps I am less experienced in Guix customization than you are, and
don't understand the use cases that cannot be achieved with
modify-services.

Thanks,
Declan Tsien Oct. 21, 2022, 9:15 a.m. UTC | #34 
Taiju HIGASHI <higashi@taiju.info> writes:

> See: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#71
>
> We have to come up with a merge strategy if we allow to extend, how
> would you like to extend it?

OK, got it. Sounds reasonable. 
I should have followed the conversation thoroughly,  Sorry about that.

>
> Perhaps I am less experienced in Guix customization than you are, and

Nah, I haven't contributed much to Guix community yet. Only poking around with my
guix-config ha.

> don't understand the use cases that cannot be achieved with
> modify-services.

I'd prefer =simple-service= over =modify-services= when possible.
For example, in this case. Let's say I want to add an item to
=font-directories=, I should not forget to include =guix-home-font-dir=
too, like this:

#+begin_src scheme
(home-fontconfig-configuration
 (font-directories
  (string-list (list guix-home-font-dir "another-dir")))
#+end_src

But with service extension I can just write:

#+begin_src scheme
(home-fontconfig-extension
 (font-directories
  (string-list (list "another-dir")))
#+end_src

=guix-home-font-dir= doesn't need to show up in my configuration.

----
Thanks
Taiju HIGASHI Oct. 23, 2022, 6:32 a.m. UTC | #35 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>> See: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#71
>>
>> We have to come up with a merge strategy if we allow to extend, how
>> would you like to extend it?
>
> OK, got it. Sounds reasonable.
> I should have followed the conversation thoroughly,  Sorry about that.
>
>>
>> Perhaps I am less experienced in Guix customization than you are, and
>
> Nah, I haven't contributed much to Guix community yet. Only poking around with my
> guix-config ha.
>
>> don't understand the use cases that cannot be achieved with
>> modify-services.
>
> I'd prefer =simple-service= over =modify-services= when possible.
> For example, in this case. Let's say I want to add an item to
> =font-directories=, I should not forget to include =guix-home-font-dir=
> too, like this:
>
> #+begin_src scheme
> (home-fontconfig-configuration
>  (font-directories
>   (string-list (list guix-home-font-dir "another-dir")))
> #+end_src
>
>
> But with service extension I can just write:
>
> #+begin_src scheme
> (home-fontconfig-extension
>  (font-directories
>   (string-list (list "another-dir")))
> #+end_src
>
> =guix-home-font-dir= doesn't need to show up in my configuration.

I see.  We may make the interface even more unsuitable for extensions
since we plan to allow the user to choose whether to configure with the
fontconfig-configuration or a file-like object.

I am taking a very long time to finalize the interface on this issue,
should I still think about it more carefully...?

Thanks,
Declan Tsien Oct. 23, 2022, 7:33 a.m. UTC | #36 
Taiju HIGASHI <higashi@taiju.info> writes:

>
> I see.  We may make the interface even more unsuitable for extensions
> since we plan to allow the user to choose whether to configure with the
> fontconfig-configuration or a file-like object.
>
> I am taking a very long time to finalize the interface on this issue,
> should I still think about it more carefully...?
>
> Thanks,
> -- 
> Taiju

I think we can stick to the current specification for now. We can go from
there later.
Taiju HIGASHI Oct. 23, 2022, 11:40 a.m. UTC | #37 
Declan Tsien <declantsien@riseup.net> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>>
>> I see.  We may make the interface even more unsuitable for extensions
>> since we plan to allow the user to choose whether to configure with the
>> fontconfig-configuration or a file-like object.
>>
>> I am taking a very long time to finalize the interface on this issue,
>> should I still think about it more carefully...?
>>
>> Thanks,
>> --
>> Taiju
>
> I think we can stick to the current specification for now. We can go from
> there later.
>

Thank you.  I will proceed with the current specification.

Thanks,
Taiju HIGASHI Oct. 27, 2022, 4 a.m. UTC | #38 
Hi,

Taiju HIGASHI <higashi@taiju.info> writes:

> Taiju HIGASHI <higashi@taiju.info> writes:
>
>> Hi,
>>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hi,
>>>
>>> Taiju HIGASHI <higashi@taiju.info> skribis:
>>>
>>>> Andrew Tropin <andrew@trop.in> writes:
>>>
>>> [...]
>>>
>>>>>> Andrew Tropin <andrew@trop.in> skribis:
>>>>>>
>>>>>>> If serialization would support G-exps, we could write
>>>>>>>
>>>>>>> (list #~"RAW_XML_HERE")
>>>>>>
>>>>>> There’s a one-to-one lossless mapping between XML and SXML, so I don’t
>>>>>> think it makes sense to support XML-in-strings when we have SXML.
>>>>>>
>>>>>> The only thing it would give us, as I see it, is the ability to generate
>>>>>> syntactically-invalid XML.  Maybe we can live without it?  :-)
>>>>>
>>>>> Of course we can :), but we won't be able:
>>>>>
>>>>> 1. To take already existing xml config and use it without rewriting.
>>>>
>>>> I find it surprisingly important to be able to simply copy and paste
>>>> settings without having to rewrite existing settings or those listed on
>>>> a web page somewhere.  I know we can easily convert from XML to SXML,
>>>> but those unfamiliar with SXML may find it a bothering task.
>>>
>>> OK, that makes sense.
>>>
>>> But then, let’s not allow users to intersperse XML-in-strings in the
>>> middle of XML.  It should be either a user-provided file/string or the
>>> generated config, but not a mixture of both; that’d be a recipe for
>>> confusion.
>>>
>>> How about this: the service takes either a <fontconfig-configuration>
>>> record or a file-like object?
>>>
>>> (We can even have a “gexp compiler” for <fontconfig-configuration> to
>>> make that transparent.)
>>
>> Thank you for your consideration.
>>
>> That idea sounds good.  I don't know if I can successfully implement
>> this, but I will consider it and give it a try.
>>
>> Thanks,
>
> I'm trying to implement the following, is it consistent with the intent
> of what you suggested?
>
> (define (add-fontconfig-config-file user-config)
>   `(("fontconfig/fonts.conf"
>      ,(if (home-fontconfig-configuration? user-config)
>           (mixed-text-file
>            "fonts.conf"
>            "<?xml version='1.0'?>
> <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
> <fontconfig>"
>            (serialize-configuration user-config home-fontconfig-configuration-fields)
>            "</fontconfig>\n")
>           user-config))))
>
>
> It is assumed that configurations can be specified in one of the
> following ways.
>
> * fontconfig-configuration:
>
> (home-environment
>  (packages (list font-google-noto))
>  (services
>   (append
>       (list
>        (service home-bash-service-type))
>       (modify-services %home-base-services
>         (home-fontconfig-service-type
>          config => (home-fontconfig-configuration
>                     (font-directories
>                      (cons* "~/fonts" %home-fontconfig-base-font-directories))
>                     (default-font-serif-family "Noto Serif CJK JP")
>                     (default-font-sans-serif-family "Noto Sans Serif CJK JP")
>                     (default-font-monospace-family "PlemolJP Console")
>                     (extra-config
>                      '(foo "bar"))))))))
>
>
> Note:
> %home-fontconfig-base-font-directories is the new variable I plan to
> export as the default value, based on Andrew's and Liliana's point.
>
> * file-like objects:
>
> (home-environment
>  (packages (list font-google-noto))
>  (services
>   (append
>       (list
>        (service home-bash-service-type))
>       (modify-services %home-base-services
>         (home-fontconfig-service-type
>          config => (local-file "/path/to/your/fonts.conf"))))))
>
> Thanks,

Sorry for the long time it has taken to resolve the issue.
What do you think about it?

Thanks,
Liliana Marie Prikler Oct. 27, 2022, 5:18 a.m. UTC | #39 
Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju HIGASHI:
> Sorry for the long time it has taken to resolve the issue.
> What do you think about it?
Putting the discussion with Declan aside, the last thing mentioned was
not trying to mix SXML and XML-in-strings.  Ludo offered the solutions:
1. Taking a <fontconfig-configuration> or a file-like object
2. (Optionally) using a gexp-compiler for the former

Cheers
Taiju HIGASHI Oct. 27, 2022, 5:31 a.m. UTC | #40 
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju HIGASHI:
>> Sorry for the long time it has taken to resolve the issue.
>> What do you think about it?
> Putting the discussion with Declan aside, the last thing mentioned was
> not trying to mix SXML and XML-in-strings.  Ludo offered the solutions:
> 1. Taking a <fontconfig-configuration> or a file-like object
> 2. (Optionally) using a gexp-compiler for the former
>
> Cheers

Sorry for the lack of clarity.
I had sent you a past email confirming that the direction of the
implementation was correct and was waiting for your response.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#239

Thanks,
Liliana Marie Prikler Oct. 27, 2022, 6:36 a.m. UTC | #41 
Am Donnerstag, dem 27.10.2022 um 14:31 +0900 schrieb Taiju HIGASHI:
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> 
> > Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju HIGASHI:
> > > Sorry for the long time it has taken to resolve the issue.
> > > What do you think about it?
> > Putting the discussion with Declan aside, the last thing mentioned
> > was
> > not trying to mix SXML and XML-in-strings.  Ludo offered the
> > solutions:
> > 1. Taking a <fontconfig-configuration> or a file-like object
> > 2. (Optionally) using a gexp-compiler for the former
> > 
> > Cheers
> 
> Sorry for the lack of clarity.
> I had sent you a past email confirming that the direction of the
> implementation was correct and was waiting for your response.
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#239
Ahh, I missed that.  If you pull in the XML declarations and the
<fontconfig></fontconfig> stuff to the serialization, you should
basically have most of what you'd need for a G-Exp compiler, but even
if not it'd simplify this to

(match
  ((? home-font-config-configuration? config)
   (serialize-... config ...))
  ((? file-like? config) config))

Not sure if a match for type-checking would be needed since it's
already taken care of elsewhere, so writing it just in case.

Cheers
Taiju HIGASHI Nov. 2, 2022, 1:43 a.m. UTC | #42 
Hi,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Donnerstag, dem 27.10.2022 um 14:31 +0900 schrieb Taiju HIGASHI:
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> > Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju HIGASHI:
>> > > Sorry for the long time it has taken to resolve the issue.
>> > > What do you think about it?
>> > Putting the discussion with Declan aside, the last thing mentioned
>> > was
>> > not trying to mix SXML and XML-in-strings.  Ludo offered the
>> > solutions:
>> > 1. Taking a <fontconfig-configuration> or a file-like object
>> > 2. (Optionally) using a gexp-compiler for the former
>> >
>> > Cheers
>>
>> Sorry for the lack of clarity.
>> I had sent you a past email confirming that the direction of the
>> implementation was correct and was waiting for your response.
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#239
> Ahh, I missed that.  If you pull in the XML declarations and the
> <fontconfig></fontconfig> stuff to the serialization, you should
> basically have most of what you'd need for a G-Exp compiler, but even
> if not it'd simplify this to
>
> (match
>   ((? home-font-config-configuration? config)
>    (serialize-... config ...))
>   ((? file-like? config) config))
>
> Not sure if a match for type-checking would be needed since it's
> already taken care of elsewhere, so writing it just in case.
>
> Cheers

Sorry for my response delay.
Is my recognition correct?  I have plan to rewrite it as below.

--8<---------------cut here---------------start------------->8---
(define (serialize-fontconfig-configuration config)
  (define start-of-fontconfig "<?xml version='1.0'?>
<!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
<fontconfig>")

  (define end-of-fontconfig "</fontconfig>\n")

  (mixed-text-file
   "fonts.conf"
   start-of-fontconfig
   (serialize-configuration config home-fontconfig-configuration-fields)
   end-of-fontconfig))

(define (add-fontconfig-config-file user-config)
  `(("fontconfig/fonts.conf"
     ,(match user-config
          ((? home-fontconfig-configuration? user-config)
           (serialize-fontconfig-configuration user-config))
        ((? file-like? user-config) user-config)))))
--8<---------------cut here---------------end--------------->8---

Thanks,
Liliana Marie Prikler Nov. 2, 2022, 6:45 a.m. UTC | #43 
Am Mittwoch, dem 02.11.2022 um 10:43 +0900 schrieb Taiju HIGASHI:
> Hi,
> 
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> 
> > Am Donnerstag, dem 27.10.2022 um 14:31 +0900 schrieb Taiju HIGASHI:
> > > Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> > > 
> > > > Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju
> > > > HIGASHI:
> > > > > Sorry for the long time it has taken to resolve the issue.
> > > > > What do you think about it?
> > > > Putting the discussion with Declan aside, the last thing
> > > > mentioned
> > > > was
> > > > not trying to mix SXML and XML-in-strings.  Ludo offered the
> > > > solutions:
> > > > 1. Taking a <fontconfig-configuration> or a file-like object
> > > > 2. (Optionally) using a gexp-compiler for the former
> > > > 
> > > > Cheers
> > > 
> > > Sorry for the lack of clarity.
> > > I had sent you a past email confirming that the direction of the
> > > implementation was correct and was waiting for your response.
> > > 
> > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#239
> > Ahh, I missed that.  If you pull in the XML declarations and the
> > <fontconfig></fontconfig> stuff to the serialization, you should
> > basically have most of what you'd need for a G-Exp compiler, but
> > even
> > if not it'd simplify this to
> > 
> > (match
> >   ((? home-font-config-configuration? config)
> >    (serialize-... config ...))
> >   ((? file-like? config) config))
> > 
> > Not sure if a match for type-checking would be needed since it's
> > already taken care of elsewhere, so writing it just in case.
> > 
> > Cheers
> 
> Sorry for my response delay.
> Is my recognition correct?  I have plan to rewrite it as below.
> 
> --8<---------------cut here---------------start------------->8---
> (define (serialize-fontconfig-configuration config)
>   (define start-of-fontconfig "<?xml version='1.0'?>
> <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
> <fontconfig>")
> 
>   (define end-of-fontconfig "</fontconfig>\n")
> 
>   (mixed-text-file
>    "fonts.conf"
>    start-of-fontconfig
>    (serialize-configuration config home-fontconfig-configuration-
> fields)
>    end-of-fontconfig))
> 
> (define (add-fontconfig-config-file user-config)
>   `(("fontconfig/fonts.conf"
>      ,(match user-config
>           ((? home-fontconfig-configuration? user-config)
>            (serialize-fontconfig-configuration user-config))
>         ((? file-like? user-config) user-config)))))
> --8<---------------cut here---------------end--------------->8---
More or less.  For one, I don't think start-of-fontconfig and end-of-
fontconfig need to be declared.  The (serialize-configuration ) call is
a little opaque atm, but let's suppose it returns properly formatted
XML.  Finally, as hinted already and since you're returning a file-like
object anyway, you may want to make this serializer a gexp-compiler
instead.

Cheers
Taiju HIGASHI Nov. 4, 2022, 8:46 a.m. UTC | #44 
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Am Mittwoch, dem 02.11.2022 um 10:43 +0900 schrieb Taiju HIGASHI:
>> Hi,
>>
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>> > Am Donnerstag, dem 27.10.2022 um 14:31 +0900 schrieb Taiju HIGASHI:
>> > > Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>> > >
>> > > > Am Donnerstag, dem 27.10.2022 um 13:00 +0900 schrieb Taiju
>> > > > HIGASHI:
>> > > > > Sorry for the long time it has taken to resolve the issue.
>> > > > > What do you think about it?
>> > > > Putting the discussion with Declan aside, the last thing
>> > > > mentioned
>> > > > was
>> > > > not trying to mix SXML and XML-in-strings.  Ludo offered the
>> > > > solutions:
>> > > > 1. Taking a <fontconfig-configuration> or a file-like object
>> > > > 2. (Optionally) using a gexp-compiler for the former
>> > > >
>> > > > Cheers
>> > >
>> > > Sorry for the lack of clarity.
>> > > I had sent you a past email confirming that the direction of the
>> > > implementation was correct and was waiting for your response.
>> > >
>> > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=57963#239
>> > Ahh, I missed that.  If you pull in the XML declarations and the
>> > <fontconfig></fontconfig> stuff to the serialization, you should
>> > basically have most of what you'd need for a G-Exp compiler, but
>> > even
>> > if not it'd simplify this to
>> >
>> > (match
>> >   ((? home-font-config-configuration? config)
>> >    (serialize-... config ...))
>> >   ((? file-like? config) config))
>> >
>> > Not sure if a match for type-checking would be needed since it's
>> > already taken care of elsewhere, so writing it just in case.
>> >
>> > Cheers
>>
>> Sorry for my response delay.
>> Is my recognition correct?  I have plan to rewrite it as below.
>>
>> --8<---------------cut here---------------start------------->8---
>> (define (serialize-fontconfig-configuration config)
>>   (define start-of-fontconfig "<?xml version='1.0'?>
>> <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
>> <fontconfig>")
>>
>>   (define end-of-fontconfig "</fontconfig>\n")
>>
>>   (mixed-text-file
>>    "fonts.conf"
>>    start-of-fontconfig
>>    (serialize-configuration config home-fontconfig-configuration-
>> fields)
>>    end-of-fontconfig))
>>
>> (define (add-fontconfig-config-file user-config)
>>   `(("fontconfig/fonts.conf"
>>      ,(match user-config
>>           ((? home-fontconfig-configuration? user-config)
>>            (serialize-fontconfig-configuration user-config))
>>         ((? file-like? user-config) user-config)))))
>> --8<---------------cut here---------------end--------------->8---
> More or less.  For one, I don't think start-of-fontconfig and end-of-
> fontconfig need to be declared.  The (serialize-configuration ) call is
> a little opaque atm, but let's suppose it returns properly formatted
> XML.  Finally, as hinted already and since you're returning a file-like
> object anyway, you may want to make this serializer a gexp-compiler
> instead.
>
> Cheers

Sorry. I did not understand what you meant by making it a gexp-compiler
instead. Is there anything reference documents or codes?

I believe it was presented to me in advance as an optional proposal, but
I do not know what it means and have not been able to respond.

Thanks,
\( Nov. 4, 2022, 4:29 p.m. UTC | #45 
Heya,

On Fri Nov 4, 2022 at 8:46 AM GMT, Taiju HIGASHI wrote:
> Sorry. I did not understand what you meant by making it a gexp-compiler
> instead. Is there anything reference documents or codes?

Guix's file-like objects are compiled into derivations using gexp-compilers,
which may be defined using the ``define-gexp-compiler'' form. These two are
equivalent:

  ;;; with procedure

  (define (foo->file-like foo)
    "Turns FOO into a derivation."
    (plain-file "foo"
      (foo-text foo)))

  ;; this way, you need to use foo->file-like whenever you want to use
  ;; foo in place of a file-like object

  (foo->file-like (foo (text "hello")))

  ;;; with gexp compiler

  (define-gexp-compiler (foo-compiler (foo <foo-record>) system target)
    ;;
    (lower-object
     (plain-file "foo"
       (foo-text foo))))

  ;; now, a ``foo'' can be treated as a lowerable file-like object! you
  ;; can put it anywhere you'd put a file-like.
  (foo (text "hello"))

So, basically, define-gexp-compiler lets you make new kinds of file-like
object from records! This is actually how computed-file, file-append, et
al are defined; see guix/gexp.scm. (Many of the gexp-compilers define both
a compiler and an expander; the compiler is a derivation to build when
the object is built, and the expander is the string to return when it's
gexped. file-append [line 680 in my checkout] is a good, clear example of
this.)

    -- (
Taiju HIGASHI Nov. 6, 2022, 1:24 p.m. UTC | #46 
"(" <paren@disroot.org> writes:

> Heya,
>
> On Fri Nov 4, 2022 at 8:46 AM GMT, Taiju HIGASHI wrote:
>> Sorry. I did not understand what you meant by making it a gexp-compiler
>> instead. Is there anything reference documents or codes?
>
> Guix's file-like objects are compiled into derivations using gexp-compilers,
> which may be defined using the ``define-gexp-compiler'' form. These two are
> equivalent:
>
>   ;;; with procedure
>
>   (define (foo->file-like foo)
>     "Turns FOO into a derivation."
>     (plain-file "foo"
>       (foo-text foo)))
>
>   ;; this way, you need to use foo->file-like whenever you want to use
>   ;; foo in place of a file-like object
>
>   (foo->file-like (foo (text "hello")))
>
>   ;;; with gexp compiler
>
>   (define-gexp-compiler (foo-compiler (foo <foo-record>) system target)
>     ;;
>     (lower-object
>      (plain-file "foo"
>        (foo-text foo))))
>
>   ;; now, a ``foo'' can be treated as a lowerable file-like object! you
>   ;; can put it anywhere you'd put a file-like.
>   (foo (text "hello"))
>
> So, basically, define-gexp-compiler lets you make new kinds of file-like
> object from records! This is actually how computed-file, file-append, et
> al are defined; see guix/gexp.scm. (Many of the gexp-compilers define both
> a compiler and an expander; the compiler is a derivation to build when
> the object is built, and the expander is the string to return when it's
> gexped. file-append [line 680 in my checkout] is a good, clear example of
> this.)
>
>     -- (

Thank you for your kindness.
I think I understand a little more now, and I will read the surrounding
source code to better understand it. Thanks to you, I may be able to
understand what was suggested earlier in this thread!

Thanks,
diff mbox series

Patch

diff --git a/gnu/home/services/fontutils.scm b/gnu/home/services/fontutils.scm
index 6062eaed6a..4b3caf3985 100644
--- a/gnu/home/services/fontutils.scm
+++ b/gnu/home/services/fontutils.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2022 Taiju HIGASHI <higashi@taiju.info>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -20,9 +21,17 @@ 
 (define-module (gnu home services fontutils)
   #:use-module (gnu home services)
   #:use-module (gnu packages fontutils)
+  #:use-module (gnu services configuration)
+  #:use-module (guix diagnostics)
   #:use-module (guix gexp)
+  #:use-module (guix i18n)
+  #:use-module (guix records)
+  #:use-module (srfi srfi-1)
+  #:use-module (sxml simple)
+  #:use-module (ice-9 match)
 
-  #:export (home-fontconfig-service-type))
+  #:export (home-fontconfig-service-type
+            home-fontconfig-configuration))
 
 ;;; Commentary:
 ;;;
@@ -33,15 +42,83 @@  (define-module (gnu home services fontutils)
 ;;;
 ;;; Code:
 
-(define (add-fontconfig-config-file he-symlink-path)
+(define (sxml->xml-string sxml)
+  "Serialize the sxml tree @var{tree} as XML. The output will be string."
+  (call-with-output-string
+    (lambda (port)
+      (sxml->xml sxml port))))
+
+(define guix-home-font-dir "~/.guix-home/profile/share/fonts")
+
+(define (string-list? value)
+  (and (pair? value) (every string? value)))
+
+(define (serialize-string-list field-name value)
+  (sxml->xml-string
+   (map
+    (lambda (path) `(dir ,path))
+    (if (member guix-home-font-dir value)
+        value
+        (append (list guix-home-font-dir) value)))))
+
+(define (serialize-string field-name value)
+  (define (serialize type value)
+    (sxml->xml-string
+     `(alias
+       (family ,type)
+       (prefer
+        (family ,value)))))
+  (match (list field-name value)
+    (('default-font-serif-family family)
+     (serialize 'serif family))
+    (('default-font-sans-serif-family family)
+     (serialize 'sans-serif family))
+    (('default-font-monospace-family family)
+     (serialize 'monospace family))))
+
+(define-maybe string)
+
+(define extra-config-list? list?)
+
+(define-maybe extra-config-list)
+
+(define (serialize-extra-config-list field-name value)
+  (sxml->xml-string
+   (map (match-lambda
+          ((? pair? sxml) sxml)
+          ((? string? xml) (xml->sxml xml))
+          (else
+           (raise (formatted-message
+                   (G_ "'extra-config' type must be xml string or sxml list, was given: ~a")
+                   value))))
+        value)))
+
+(define-configuration home-fontconfig-configuration
+  (font-directories
+   (string-list (list guix-home-font-dir))
+   "The directory list that provides fonts.")
+  (default-font-serif-family
+    maybe-string
+    "The preffered default fonts of serif.")
+  (default-font-sans-serif-family
+    maybe-string
+    "The preffered default fonts of sans-serif.")
+  (default-font-monospace-family
+    maybe-string
+    "The preffered default fonts of monospace.")
+  (extra-config
+   maybe-extra-config-list
+   "Extra configuration values to append to the fonts.conf."))
+
+(define (add-fontconfig-config-file user-config)
   `(("fontconfig/fonts.conf"
      ,(mixed-text-file
        "fonts.conf"
        "<?xml version='1.0'?>
 <!DOCTYPE fontconfig SYSTEM 'fonts.dtd'>
-<fontconfig>
-  <dir>~/.guix-home/profile/share/fonts</dir>
-</fontconfig>"))))
+<fontconfig>"
+       (serialize-configuration user-config home-fontconfig-configuration-fields)
+       "</fontconfig>\n"))))
 
 (define (regenerate-font-cache-gexp _)
   `(("profile/share/fonts"
@@ -59,7 +136,7 @@  (define home-fontconfig-service-type
                        (service-extension
                         home-profile-service-type
                         (const (list fontconfig)))))
-                (default-value #f)
+                (default-value (home-fontconfig-configuration))
                 (description
                  "Provides configuration file for fontconfig and make
 fc-* utilities aware of font packages installed in Guix Home's profile.")))