Re: [PATCH 2/5] services: Factorize define-maybe macro.

[Clément Lassieur]

Mathieu Othacehe <address@hidden> writes:

> * gnu/services/configuration.scm (define-maybe): New exported
>   procedure.
>   (id): New procedure extracted from define-configuration.
> * gnu/services/messaging.scm (define-all-configurations): Define id
>   inside procedure as define-all-configurations is now the only user.
> ---
>  gnu/services/configuration.scm | 28 ++++++++++++++++++++--------
>  gnu/services/messaging.scm     | 22 +++-------------------
>  2 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index 2ad3a637a..3fdaf705a 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2015 Andy Wingo <address@hidden>
> +;;; Copyright © 2017 Mathieu Othacehe <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -36,6 +37,7 @@
>              configuration-field-default-value-thunk
>              configuration-field-documentation
>              serialize-configuration
> +            define-maybe
>              define-configuration
>              validate-configuration
>              generate-documentation
> @@ -85,16 +87,26 @@
>                     (configuration-field-name field) val))))
>              fields))
>  
> +(define-syntax-rule (id ctx parts ...)
> +      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))

The docstring was helpful!  Could you put it too?

> +(define-syntax define-maybe
> +  (lambda (x)
> +    (syntax-case x ()
> +      ((_ stem)
> +       (with-syntax
> +           ((stem?                (id #'stem #'stem #'?))
> +            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
> +            (serialize-stem       (id #'stem #'serialize- #'stem))
> +            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
> +         #'(begin
> +             (define (maybe-stem? val)
> +               (or (eq? val 'disabled) (stem? val)))
> +             (define (serialize-maybe-stem field-name val)
> +               (when (stem? val) (serialize-stem field-name val)))))))))
> +
>  (define-syntax define-configuration
>    (lambda (stx)
> -    (define (id ctx part . parts)
> -      (let ((part (syntax->datum part)))
> -        (datum->syntax
> -         ctx
> -         (match parts
> -           (() part)
> -           (parts (symbol-append part
> -                                 (syntax->datum (apply id ctx parts))))))))

I think this part, that replaces the old version of "id" with the new
one deserves a specific patch, because it is not directly related to
moving define-maybe, and because it does more that just "moving code".

>      (syntax-case stx ()
>        ((_ stem (field (field-type def) doc) ...)
>         (with-syntax (((field-getter ...)
> diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
> index 34723dc11..e50eeba8c 100644
> --- a/gnu/services/messaging.scm
> +++ b/gnu/services/messaging.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2017 Clément Lassieur <address@hidden>
> +;;; Copyright © 2017 Mathieu Othacehe <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -49,27 +50,10 @@
>  ;;;
>  ;;; Code:
>  
> -(define-syntax-rule (id ctx parts ...)
> -  "Assemble PARTS into a raw (unhygienic) identifier."
> -  (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))
> -
> -(define-syntax define-maybe
> -  (lambda (x)
> -    (syntax-case x ()
> -      ((_ stem)
> -       (with-syntax
> -           ((stem?                (id #'stem #'stem #'?))
> -            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
> -            (serialize-stem       (id #'stem #'serialize- #'stem))
> -            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
> -         #'(begin
> -             (define (maybe-stem? val)
> -               (or (eq? val 'disabled) (stem? val)))
> -             (define (serialize-maybe-stem field-name val)
> -               (when (stem? val) (serialize-stem field-name val)))))))))
> -

>  (define-syntax define-all-configurations
>    (lambda (stx)
> +    (define-syntax-rule (id ctx parts ...)
> +      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))

This could go in the second patch as well.

>      (define (make-pred arg)
>        (lambda (field target)
>          (and (memq (syntax->datum target) `(common ,arg)) field)))