[PATCH 0/4] eww, elfeed: Improve inbox and message ID detection

[PATCH 0/4] eww, elfeed: Improve inbox and message ID detection

[Kyle Meyer]

As noted in the piem-eww.el comment, the intial approach was quite
flawed.

  [1/4] elfeed, eww: Don't assume inbox name in piem-inboxes and URL match
  [2/4] Add accessor for piem-inboxes
  [3/4] piem-inbox-{codrepo,get}: Allow caller to specify inbox
  [4/4] elfeed, eww: Be stricter about the returned message ID

 piem-b4.el     |  2 +-
 piem-elfeed.el | 12 +++++------
 piem-eww.el    | 16 ++++++---------
 piem.el        | 56 ++++++++++++++++++++++++++++++++------------------
 4 files changed, 49 insertions(+), 37 deletions(-)


base-commit: 5cada85c7571956345f04e326c8af045cdb62131
-- 
2.28.0

[PATCH 1/4] elfeed, eww: Don't assume inbox name in piem-inboxes and URL match

[Kyle Meyer]

piem-elfeed-get-inbox and piem-eww-get-inbox match the URL against
piem-link-re and take the second group as the inbox name.  That's a
bad approach because the inbox name in the URL doesn't necessarily
match the one in piem-inboxes.  For example, public-inbox's own
archive is https://public-inbox.org/meta/, but my entry in
piem-inboxes uses the name "public-inbox":

    ("public-inbox" :url "https://public-inbox.org/meta/" ...)

The approach also fails if the URL isn't a public-inbox message URL
because piem-link-re isn't very specific.  (That will be improved in
an upcoming commit.)

