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
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
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
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
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