Subject: Re: bug#36052: 26.2.50; [PATCH] Improve auth-source-pass

From: Noam Postavsky <[email-masked]>
Date: Thu, 13 Jun 2019 17:23:47 -0400

Damien Cassou <[email-masked]> writes:

> thank you so much to Eli and Noam for their detailed review. Here is a
> new patch series that address every concern received so far. This series
> also includes a new patch rewriting the "The Unix password store"
> section of auth.texi.

A couple more minor points, and some formatting nitpicks.

> Subject: [PATCH 04/13] Add auth-source-pass-filename option

> +(defcustom auth-source-pass-filename "~/.password-store"
> +  "Path to the password-store folder."

As Eli mentioned, the docstring should say Filename rather than Path too
(I guess this got missed/mixed up with my similar point about the
variable name).

> Subject: [PATCH 10/13] Refactoring of auth-source-pass
>
> * lisp/auth-source-pass.el (auth-source-pass--find-match): Refactor by
> moving some code to auth-source-pass--disambiguate.
> (auth-source-pass--disambiguate):
> (auth-source-pass--entries-matching-suffix): New function.

When you have two or more ChangeLog entries from the same file sharing
the same message, don't leave a colon at the end of the blank lines (see
also the example in CONTRIBUTE).  I.e., this should be

    (auth-source-pass--disambiguate)
    (auth-source-pass--entries-matching-suffix): New function.

Same for the other entries below.

> (auth-source-pass--find-match-unambiguous):
> (auth-source-pass--select-from-entries):
> (auth-source-pass--entry-reducer): Refactor to simplify and improve
> logging.
> (auth-source-pass--matching-entries):
> (auth-source-pass--accumulate-matches): Remove
                                                ^
> * test/lisp/auth-source-pass-tests.el: Complete rewrite to facilitate
> maintenance.
> (auth-source-pass--have-message-containing): Remove
                                                     ^
Period at the end of sentence.

> (auth-source-pass--have-message-matching):
> (auth-source-pass--explain--have-message-matching):
> (auth-source-pass--explain-match-entry-p):
> (auth-source-pass--includes-sorted-entries):
> (auth-source-pass--explain-includes-sorted-entries):
> (auth-source-pass--explain-match-any-entry-p):
> (auth-source-pass--matching-entries):
> (auth-source-pass-match-entry-p):
> (auth-source-pass-match-any-entry-p): New function.

> Subject: [PATCH 12/13] * etc/NEWS: Describe changes to auth-source-pass

> +** auth-source-pass
> +
> +*** New customizable variable 'auth-source-pass-filename'.
> +Allows setting the path to the password-store, defaults to
> +~/.password-store.
> +
> +*** New customizable variable 'auth-source-pass-port-separator'.
> +Specifies separator between host and port, defaults to colon ":".

I guess these NEWS entries should be marked +++ since you have added
text to the manual about them.

> +*** Minimize the number of decryptions during password lookup.
> +This makes the package usable with physical tokens requiring touching
> +a sensor for every decryption.
> +
> +*** 'auth-source-pass-get' is now autoloaded.

And these should have ---.

> Subject: [PATCH 13/13] * doc/misc/auth.texi (The Unix password store):
>  Complete rewrite

> --- a/doc/misc/auth.texi
> +++ b/doc/misc/auth.texi
> @@ -445,19 +445,34 @@ The Unix password store
>  
>  @uref{http://www.passwordstore.org,,The standard unix password
>  manager} (or just @code{pass}) stores your passwords in

> +@code{gpg}-protected files following the Unix philosophy. The store
                                                            ^ 
Double spacing at the end of sentence.  Same for the rest of this patch.

> +Emacs integration of @code{pass} follows the approach suggested by the
> +pass project itself for data organization to find data. In particular,
> +to store a password for the user @code{rms} on the host @code{gnu.org}
> +on port @code{22}, you should use one of the following filenames.

> +@itemize
> +@item @code{"gnu.org.gpg"} : No username or port in the filename means that any username and port will match.
> +@item @code{"gnu.org/rms.gpg"} : The username to match can be expressed as filename inside a directory whose name matches the host. This is useful if the store has passwords for several users on the same host.
> +@item @code{"rms@@gnu.org.gpg"} : The username can also be expressed as a prefix, separated from the host with an at-sign (@code{@@}).
> +@item @code{"gnu.org:22.gpg"} : The port (aka. service) to match can only be expressed after the host and separated with a colon (@code{:}). The separator can be changed through the @code{auth-source-pass-port-separator} variable.

> +If several entries match, the one matching the most is
> +preferred.

"the most" meaning highest number of matching items (where an "item" is
one of username, port or filename)?  The current wording could be
understood as "longest match" (counting characters) which I don't think
is what you meant.