Find the inbox name by matching the buffer URL against the :url values
in piem-inboxes.
---
 piem-elfeed.el |  3 +--
 piem-eww.el    |  7 +------
 piem.el        | 14 ++++++++++++++
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/piem-elfeed.el b/piem-elfeed.el
index e1ee2f8..5a6472b 100644
--- a/piem-elfeed.el
+++ b/piem-elfeed.el
@@ -39,8 +39,7 @@ (defun piem-elfeed-get-inbox ()
   "Return inbox name from an `elfeed-show-mode' buffer."
   (when (derived-mode-p 'elfeed-show-mode)
     (when-let ((link (elfeed-entry-link elfeed-show-entry)))
-      (and (string-match piem-link-re link)
-           (match-string 1 link)))))
+      (piem-inbox-by-url-match link))))
 
 (defun piem-elfeed-get-mid ()
   "Return the message ID of an `elfeed-show-mode' buffer."
diff --git a/piem-eww.el b/piem-eww.el
index e8c77f5..5fb629b 100644
--- a/piem-eww.el
+++ b/piem-eww.el
@@ -38,12 +38,7 @@ (defun piem-eww-get-inbox ()
   "Return inbox name from an EWW buffer."
   (when (derived-mode-p 'eww-mode)
     (when-let ((link (plist-get eww-data :url)))
-      ;; NEEDSWORK: This relies on the inbox name in the URL matching
-      ;; the inbox name in `piem-inboxes', which might not be the
-      ;; case.  If there's a :url in `piem-inboxes', that could be
-      ;; matched instead.
-      (and (string-match piem-link-re link)
-           (match-string 1 link)))))
+      (piem-inbox-by-url-match link))))
 
 (defun piem-eww-get-mid ()
   "Return the message ID of an EWW buffer."
diff --git a/piem.el b/piem.el
index 42646a0..f59f723 100644
--- a/piem.el
+++ b/piem.el
@@ -252,6 +252,9 @@ (defun piem-process-call-with-buffer-input
 \f
 ;;;; Extractors
 
+(defun piem--ensure-trailing-slash (s)
+  (if (string-match-p ".+/\\'" s) s (concat s "/")))
+
 (defvar piem-link-re
   (rx "/" (group (one-or-more (not (any "/" "\n"))))
       "/" (group (one-or-more (not (any "/" "\n"))))
@@ -301,6 +304,17 @@ (defun piem-inbox-coderepo ()
              (repo (plist-get (cdr (assoc p piem-inboxes)) :coderepo)))
     (expand-file-name repo)))
 
+(defun piem-inbox-by-url-match (url)
+  "Return inbox based on matching URL against `:url'."
+  (setq url (piem--ensure-trailing-slash url))
+  (catch 'hit
+    (dolist (inbox piem-inboxes)
+      (when-let ((info (cdr inbox))
+                 (p-url (plist-get info :url)))
+        (setq p-url (piem--ensure-trailing-slash p-url))
+        (when (string-match-p (regexp-quote p-url) url)
+          (throw 'hit (car inbox)))))))
+
 (defun piem-inbox-url ()
   "Return the URL of current buffer's inbox."
   (when-let ((p (piem-inbox)))
-- 
2.28.0

[PATCH 2/4] Add accessor for piem-inboxes

[Kyle Meyer]

There's no need to have a function like piem-inbox-url for every key.
Drop piem-inbox-url, but keep piem-inbox-coderepo around because it
does a bit of extra processing on top.
---
 piem-b4.el |  2 +-
 piem.el    | 15 +++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/piem-b4.el b/piem-b4.el
index 6de0d2b..f5aa777 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -63,7 +63,7 @@ (defun piem-b4--get-am-files (mid coderepo args)
     ;; try to download it from a URL at `piem-inboxes'.  Finally, fall
     ;; back to b4's configuration.
     (unless local-mbox-p
-      (when-let ((url (piem-inbox-url))
+      (when-let ((url (piem-inbox-get :url))
                  (mid (piem-mid))
                  (buffer (condition-case nil
                              (piem-download-and-decompress
diff --git a/piem.el b/piem.el
index f59f723..6102b0f 100644
--- a/piem.el
+++ b/piem.el
@@ -298,10 +298,14 @@ (defun piem-inbox ()
   "Return the current buffer's inbox."
   (run-hook-with-args-until-success 'piem-get-inbox-functions))
 
+(defun piem-inbox-get (key)
+  "Get info KEY for the inbox entry in `piem-inboxes'."
+  (when-let ((p (piem-inbox)))
+    (plist-get (cdr (assoc p piem-inboxes)) key)))
+
 (defun piem-inbox-coderepo ()
   "Return the code repository of current buffer's inbox."
-  (when-let ((p (piem-inbox))
-             (repo (plist-get (cdr (assoc p piem-inboxes)) :coderepo)))
+  (when-let ((repo (piem-inbox-get :coderepo)))
     (expand-file-name repo)))
 
 (defun piem-inbox-by-url-match (url)
@@ -315,11 +319,6 @@ (defun piem-inbox-by-url-match (url)
         (when (string-match-p (regexp-quote p-url) url)
           (throw 'hit (car inbox)))))))
 
-(defun piem-inbox-url ()
-  "Return the URL of current buffer's inbox."
-  (when-let ((p (piem-inbox)))
-    (plist-get (cdr (assoc p piem-inboxes)) :url)))
-
 (defun piem-inbox-coderepo-maybe-read ()
   "Like `piem-inbox-coderepo', but fall back to reading the repo."
   (let ((inbox
@@ -495,7 +494,7 @@ (defun piem-inject-thread-into-maildir (mid &optional message-only)
      "`piem-maildir-directory' does not look like a Maildir directory"))
    ((not (or message-only (piem-check-gunzip)))
     (user-error "gunzip executable not found")))
-  (when-let ((url (concat (or (piem-inbox-url)
+  (when-let ((url (concat (or (piem-inbox-get :url)
                               (user-error
                                "Could not find inbox URL for current buffer"))
                           mid
-- 
2.28.0

[PATCH 3/4] piem-inbox-{codrepo,get}: Allow caller to specify inbox

[Kyle Meyer]

All the callers at the moment only care about the current inbox, but
this is still useful for avoiding a repeated call to piem-inbox (and
an upcoming commit will use it to do so).
---
 piem.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/piem.el b/piem.el
index 6102b0f..4b840cb 100644
--- a/piem.el
+++ b/piem.el
@@ -298,14 +298,15 @@ (defun piem-inbox ()
   "Return the current buffer's inbox."
   (run-hook-with-args-until-success 'piem-get-inbox-functions))
 
-(defun piem-inbox-get (key)
-  "Get info KEY for the inbox entry in `piem-inboxes'."
-  (when-let ((p (piem-inbox)))
+(defun piem-inbox-get (key &optional inbox)
+  "Get info KEY for INBOX's entry in `piem-inboxes'.
+If INBOX is nil, use the inbox returned by `piem-inbox'."
+  (when-let ((p (or inbox (piem-inbox))))
     (plist-get (cdr (assoc p piem-inboxes)) key)))
 
-(defun piem-inbox-coderepo ()
+(defun piem-inbox-coderepo (&optional inbox)
   "Return the code repository of current buffer's inbox."
-  (when-let ((repo (piem-inbox-get :coderepo)))
+  (when-let ((repo (piem-inbox-get :coderepo inbox)))
     (expand-file-name repo)))
 
 (defun piem-inbox-by-url-match (url)
-- 
2.28.0

[PATCH 4/4] elfeed, eww: Be stricter about the returned message ID

[Kyle Meyer]

Using the second group in piem-link-re is not reliable because the
trailing part of the URL may be anything.  Instead get the inboxes
:url first and then generate a regular expression that has that value
as the prefix.
---
 piem-elfeed.el |  9 +++++----
 piem-eww.el    |  9 +++++----
 piem.el        | 26 ++++++++++++++------------
 3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/piem-elfeed.el b/piem-elfeed.el
index 5a6472b..b1e112b 100644
--- a/piem-elfeed.el
+++ b/piem-elfeed.el
@@ -43,10 +43,11 @@ (defun piem-elfeed-get-inbox ()
 
 (defun piem-elfeed-get-mid ()
   "Return the message ID of an `elfeed-show-mode' buffer."
-  (when (derived-mode-p 'elfeed-show-mode)
-    (when-let ((link (elfeed-entry-link elfeed-show-entry)))
-      (and (string-match piem-link-re link)
-           (match-string 2 link)))))
+  (when-let ((inbox (piem-elfeed-get-inbox))
+             (inbox-url (piem-inbox-get :url inbox))
+             (link (elfeed-entry-link elfeed-show-entry)))
+    (and (string-match (piem-message-link-re inbox-url) link)
+         (match-string 1 link))))
 
 ;;;###autoload
 (define-minor-mode piem-elfeed-mode
diff --git a/piem-eww.el b/piem-eww.el
index 5fb629b..62b7166 100644
--- a/piem-eww.el
+++ b/piem-eww.el
@@ -42,10 +42,11 @@ (defun piem-eww-get-inbox ()
 
 (defun piem-eww-get-mid ()
   "Return the message ID of an EWW buffer."
-  (when (derived-mode-p 'eww-mode)
-    (when-let ((link (plist-get eww-data :url)))
-      (and (string-match piem-link-re link)
-           (match-string 2 link)))))
+  (when-let ((inbox (piem-eww-get-inbox))
+             (inbox-url (piem-inbox-get :url inbox))
+             (url (plist-get eww-data :url)))
+    (and (string-match (piem-message-link-re inbox-url) url)
+         (match-string 1 url))))
 
 ;;;###autoload
 (define-minor-mode piem-eww-mode
diff --git a/piem.el b/piem.el
index 4b840cb..da1a693 100644
--- a/piem.el
+++ b/piem.el
@@ -255,18 +255,20 @@ (defun piem-process-call-with-buffer-input
 (defun piem--ensure-trailing-slash (s)
   (if (string-match-p ".+/\\'" s) s (concat s "/")))
 
-(defvar piem-link-re
-  (rx "/" (group (one-or-more (not (any "/" "\n"))))
-      "/" (group (one-or-more (not (any "/" "\n"))))
-      "/" (group (zero-or-one
-                  (or "raw"
-                      "t.mbox.gz"
-                      (and (or "t" "T") "/#"
-                           (one-or-more (not (any "/" "\n")))))))
-      string-end)
-  "Regular expression matching public-inbox HTTP link.
-The first group is the inbox, the second is the message ID, and
-the rest is any trailing endpoint.")
+(defun piem-message-link-re (url &optional mid)
+  "Return a regular expression matching a public-inbox url.
+URL should be the top-level url for the inbox.  If MID is
+non-nil, make the match specific for that message."
+  (rx-to-string
+   `(and ,(piem--ensure-trailing-slash url)
+         (group ,(or mid
+                     '(one-or-more (not (any "/" "\n")))))
+         "/" (group (zero-or-one
+                     (or "raw"
+                         "t.mbox.gz"
+                         (and (or "t" "T") "/#"
+                              (one-or-more (not (any "/" "\n")))))))
+         string-end)))
 
 (defun piem-inbox-by-header-match ()
   "Return inbox based on matching message headers.
-- 
2.28.0