Re: problems importing keys via epa-search-keys

[Filipp Gunbin <fgunbin@fastmail.fm>]

On 12/03/2023 08:33 +0200, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: help-gnu-emacs@gnu.org
>> Date: Sun, 12 Mar 2023 02:04:21 +0300
>> 
>> On 11/03/2023 09:17 +0200, Eli Zaretskii wrote:
>> 
>> > It's a regression that existed since Emacs 28.1, right?  So its fix
>> > should try to avoid changing APIs.  Therefore, please try to make a
>> > change that leaves the signature of epa-ks--query-url unchanged, I
>> > think it should be easy in this case.  (Yes, I know it's an internal
>> > function, but that doesn't mean we can change the signature when we
>> > can avoid that.)
>> 
>> Yes, since 28.1.  The updated patch is below.
>> 
>> But why an internal function is part of API?  Don't we assume that
>> anyone using them is doing so at their own risk?  In this case the
>> default for operation is "index" (to preserve compatibility), but it's
>> better here to not have the default value at all.
>
> I agree that we can break the API if we need, but we don't need to,
> and it is always better to keep the signature unless it's impossible.

Thanks, noted.

> So please modify your patch to leave the signature intact, I don't
> think it's hard in this case.

But I did in the last patch?  There's now an optional parameter, which
preserves API compatibility.  Or did you mean something else?

For convenience, I post the patch again below.

I still need your decision where to install this - master or emacs-29.

>> > P.S. And why are you posting patches and their discussion here, not on
>> > the bug tracker?
>> 
>> Because OP wrote here?  And we didn't open a bug for this.  Should we,
>> even for such simple fixes?
>
> It's better, because then the discussion is recorded and referenced in
> the commit.

Noted.

Thanks.


diff --git a/lisp/epa-ks.el b/lisp/epa-ks.el
index 77d896fa438..015bf910ac6 100644
--- a/lisp/epa-ks.el
+++ b/lisp/epa-ks.el
@@ -140,8 +140,8 @@ epa-ks-do-key-to-fetch
           (epa-ks--fetch-key id)))))
   (tabulated-list-clear-all-tags))
 
-(defun epa-ks--query-url (query exact)
-  "Return URL for QUERY.
+(defun epa-ks--query-url (query exact &optional operation)
+  "Return URL for QUERY and OPERATION (defaults to \"index\").
 If EXACT is non-nil, don't accept approximate matches."
   (format "https://%s/pks/lookup?%s"
           (cond ((null epa-keyserver)
@@ -154,13 +154,13 @@ epa-ks--query-url
           (url-build-query-string
            (append `(("search" ,query)
                      ("options" "mr")
-                     ("op" "index"))
+                     ("op" ,(or operation "index")))
                    (and exact '(("exact" "on")))))))
 
 (defun epa-ks--fetch-key (id)
   "Send request to import key with specified ID."
   (url-retrieve
-   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t)
+   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t "get")
    (lambda (status)
      (when (plist-get status :error)
        (error "Request failed: %s"
@@ -236,7 +236,7 @@ epa-search-keys
         (erase-buffer))
       (epa-ks-search-mode))
     (url-retrieve
-     (epa-ks--query-url query exact)
+     (epa-ks--query-url query exact "index")
      (lambda (status)
        (when (plist-get status :error)
          (when buf