On 2023-01-14 19:00, Ludovic Courtès wrote:

> Hello!
>
> David Wilson <david@daviwil.com> skribis:
>
>> Yes, I saw that you meant to run Emacs as a daemon and I think it should
>> be added to this service (or another daemon-specific service) in a
>> future patch.  I figured it would be easier to get a patch accepted
>> without the daemon functionality just yet since there seemed to be more
>> feedback about that aspect in your patch thread.
>
> Agree, I think we should add the daemon functionality in a subsequent
> patch.
>
> Overall it looks nice to me!  Some comments and suggestions:
>
>> +@node Emacs Home Services
>> +@subsection Emacs Home Services
>> +
>> +@defvr {Scheme Variable} home-emacs-service-type
>
> It would be nice if you could start the section with a few sentences
> explaining the rationale and what’s being described here.
>
>> +This is the service type for configuring the Emacs text editor.  It
>> +enables you to assemble @file{init.el} and @file{early-init.el} files
>> +from snippets in your home configuration and other Emacs Lisp files you
>> +have in your personal configuration folder.
>
> Maybe like so:
>
>   … to assemble the @file{init.el} (@pxref{Init File,,, emacs, GNU Emacs
>   Manual}) and @file{early-init.el} (@pxref{Early Init File,,, emacs,
>   GNU Emacs Manual}) files …
>
>> +@example
>> +$ cp ~/.emacs.d $XDG_CONFIG_HOME/emacs
>> +$ cp ~/.emacs $XDG_CONFIG_HOME/emacs/init.el
>> +@end example
>
> I’d drop the prompt.
>
>> +@item @code{init-file} (default: @code{'()})
>> +Configuration text or files to include in @file{init.el}.
>> +
>> +@item @code{early-init-file} (default: @code{'()})
>> +Configuration text or files to include in @file{early-init.el}.
>
> What about accepting sexps (or gexps) instead of strings?  As in:
>
>   (init-file '((require 'whatever) (setq something t)))

A quick minor note on this approach: it won't be possible to use
#'elisp-function inside such configuration because it will be
interpreted by guile reader, but actually rde lives without this
functionality completely ok.

Do we want something like this possible?

(init-file `((require 'whatever)
             (setq something t)
             (load ,(local-file "old-init.el")))

>
> Also I find it confusing that it’s either text or files.  In the code it
> has type ‘text-config’, which means list of file-like objects IIUC, no?
>
>> +@item @code{load-paths} (default: @code{'()})
>> +Additional load paths to add to Emacs' @code{load-path} variable.  Lines
>> +will be inserted at the beginning of @file{early-init.el}.
>
> Nitpick: I think this should be ‘load-path’ (singular), because it’s one
> search path (i.e., a list of directories).
>
> And: s/Additional load paths/Additional directories/
>
>> +@end table
>> +@end deftp
>
> Would be nice to have a couple of commented examples here, like you had
> in the first message in this thread.  :-)
>
>> +@deftp {Data Type} home-emacs-extension
>> +The extension record for @code{home-emacs-service-type}.
>
> Would be nice to have a sentence above, like “This service type can be
> extended with @code{home-emacs-extension} records, described below:”.
>
>> +@table @asis
>> +@item @code{packages} (default: @code{'()})
>> +Additional packages required by the Emacs configuration.
>> +
>> +@item @code{init-file} (default: @code{'()})
>> +Configuration text or files to include in @file{init.el}.
>> +
>> +@item @code{early-init-file} (default: @code{'()})
>> +Configuration text or files to include in @file{early-init.el}.
>> +
>> +@item @code{load-paths} (default: @code{'()})
>
> Ditto.
>
>> +@end deftp
>
> Would be great to have an example of that too.  :-)
>
> Thank you!
>
> Ludo’.
>
>
>

-- 
Best regards,
Andrew Tropin