GNU bug report logs - #75959
[PATCH] (home-)syncthing-service: added support for config serialization

Previous Next

Full log

Message #73 received at 75959 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Zacchaeus Scheffer <eikcaz <at> zacchae.us>
Cc: Bruno Victal <mirai <at> makinata.eu>, 75959 <at> debbugs.gnu.org,
 Leo Famulari <leo <at> famulari.name>
Subject: Re: [bug#75959] [PATCH v10] services: syncthing: Add support for
 config file generation.
Date: Fri, 21 Feb 2025 12:05:58 +0100

Hi,

Zacchaeus Scheffer <eikcaz <at> zacchae.us> skribis:

> I believe that is an accurate term:
> https://en.wikipedia.org/wiki/Naming_convention_(programming)#Delimiter-separated_words

Woow, TIL.  :-)

> Sure thing.  I assume I can send it to the same issue even though it is
> closed?

Yes, but maybe open a new issue, for clarity.

>> As for the value itself, it would help a lot to use #t and #f instead of
>> the strings "true" and "false" (both of which have truth value in
>> Scheme).  It leads to a bit of extra work in the serializer, but I think
>> it’s worth it.  Because then we can also have type-checking of fields.
>
> This would be easiest to implement as a sanitizer as in:
>
> (define (bool-string bool)
>   (if bool "true" "false"))
> (define-record-type* <syncthing-config-file>
> ...
>   (gui-enabled <accessor> (default "true")
>                           (sanitizer bool-string))
> ...
>
> But then the guix docs would technically be wrong reporting a default
> value of #t instead of the actual default value of "true".  I think this
> would yield a more helpful type-checking error though, so I'll go this
> direction unless you think otherwise.

I would advise against using sanitizer in this way because it would lead
to inconsistencies: users would provide a boolean, but they’d get a
string when calling the accessor.

Instead the record should contain records of the “right” type;
converting to the relevant string should be left to whatever serializes
the record to the config file.

(This is something ‘define-configuration’ really helps with; I think
it’s worth looking into it!)

>>> +@item @code{gui-tls} (default: @var{"false"})
>>> +@item @code{gui-debugging} (default: @var{"false"})
>>> +@item @code{gui-send-basic-auth-prompt} (default: @var{"false"})
>>> +@item @code{gui-address} (default: @var{"127.0.0.1:8384"})
>>> +@item @code{gui-user} (default: @var{#f})
>>> +@item @code{gui-password} (default: @var{#f})
>>> +A bcrypt hash of the GUI password.  Remember that this will be globally
>>> +exposed in @file{/gnu/store}.
>>
>> I believe you want @itemx for all but the first item.
>
> Does it matter that the describing text only applies to gui-password,
> not to the whole block?

It does yes, so my suggestion is actually bogus, sorry!

Normally each field would be documented, not just ‘gui-password’.

> My goal was to, as much as possible, avoid needing to maintain Syncthing
> documentation in Guix.  My original patch even had camelcase for record
> names so the user could easily search the Syncthing documentation
> knowing what keyword to search.  Since you can't directly search anymore
> anyway, maybe it does make sense to expand out some names.  I'll do that
> for these:
>
> auth - authorization
> s - seconds
> m - minutes
> h - hours
> fs - file-system
> pct - percentage
> mcaddr - mac-address
> recv - recieve
> ur - usage-reporting

Sounds good to me.

Thanks!

Ludo’.

Previous Next

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