Thanks, added the fixes, attached the version 2 patch On 2024-02-15 08:07, Eli Zaretskii wrote: >> Date: Wed, 14 Feb 2024 22:47:08 +0300 >> From: Aleksandr Vityazev via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" >> >> Currently in epa.el it is possible to select keys only through a >> separate buffer, I think adding the option to select via minibuffer >> would be a nice addition. The current behavior is set to default. Any >> comments? > > Thanks, some comments below, > >> --- a/etc/NEWS >> +++ b/etc/NEWS >> @@ -1352,6 +1352,14 @@ characters, such as ½ (U+00BD VULGAR FRACTION ONE HALF), are also >> recognized as rational fractions. They have been since 2004, but it >> looks like it was never mentioned in the NEWS, or even the manual. >> >> +** EasyPG >> + >> +--- >> +*** New user option 'epa-keys-select-method' > > Heading lines in NEWS should end with a period. > Fixed >> +This allows the user to customize the key selection method, a minibuffer >> +or buffer option is available, which is set by default and was used in >> +all earlier versions. > > This sentence is too complex. Suggest to reword as two sentences: > > This allows the user to customize the key selection method, which > can be either by using a pop-up buffer or from the minibuffer. The > pop-up buffer method is the default, which preserves previous > behavior. > Applied > Also, why did you decide not to document this in the manual? epa.el > has its own manual, so how about adding this option to it? Added > >> +(defcustom epa-keys-select-method 'buffer >> + "Method used to select keys. >> +It can have two values: buffer or minibuffer. >> +Can have two values: buffer or minibuffer. In the first case, the keys >> +can be selected via a pop-up buffer. In the second case, the keys >> +can be selected via a minibuffer and `completing-read-multiple' will be >> +used." > > The doc string could also be simplified. Like this, for example: > > Method used to select keys in `epa-select-keys'. > If the value is \\='buffer, the default, keys are selected via a > pop-up buffer. If the value is \\='minibuffer, keys are selected > via the minibuffer instead, using `completing-read-multiple'. > Applied > This avoids explaining the values twice (first what they are, then > what they do), and also mentions the default value. > > Please also make sure comments, doc strings, and commit log messages > leave two spaces between sentences, per our conventions. You had a > couple of problems with that in the patch, which my suggested > rephrasing fixed, but please keep this in mind in the future. > >> + :type '(choice (const :tag "Read keys from buffer" buffer) > ^^^^^^^^^^^^^^^^^^^^^ > This should probably say "...from a pop-up buffer". Fixed > >> (defun epa-select-keys (context prompt &optional names secret) >> "Display a user's keyring and ask him to select keys. >> @@ -459,7 +484,10 @@ epa-select-keys >> the keys are listed. >> If SECRET is non-nil, list secret keys instead of public keys." >> (let ((keys (epg-list-keys context names secret))) >> - (epa--select-keys prompt keys))) >> + (pcase epa-keys-select-method >> + ('buffer (epa--select-keys prompt keys)) >> + ('minibuffer (epa--select-keys-in-minibuffer prompt keys)) >> + (_ (error "Wrong method for key selection is specified"))))) > > Hmm... are you assuming users know how to input multiple strings from > the minibuffer, in particular what is the value of crm-separator? the > function epa--select-keys inserts some instructions into the pop-up > buffer, but this new function you propose just shows the prompt and > nothing else. Should it somehow help the user with the job of typing > the keys at the prompt? The crm-separator is "[ \t]*,[ \t]*", so I added a "(comma separated)" hint that will be in all prompts. > > Also, maybe instead of signaling an error for a value that's neither > 'buffer' nor 'minibuffer', we should treat such values as 'buffer'? Agree, fixed