guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#62461] Additional ssh configuration options.


From: Ludovic Courtès
Subject: [bug#62461] Additional ssh configuration options.
Date: Sat, 01 Apr 2023 09:45:26 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

Hi Nicolas,

Nice work!

Nicolas Graves <ngraves@ngraves.fr> skribis:

> ---
>  gnu/home/services/ssh.scm | 44 +++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 9 deletions(-)

Bonus point if you add a ChangeLog-style commit log.  :-)

> +(define (serialize-add-keys-to-agent value)
> +  (define (is-valid-time-string? str)
> +    (and (> (string-length str) 0)
> +         (eq?
> +          (cdr (vector-ref
> +                (string-match "\
> +[0-9]+|([0-9]+[Ww])?([0-9]+[Dd])?([0-9]+[Hh])?([0-9]+[Mm])?([0-9]+[Ss])?" 
> str)
> +                1))
> +          (string-length str))))

In general please use ‘match’ instead of car/cdr/cadddr (info "(guix)
Data Types and Pattern Matching").

That said, the result of ‘string-match’ is meant to be accessed with
‘match:substring’, not with ‘vector-ref’ (info "(guile) Match
Structures").

Nitpick: you can remove ‘is-’ from the name.

> +  (string-append "AddKeysToAgent "
> +                 (cond ((member value '("yes" "no" "confirm" "ask")) value)
> +                       ((is-valid-time-string? value) value)
> +                       ((and (string-prefix? "confirm" value)
> +                             (is-valid-time-string?
> +                              (cdr (string-split value #\ )))) value)
> +                       ;; The 'else' branch is unreachable.
> +                       (else (raise (condition (&error)))))))

I guess the ‘else’ branch is reachable if one uses the wrong value?
Should it instead be:

  (raise (formatted-message (G_ "~s: invalid 'add-keys-to-agent' value")
                            value))

?

Ludo’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]