[bug#64620] [PATCH] gnu: home: Add home-emacs-service-type.

["Ludovic Courtès" <ludo@gnu.org>]

So, here is a short review of the parts I’m most familiar with.

fernseed@fernseed.me skribis:


[...]

> +(define-module (gnu home services emacs)
> +  #:use-module (gnu home services)

[...]

> +  #:re-export (blank?
> +
> +               vertical-space
> +               vertical-space?

Why re-export these things from here?  Sounds surprising because we’re
in a service module.


[...]

> +(define* (elisp->file-builder exps #:key (special-forms '()))
> +  "Return a G-expression that builds a file containing the Elisp
> +expressions (<elisp> objects or s-expressions) or G-epxressions in list EXPS.
> +See `elisp-file' for a description of SPECIAL-FORMS."

[...]

> +  (with-imported-modules (source-module-closure
> +                          '((guix read-print)))
> +    (gexp (begin
> +            (use-modules (guix read-print))
> +            (call-with-output-file (ungexp output "out")
> +              (lambda (port)

For clarity/conciseness, you can use #~ and #$ in this code.

> +(define-gexp-compiler (elisp-file-compiler (elisp-file <elisp-file>)
> +                                           system target)
> +  (match-record elisp-file <elisp-file>
> +                (name gexp)
> +    (with-monad %store-monad
> +      (gexp->derivation name gexp
> +                        #:system system
> +                        #:target target
> +                        #:local-build? #t
> +                        #:substitutable? #f))))
> +
> +(define* (elisp-file* name exps #:key (special-forms '()))
> +  "Return as a monadic value a derivation that builds an Elisp file named NAME
> +containing the expressions in EXPS, a list of Elisp expression objects or
> +G-expressions.
> +
> +This is the monadic counterpart of `elisp-file', which see for a description
> +of SPECIAL-FORMS,"
> +  (define builder
> +    (elisp->file-builder exps
> +                         #:special-forms special-forms))
> +
> +  (gexp->derivation name builder
> +                    #:local-build? #t
> +                    #:substitutable? #f))

I think you don’t need to fiddle with the monadic interface.  I’d
suggest removing the <elisp-file> type and gexp compiler and instead
defining ‘elisp-file’ in terms of ‘computed-file’.  WDYT?

> +(define (record-value rec field)
> +  "Return the value of field named FIELD in record REC."
> +  ((record-accessor (record-type-descriptor rec) field) rec))
> +
> +(define-syntax extend-record
> +  ;; Extend record ORIGINAL by creating a new copy using CONSTRUCTOR,
> +  ;; replacing each field specified by ORIG-FIELD with the evaluation of (PROC
> +  ;; ORIG-VAL EXT-VALS), where ORIG-VAL is the value of ORIG-FIELD in ORIGINAL
> +  ;; and EXT-VALS is the list of values of EXT-FIELD in EXTENSIONS.
> +  (lambda (s)
> +    (syntax-case s ()
> +      ((_ constructor original extensions (proc orig-field ext-field) ...)
> +       (with-syntax (((field-specs ...)
> +                      (map
> +                       (lambda (spec)
> +                         (syntax-case spec ()
> +                           ((proc orig-field ext-field)
> +                            #'(orig-field
> +                               (apply
> +                                proc
> +                                (list
> +                                 (record-value original 'orig-field)

I would advice against accessing record fields by name, with run-time
field name resolution.

The spirit of records, unlike alists, is that there’s a
statically-defined mapping of fields to their offsets in the struct;
without having access to record accessors, you’re not supposed to be
able to access the record (I know Guile has ‘struct-ref’,
‘record-accessor’, etc., but these are abstraction-breaking primitives
that should be avoided IMO).

How could this code be adjusted accordingly?  I guess you’re looking for
a way to iterate over fields?

> +;;; Elisp reader extension.
> +;;;
> +
> +(eval-when (expand load eval)
> +
> +  (define (read-elisp-extended port)
> +    (read-with-comments port
> +                        #:blank-line? #f
> +                        #:elisp? #t
> +                        #:unelisp-extensions? #t))
> +
> +  (define (read-elisp-expression chr port)
> +    `(elisp ,(read-elisp-extended port)))
> +
> +  (read-hash-extend #\% read-elisp-expression))

I’d lean towards not having a reader extension because they don’t
compose and it’s easy to end up colliding with another, unrelated
extension.  I think it’s okay if people write:

  (elisp …)

rather than:

  #%(…)

It’s also arguably easier to understand for a newcomer.

> +++ b/guix/read-print.scm

This part is the most “problematic” for me: I’m already dissatisfied
with the current state of things (the pretty-printer in particular is
too complex and hard to work with), and this change brings more
complexity and lacks orthogonality.

What I’d like to see, ideally, is a clear separation between elisp
concerns and Scheme concerns in the reader and in the pretty printer.

Probably, a preliminary step (I could look into it) would be to rewrite
the pretty printer based on Wadler’s “prettier printer” paper and/or
Shinn’s formatting combinators¹.

WDYT?

Thanks,
Ludo’.

¹ https://srfi.schemers.org/srfi-159/srfi-159.html