diff mbox series

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

Message ID 867cvo9y6b.fsf@conses.eu
State New
Headers show
Series [bug#62101] home: services: Add xmodmap. | expand

Commit Message

Miguel Ángel Moreno March 10, 2023, 7:57 p.m. UTC
---
 gnu/home/services/desktop.scm | 109 ++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

Comments

Ludovic Courtès March 13, 2023, 2 p.m. UTC | #1
Hello,

This looks like a useful addition!

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

conses <contact@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’.
Miguel Ángel Moreno March 16, 2023, 1:15 p.m. UTC | #2
Ludovic Courtès <ludo@gnu.org> writes:

> 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.
>

Apologies for missing this, I added it in v2.

> 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).
>

Likewise.

>> +   (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?

I've amended the type of config to be of list instead.  As I've hinted
in the manual, the motivation for including vectors and lists is to
provide a more Lisp-like configuration syntax for those that want it, it
can simply take strings too.  IMO there's no point in making it
consistent since the original syntax isn't consistent in the first
place, you can check out some examples here
<https://wiki.archlinux.org/title/xmodmap>.

>
>> +(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.
>

I found here <https://askubuntu.com/a/1155211> that xmodmap settings can
be simply reversed by calling setxkbmap, so I've added a stop procedure
for this.

>> +(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.
>

Done.
diff mbox series

Patch

diff --git a/gnu/home/services/desktop.scm b/gnu/home/services/desktop.scm
index cb25b03b64..b3c60b017e 100644
--- a/gnu/home/services/desktop.scm
+++ b/gnu/home/services/desktop.scm
@@ -22,10 +22,13 @@  (define-module (gnu home services desktop)
   #:use-module (gnu home services shepherd)
   #:use-module (gnu services configuration)
   #:autoload   (gnu packages glib)    (dbus)
+  #:autoload   (gnu packages xorg) (xmodmap)
   #:autoload   (gnu packages xdisorg) (redshift)
+  #:use-module (guix packages)
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-43)
   #:use-module (ice-9 match)
   #:export (home-redshift-configuration
             home-redshift-configuration?
@@ -226,3 +229,109 @@  (define home-dbus-service-type
    (default-value (home-dbus-configuration))
    (description
     "Run the session-specific D-Bus inter-process message bus.")))
+
+
+;;;
+;;; xmodmap
+;;;
+
+(define-configuration/no-serialization home-xmodmap-configuration
+  (xmodmap
+   (package xmodmap)
+   "The @code{xmodmap} package to use.")
+  (config
+   (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"))
+
+(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))))
+
+(define (get-xmodmap-configuration field-name val)
+  (define serialize-term
+    (match-lambda
+      ((? vector? e)
+       (string-join
+        (vector-fold
+         (lambda (i acc e)
+           (append
+             acc
+             (list (serialize-term e))))
+         '() e) " "))
+      ((? symbol? e) (symbol->string e))
+      ((? number? e) (number->string e))
+      (e e)))
+
+  (define serialize-field
+    (match-lambda
+      ((? list? e)
+       (string-join
+        (map
+         (lambda (x)
+           (serialize-term x))
+         e)
+        " "))
+      ((key . value)
+       (format #f "~a = ~a"
+               (serialize-term key)
+               (serialize-term value)))
+      (key (string-append (serialize-term key)))))
+
+  #~(string-append
+     #$@(interpose
+         (map serialize-field val)
+         "\n" 'suffix)))
+
+(define (home-xmodmap-files-service config)
+  `(("xmodmap/config"
+     ,(home-xmodmap-file config))))
+
+(define (home-xmodmap-file config)
+  (mixed-text-file
+   "config"
+   (get-xmodmap-configuration
+    #f (home-xmodmap-configuration-config config))))
+
+(define (home-xmodmap-profile-service config)
+  (list (home-xmodmap-configuration-xmodmap config)))
+
+(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.")
+   (default-value (home-xmodmap-configuration))))
+
+(define (generate-home-xmodmap-documentation)
+  (generate-documentation
+   `((home-xmodmap-configuration
+      ,home-xmodmap-configuration-fields))
+   'home-xmodmap-configuration))