GNU bug report logs - #69013
New package for NonGNU ELPA: totp-auth

Previous Next

Full log

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 69013 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>,
 Stefan Kangas <stefankangas <at> gmail.com>, Vivek Das Mohapatra <vivek <at> etla.org>
Subject: Re: bug#69013: New package for NonGNU ELPA: totp-auth
Date: Sat, 10 Feb 2024 13:41:21 -0500

> -  (delq nil
> -        (mapcar (lambda (s &optional secure type source handler)
> -                  (setq source  (slot-value s 'source)
> -                        type    (slot-value s 'type)
> -                        secure  (or (eq type 'secrets)
> -                                    (eq type 'plist)
> -                                    (equal (file-name-extension source) "gpg"))
> -                        handler (if (eq type 'secrets) :secrets :default))
> -                  (if (and encrypted (not secure))
> -                      nil
> -                    (list (cons :source    s)
> -                          (cons :handler   handler)
> -                          (cons :encrypted secure))))
> -                (mapcar #'auth-source-backend-parse (totp-auth-sources)))))
> +  (mapcan
> +   (lambda (s &optional secure type source handler)
> +     (setq source  (slot-value s 'source)
> +           type    (slot-value s 'type)
> +           secure  (or (eq type 'secrets)
> +                       (eq type 'plist)
> +                       (equal (file-name-extension source) "gpg"))
> +           handler (if (eq type 'secrets) :secrets :default))
> +     (if (and encrypted (not secure))
> +         nil
> +       `(((:source ,s) (:handler ,handler) (:encrypted ,secure)))))
> +   (mapcar #'auth-source-backend-parse (totp-auth-sources))))

Also `mapcar/mapcan` will only pass one argument to the loop-body
function, so in the code above the args `&optional secure type source
handler` will always be nil :-(

Maybe you'd want to merge the two loops:

    (mapcan
     (lambda (entry)
       (let* ((s (auth-source-backend-parse entry))
              (source  (slot-value s 'source))
              (type    (slot-value s 'type))
              (secure  (or (memq type '(secrets plist))
                           (equal (file-name-extension source) "gpg")))
              (handler (if (eq type 'secrets) :secrets :default)))
       (if (and encrypted (not secure))
           nil
         `(((:source ,s) (:handler ,handler) (:encrypted ,secure))))))
     (totp-auth-sources))))

> I have to admit that your
>
> (let (foo)
>   (setq foo bar)
>   ...)
>
> pattern confuses me, it seems intentional but unnecessary, so I didn't
> comment on every instance.

AFAIK it's common for coders coming from imperative programming
languages like C where it's common to declare variables separately from
their initialization.

In Lisp it's unidiomatic (tho I've seen this style used in old
Common-Lisp code as well) and in ELisp it comes almost exclusively with
downsides:
- Of course, the usual downside is that it introduces a time-window
  during which the variable is not initialized, so it makes the code
  fundamentally more complex.
- It makes the code more verbose.
- It makes the code less efficient.


        Stefan

Previous Next

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