[bug#62101] [PATCH] home: services: Add xmodmap.

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

Hi,

conses <contact@conses.eu> skribis:

> * gnu/home/services/desktop.scm (home-xmodmap-service-type)
> (home-xmodmap-configuration): New variables;
> (xmodmap-shepherd-service)
> (get-xmodmap-configuration)
> (get-xmodmap-file)
> (add-xmodmap-config-file)
> (add-xmodmap-package): New procedures;
> * doc/guix.texi (Desktop Services): Document it.

Overall LGTM, with minor issues:

> +The syntax for the expression grammar is quite straightforward.  You can
> +either provide a list of cons cells and strings like this:

I’d suggest avoiding the first sentence, because what looks
straightforward to someone might be intimidating to another.  We also
avoid jargon like “cons cell” in the manual.

What about something like this:

  The @code{key-map} field takes a list of objects, each of which is
  either a @dfn{statement} (a string) or an @dfn{assignment} (a pair of
  strings).  As an example, the snippet below configures the @kbd{mod4}
  key (???) such that it does XYZ, FIXME: finish sentence :-)

… where ‘key-map’ is IMO a better name for ‘config’.

> +Alternatively, there is a more Lisp-like configuration syntax via Scheme
> +symbols, lists, and vectors, that you can use like this:
> +
> +@lisp
> +(service home-xmodmap-service-type
> +         (home-xmodmap-configuration
> +          (config '((#(add mod4) . Print)
> +                    (clear lock)

I don’t find it very useful; I’d rather support only one syntax, but
clearly explained.  So my suggestion would be to drop this.

> +@item @code{config} (default: @code{config}) (type: list)

So this would be renamed to @code{key-map} maybe.

> +(define (get-xmodmap-configuration field-name val)

As a rule of thumb, you can drop ‘get-’ from procedure names; procedures
like this one are rarely called ‘get-SOMETHING’.

> +     (service-extension home-profile-service-type
> +                        add-xmodmap-package)

I believe this extension is unnecessary.

Could you send an updated patch?

Thank you!

Ludo’.