GNU bug report logs - #62101
[PATCH] home: services: Add xmodmap.

Previous Next

Full log

Message #8 received at 62101 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: conses <contact <at> conses.eu>
Cc: Andrew Tropin <andrew <at> trop.in>, 62101 <at> debbugs.gnu.org
Subject: Re: bug#62101: [PATCH] home: services: Add xmodmap.
Date: Mon, 13 Mar 2023 15:00:17 +0100

Hello,

This looks like a useful addition!

Some comments below; maybe Andrew will bring a different perspective.

conses <contact <at> conses.eu> skribis:

> ---
>  gnu/home/services/desktop.scm | 109 ++++++++++++++++++++++++++++++++++

It’d be great if you could provide a ChangeLog-style commit log (you can
check the manual and ‘git log’ to see what it’s like); if you can’t, we
can help with that.

More importantly, could you add documentation to guix.texi, similar to
what is done for the other Home services?  That is: a few introductory
lines, an example, and the reference (data types, variables,
procedures).

> +   (alist '())
> +   "Association list of key and value pairs for the
> + @code{xmodmap} configuration file.  Its syntax can take something like
> +the following:
> +
> +@example
> +'((#(add mod4) . Print)
> +    (clear lock)
> +    (clear control)
> +    (#(keycode 66) . Control_L)
> +    (#(add control) . #(Control_L Control_R)))
> +@end example"))

I don’t quite get the syntax.

Use of vectors is unusual in this context, I’d recommend against it.

Regarding pairs: in some cases, the cdr is a symbol/vector, in other
cases it’s a one-element list (as in ‘(clear lock)’).  That also makes
it a bit confusing to me.

Perhaps we should try to make it look more consistent?  WDYT?

> +(define (home-xmodmap-shepherd-service config)
> +  (list
> +   (shepherd-service
> +    (provision '(xmodmap))
> +    (start #~(make-system-constructor
> +              (string-join
> +               (list #$(file-append
> +                        (home-xmodmap-configuration-xmodmap config)
> +                        "/bin/xmodmap")
> +                     #$(home-xmodmap-file config)))))
> +    (one-shot? #t))))

Perhaps it’d be useful to have a ‘stop’ procedure that undoes that
changes?  In which case it wouldn’t be one-shot anymore.

> +  (define serialize-field
> +    (match-lambda
> +      ((? list? e)
> +       (string-join
> +        (map
> +         (lambda (x)
> +           (serialize-term x))
> +         e)
> +        " "))

Just: (string-join (map serialize-term e)).

> +(define home-xmodmap-service-type
> +  (service-type
> +   (name 'home-xmodmap)
> +   (extensions
> +    (list
> +     (service-extension
> +      home-profile-service-type
> +      home-xmodmap-profile-service)
> +     (service-extension
> +      home-xdg-configuration-files-service-type
> +      home-xmodmap-files-service)
> +     (service-extension
> +      home-shepherd-service-type
> +      home-xmodmap-shepherd-service)))
> +   (description "Configure @code{xmodmap} bindings and rules.")

Please expand the description a bit.

TIA!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.