[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig con
From: |
Andrew Tropin |
Subject: |
[bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration. |
Date: |
Wed, 12 Oct 2022 10:43:34 +0400 |
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
--
Best regards,
Andrew Tropin
signature.asc
Description: PGP signature
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., (continued)
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Liliana Marie Prikler, 2022/10/11
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/11
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Liliana Marie Prikler, 2022/10/11
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/12
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Liliana Marie Prikler, 2022/10/12
- [bug#57963] Almost plain SXML serializer, Andrew Tropin, 2022/10/12
- [bug#57963] Almost plain SXML serializer, Taiju HIGASHI, 2022/10/12
- [bug#57963] Almost plain SXML serializer, Andrew Tropin, 2022/10/12
- [bug#57963] Almost plain SXML serializer, Liliana Marie Prikler, 2022/10/12
- [bug#57963] Almost plain SXML serializer, Andrew Tropin, 2022/10/12
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration.,
Andrew Tropin <=
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/12
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Andrew Tropin, 2022/10/12
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Ludovic Courtès, 2022/10/13
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Andrew Tropin, 2022/10/14
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/15
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Ludovic Courtès, 2022/10/17
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/18
- [bug#57963] [PATCH v5 2/2] home: services: Support user's fontconfig configuration., Taiju HIGASHI, 2022/10/19
- [bug#57963] [PATCH 0/1] Support user's fontconfig., Declan Tsien, 2022/10/20
- [bug#57963] [PATCH 0/1] Support user's fontconfig., Taiju HIGASHI, 2022/10/20