[RFC] Cosmetic changes to define-configuration usage

[RFC] Cosmetic changes to define-configuration usage

[Bruno Victal]

Forwarded from: <https://logs.guix.gnu.org/guix/2023-03-22.log#165538> & [1]

--8<---------------cut here---------------start------------->8---
<mirai>apteryx: IMO the spacing between the fields should have been kept
<mirai>it makes things easier to read
<mirai>it's a nightmare if the records grow very large
<apteryx>mirai: I was on the fence about it, but keeping the fields together in the same record appears to be the more conventional style in the code base
<apteryx>(together as in without blank lines in-between)
<mirai>apteryx: I'm planning to gradually shift the define-configurations to have a space between fields
<apteryx>mirai: it should be discussed first to guix-devel :-)
--8<---------------cut here---------------end--------------->8---


I'd like to propose for field specifications in define-configuration to be separated with a
blank line between them. Reason for this is that it makes it much easier on the eyes
to read, rather than being presented with a dense hunk of text to sift through.

I tend to always insert these blank lines when making changes in these parts to aid reading,
even if they weren't originally present and were not planned to be committed. I'd be happy if
I could simply keep the line separations and avoid the tedious insert-erase ritual.

I think I'm not alone in this opinion, consider the following snippets:


With a line separating each field:   (gnu/services/mcron.scm)
--8<---------------cut here---------------start------------->8---
(define-configuration/no-serialization mcron-configuration
  (mcron
   (file-like mcron)
   "The mcron package to use.")

  (jobs
   (list-of-gexps '())
   "This is a list of gexps (@pxref{G-Expressions}), where each gexp
corresponds to an mcron job specification (@pxref{Syntax, mcron job
specifications,, mcron, GNU@tie{}mcron}).")

  (log?
   (boolean #t)
   "Log messages to standard output.")

  (log-file
   (string "/var/log/mcron.log")
   "Log file location.")

  (log-format
   (string "~1@*~a ~a: ~a~%")
   "@code{(ice-9 format)} format string for log messages.  The default value
produces messages like @samp{@var{pid} @var{name}: @var{message}}
(@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}).
Each message is also prefixed by a timestamp by GNU Shepherd.")

  (date-format
   maybe-string
   "@code{(srfi srfi-19)} format string for date."))
--8<---------------cut here---------------end--------------->8---


Lines collapsed:   (gnu/services/linux.scm)
--8<---------------cut here---------------start------------->8---
(define-configuration fstrim-configuration
  (package
    (file-like util-linux)
    "The package providing the @command{fstrim} command."
    empty-serializer)
  (schedule
   (mcron-time "0 0 * * 0")
   "Schedule for launching @command{fstrim}.  This can be a procedure, a list
or a string.  For additional information, see @ref{Guile Syntax,,
Job specification, mcron, the mcron manual}.  By default this is set to run
weekly on Sunday at 00:00."
   empty-serializer)
  ;; The following are fstrim-related options.
  (listed-in
   (maybe-list-of-strings '("/etc/fstab" "/proc/self/mountinfo"))
   ;; Note: documentation sourced from the fstrim manpage.
   "List of files in fstab or kernel mountinfo format.  All missing or
empty files are silently ignored.  The evaluation of the list @emph{stops}
after the first non-empty file.  File systems with @code{X-fstrim.notrim} mount
option in fstab are skipped.")
  (verbose?
   (boolean #t)
   "Verbose execution.")
  (quiet-unsupported?
   (boolean #t)
   "Suppress error messages if trim operation (ioctl) is unsupported.")
  (extra-arguments
   maybe-list-of-strings
   "Extra options to append to @command{fstrim} (run @samp{man fstrim} for
more information)."
   (lambda (_ value)
     (if (maybe-value-set? value)
         value '())))
--8<---------------cut here---------------end--------------->8---


IMO, the top snippet looks much less overwhelming than the bottom one.


## “Implementation” plan

This cosmetic change can be done gradually or with a commit that performs the restyling in bulk.


[1]: <https://issues.guix.gnu.org/61964>


Cheers,
Bruno

Re: [RFC] Cosmetic changes to define-configuration usage

[Katherine Cox-Buday]

On 3/24/23 6:33 AM, Bruno Victal wrote:

> I'd like to propose for field specifications in define-configuration to be separated with a
> blank line between them. Reason for this is that it makes it much easier on the eyes
> to read, rather than being presented with a dense hunk of text to sift through.
> 
> I tend to always insert these blank lines when making changes in these parts to aid reading,
> even if they weren't originally present and were not planned to be committed. I'd be happy if
> I could simply keep the line separations and avoid the tedious insert-erase ritual.
> 
> I think I'm not alone in this opinion, consider the following snippets:

It's funny how sometimes this works out. I was just hacking on a 
service's configuration and had the same thought. I had spaced 
everything out so that it wasn't as overwhelming, and with a sigh 
grouped it all back together because that's how all existing services work.

I would definitely appreciate this change in the style standard.

--
Katherine

Re: [RFC] Cosmetic changes to define-configuration usage

[Maxim Cournoyer]

Hello,

Bruno Victal <mirai@makinata.eu> writes:

> Forwarded from: <https://logs.guix.gnu.org/guix/2023-03-22.log#165538> & [1]
>
> <mirai>apteryx: IMO the spacing between the fields should have been kept
> <mirai>it makes things easier to read
> <mirai>it's a nightmare if the records grow very large
> <apteryx>mirai: I was on the fence about it, but keeping the fields together in the same record appears to be the more conventional style in the code base
> <apteryx>(together as in without blank lines in-between)
> <mirai>apteryx: I'm planning to gradually shift the define-configurations to have a space between fields
> <apteryx>mirai: it should be discussed first to guix-devel :-)
>
>
>
> I'd like to propose for field specifications in define-configuration to be separated with a
> blank line between them. Reason for this is that it makes it much easier on the eyes
> to read, rather than being presented with a dense hunk of text to sift through.
>
> I tend to always insert these blank lines when making changes in these parts to aid reading,
> even if they weren't originally present and were not planned to be committed. I'd be happy if
> I could simply keep the line separations and avoid the tedious insert-erase ritual.
>
> I think I'm not alone in this opinion, consider the following snippets:
>
>
> With a line separating each field:   (gnu/services/mcron.scm)
>
> (define-configuration/no-serialization mcron-configuration
>   (mcron
>    (file-like mcron)
>    "The mcron package to use.")
>
>   (jobs
>    (list-of-gexps '())
>    "This is a list of gexps (@pxref{G-Expressions}), where each gexp
> corresponds to an mcron job specification (@pxref{Syntax, mcron job
> specifications,, mcron, GNU@tie{}mcron}).")
>
>   (log?
>    (boolean #t)
>    "Log messages to standard output.")
>
>   (log-file
>    (string "/var/log/mcron.log")
>    "Log file location.")
>
>   (log-format
>    (string "~1@*~a ~a: ~a~%")
>    "@code{(ice-9 format)} format string for log messages.  The default value
> produces messages like @samp{@var{pid} @var{name}: @var{message}}
> (@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}).
> Each message is also prefixed by a timestamp by GNU Shepherd.")
>
>   (date-format
>    maybe-string
>    "@code{(srfi srfi-19)} format string for date."))
>
>
>
> Lines collapsed:   (gnu/services/linux.scm)
>
> (define-configuration fstrim-configuration
>   (package
>     (file-like util-linux)
>     "The package providing the @command{fstrim} command."
>     empty-serializer)
>   (schedule
>    (mcron-time "0 0 * * 0")
>    "Schedule for launching @command{fstrim}.  This can be a procedure, a list
> or a string.  For additional information, see @ref{Guile Syntax,,
> Job specification, mcron, the mcron manual}.  By default this is set to run
> weekly on Sunday at 00:00."
>    empty-serializer)
>   ;; The following are fstrim-related options.
>   (listed-in
>    (maybe-list-of-strings '("/etc/fstab" "/proc/self/mountinfo"))
>    ;; Note: documentation sourced from the fstrim manpage.
>    "List of files in fstab or kernel mountinfo format.  All missing or
> empty files are silently ignored.  The evaluation of the list @emph{stops}
> after the first non-empty file.  File systems with @code{X-fstrim.notrim} mount
> option in fstab are skipped.")
>   (verbose?
>    (boolean #t)
>    "Verbose execution.")
>   (quiet-unsupported?
>    (boolean #t)
>    "Suppress error messages if trim operation (ioctl) is unsupported.")
>   (extra-arguments
>    maybe-list-of-strings
>    "Extra options to append to @command{fstrim} (run @samp{man fstrim} for
> more information)."
>    (lambda (_ value)
>      (if (maybe-value-set? value)
>          value '())))

I have some apprehension that if we start adding white space between the
fields here, we'll soon have people adding white space to many other
places (for consistency or other reasons), which I wouldn't welcome (I
value compactness, and since in Scheme a single newline is used to
delimit things at the top level, too much of white space can make things
less readable in my opinion).

-- 
Thanks,
Maxim

Re: [RFC] Cosmetic changes to define-configuration usage

[Bruno Victal]

On 2023-03-31 15:46, Maxim Cournoyer wrote:> I have some apprehension that if we start adding white space between the
> fields here, we'll soon have people adding white space to many other
> places (for consistency or other reasons), which I wouldn't welcome (I
> value compactness, and since in Scheme a single newline is used to
> delimit things at the top level, too much of white space can make things
> less readable in my opinion).

I don't think it needs to be an all-or-nothing situation,
the spacing rules can be always applied selectively “when it makes sense”.

I think spaces between fields is consistent with the general way of things,
for instance, throughout Guix, sections that are only scheme code often do
have some spaces here and there that were added without any adherence to some
rigid criteria but the programmer found it to be an adequate point to 
partition the logic.

The same reasoning applies here, the logic partitioning is done per field instead.
Objectively, there's also a small quantitative difference that's not commonly
present in the rest of the codebase. define-configuration handles both code and documentation,
or putting it another way, it intersperses code and (rather long) strings. The result
is that it's particularly information-dense compared to any other part of the guix codebase.


My 2¢!


Cheers,
Bruno

Re: [RFC] Cosmetic changes to define-configuration usage

[Maxim Cournoyer]

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-03-31 15:46, Maxim Cournoyer wrote:> I have some apprehension
> that if we start adding white space between the
>> fields here, we'll soon have people adding white space to many other
>> places (for consistency or other reasons), which I wouldn't welcome (I
>> value compactness, and since in Scheme a single newline is used to
>> delimit things at the top level, too much of white space can make things
>> less readable in my opinion).
>
> I don't think it needs to be an all-or-nothing situation,
> the spacing rules can be always applied selectively “when it makes sense”.
>
> I think spaces between fields is consistent with the general way of things,
> for instance, throughout Guix, sections that are only scheme code often do
> have some spaces here and there that were added without any adherence to some
> rigid criteria but the programmer found it to be an adequate point to 
> partition the logic.
>
> The same reasoning applies here, the logic partitioning is done per field instead.
> Objectively, there's also a small quantitative difference that's not commonly
> present in the rest of the codebase. define-configuration handles both code and documentation,
> or putting it another way, it intersperses code and (rather long) strings. The result
> is that it's particularly information-dense compared to any other part of the guix codebase.

You make convincing arguments.  If it's understood that it makes sense
in this specific case and not to be taken as "let's add white space to
all the records in the Guix code base", I don't oppose the change, if
people find it makes a difference for the better in terms of readability.

Are there people who would like to block the change?  Otherwise, I'll
contribute it as-is in a few days.

-- 
Thanks,
Maxim