[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#64620] [PATCH] gnu: home: Add home-emacs-service-type.
From: |
Ludovic Courtès |
Subject: |
[bug#64620] [PATCH] gnu: home: Add home-emacs-service-type. |
Date: |
Wed, 11 Oct 2023 18:48:59 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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