GNU bug report logs - #75270
[PATCH 0/3] services: greetd: Improve greeter configurations.

Previous Next

Full log

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: muradm <mail <at> muradm.net>
Cc: 75270 <at> debbugs.gnu.org
Subject: Re: [PATCH v5 1/3] services: greetd: Improve greeter configurations.
Date: Wed, 05 Feb 2025 14:27:05 +0900

Hi again!

muradm <mail <at> muradm.net> writes:

[...]

> +@item @code{xdg-session-type} (default: @code{"tty"})
> +Specify the value of @code{XDG_SESSION_TYPE}.  The User environment may

s/User/user/

[...]

> +(define-record-type* <greetd-user-session>
> +  greetd-user-session make-greetd-user-session greetd-user-session?
> +  (command greetd-user-session-command (default (file-append bash "/bin/bash")))
> +  (command-args greetd-user-session-command-args (default '("-l")))
> +  (extra-env greetd-user-session-extra-env (default '()))
> +  (xdg-session-type greetd-user-session-xdg-session-type (default "tty"))
> +  (xdg-env? greetd-user-session-xdg-env? (default #t)))
> +
> +(define (make-greetd-user-session-command config)
> +  (match-record config <greetd-user-session>
> +                (command command-args extra-env)
> +                (program-file
> +                 "greetd-user-session-command"
> +                 #~(begin
> +                     (use-modules (ice-9 match))
> +                     (for-each (match-lambda ((var . val) (setenv var val)))
> +                               (quote (#$@extra-env)))
> +                     (apply execl #$command #$command (list #$@command-args))))))

Please break long lines under 80 chars.

> +
> +(define (make-greetd-xdg-user-session-command config)
> +  (match-record config <greetd-user-session>
> +                (command command-args extra-env xdg-session-type)
> +                (program-file
> +                 "greetd-xdg-user-session-command"
> +                 #~(begin
> +                     (use-modules (ice-9 match))
> +                     (let* ((username (getenv "USER"))
> +                            (useruid (passwd:uid (getpwuid username)))
> +                            (useruid (number->string useruid)))
> +                       (setenv "XDG_SESSION_TYPE" #$xdg-session-type)
> +                       (setenv "XDG_RUNTIME_DIR" (string-append "/run/user/" useruid)))
> +                     (for-each (match-lambda ((var . val) (setenv var val)))
> +                               (quote (#$@extra-env)))
> +                     (apply execl #$command #$command (list #$@command-args))))))

Please break long lines under 80 chars.

[...]

> +  greetd-agreety-session make-greetd-agreety-session greetd-agreety-session?
> +  (agreety greetd-agreety-session-agreety (default greetd))
> +  (command greetd-agreety-session-command
> +           (default (greetd-user-session))
> +           (sanitize warn-greetd-agreety-session-command-type))
> +  (command-args greetd-agreety-command-args
> +                (default #nil)
> +                (sanitize warn-deprecated-greetd-agreety-command-args))
> +  (extra-env greetd-agreety-extra-env
> +             (default #nil)
> +             (sanitize warn-deprecated-greetd-agreety-extra-env))
> +  (xdg-env? greetd-agreety-xdg-env?
> +            (default #nil)
> +            (sanitize warn-deprecated-greetd-agreety-xdg-env?)))
> +
> +(define (warn-deprecated-greetd-agreety-command-args value)
> +  (when (not (nil? value))

#nil.  Interesting, I had never used it.  You should use 'unless' here
instead of 'when ... not' though.

> +    (warn-about-deprecation

Despite being public, this particular deprecation procedure is not the
best, as it deosn't handle the source location itself.  I've tested the
following successfully:

> +     'command-args #f
> +     #:replacement '<greetd-user-seesion>))
> +  value)

There's a typo, here and for the other deprecation procedures:
s/seesion/session/

Ideally deprecation warns and the configuration records are
automatically rewritten to the modern equivalent in the code, which
means only one code path in the service exists for one given option, but
I don't think that's possible here given the new command record replaces
multiple fields, so OK.  I couldn't find a way to preserve the syntax
source info too, in my quick experiment.

[...]

> +(define-record-type* <greetd-wlgreet-configuration>
> +  greetd-wlgreet-configuration make-greetd-wlgreet-configuration
> +  greetd-wlgreet-configuration?
> +  (output-mode greetd-wlgreet-configuration-output-mode (default "all"))
> +  (scale greetd-wlgreet-configuration-scale (default 1))
> +  (background greetd-wlgreet-configuration-background (default '(0 0 0 0.9)))
> +  (headline greetd-wlgreet-configuration-headline (default '(1 1 1 1)))
> +  (prompt greetd-wlgreet-configuration-prompt (default '(1 1 1 1)))
> +  (prompt-error greetd-wlgreet-configuration-prompt-error (default '(1 1 1 1)))
> +  (sway greetd-wlgreet-sway-session-sway (default sway))
> +  (sway-configuration greetd-wlgreet-sway-session-sway-configuration
> +                      (default #f))
> +  (wlgreet greetd-wlgreet-sway-session-wlgreet (default wlgreet))
> +  (wlgreet-configuration greetd-wlgreet-sway-session-wlgreet-configuration
> +                         (default (greetd-wlgreet-configuration)))
> +  (command greetd-wlgreet-sway-session-command (default (greetd-user-session)))
> +  (wlgreet-session
> +   greetd-wlgreet-sway-session-wlgreet-session
> +   (default #nil)
> +   (sanitize warn-deprecated-greetd-wlgreet-sway-session-wlgreet-session)))
> +
> +(define (warn-deprecated-greetd-wlgreet-sway-session-wlgreet-session value)
> +  (when (not (nil? value))
> +    (warn-about-deprecation
> +     'wlgreet-session #f
> +     #:replacement 'wlgreet-configuration))
> +  value)
> +
> +(define make-greetd-wlgreet-sway-session-sway-config
> +  (match-lambda
> +    (($ <greetd-wlgreet-sway-session>
> +        sway sway-config wlgreet wlgreet-config command)
> +     (let ((wlgreet-bin (file-append wlgreet "/bin/wlgreet"))
> +           (wlgreet-config-file
> +            (make-greetd-wlgreet-config command wlgreet-config))
> +           (swaymsg-bin (file-append sway "/bin/swaymsg")))
> +       (mixed-text-file
> +        "wlgreet-sway-config"
> +        (if sway-config "include " "")
> +        (if sway-config sway-config "")
> +        (if sway-config "\n" "")
> +        "xwayland disable\n"
> +        "exec \"" wlgreet-bin " --config " wlgreet-config-file
> +        "; " swaymsg-bin " exit\"\n")))))
> +
> +(define (greetd-wlgreet-session-to-config session config)
> +  (let* ((wlgreet (or (greetd-wlgreet config)
> +                      (greetd-wlgreet-sway-session-wlgreet session)))
> +         (default-command (greetd-wlgreet-sway-session-command session))
> +         (command (or (greetd-wlgreet-command config)
> +                      (greetd-user-session-command default-command)))
> +         (command-args (or (greetd-wlgreet-command-args config)
> +                           (greetd-user-session-command-args default-command)))
> +         (extra-env (or (greetd-wlgreet-extra-env config)
> +                        (greetd-user-session-extra-env default-command))))
> +    (greetd-wlgreet-sway-session
> +     (sway (greetd-wlgreet-sway-session-sway session))
> +     (sway-configuration (greetd-wlgreet-sway-session-sway-configuration session))

Line width > 80 :-)

Part from these nitpicks:

Reviewed-by: Maxim Cournoyer <maxim.cournoyer <at> gmail>

-- 
Thanks,
Maxim

Previous Next

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