Previously the user was able to configure a maildir to inject threads into, but the user might want the maildir to be different depending on which mailing list the threads was coming from. With the `:maildir` keyword, users can configure the maildir on a per-list basis. If there is not `:maildir` configured for a mailing list, it will fallback to the value of `piem-maildir-directory`. --- I made `piem--write-mbox-to-maildir` take the maildir as an argument, otherwise `piem-inbox-maildir-directory` would return nil because it is not in a Gnus och Notmuch buffer when being called in `piem--write-mbox-to-maildir`. Documentation/piem.texi | 3 +- piem.el | 90 ++++++++++++++++++++++++----------------- 2 files changed, 55 insertions(+), 38 deletions(-) diff --git a/Documentation/piem.texi b/Documentation/piem.texi index ebd756d..9f8779f 100644 --- a/Documentation/piem.texi +++ b/Documentation/piem.texi @@ -410,7 +410,8 @@ regular mail but are following a project via NNTP in Gnus). In this case, you can use the command @code{piem-inject-thread-into-maildir} to move the thread's messages into a local Maildir directory -(@code{piem-maildir-directory}). By default the command downloads the +(@code{piem-maildir-directory}, or the @code{:maildir} for the +current inbox). By default the command downloads the entire thread for the message ID associated with the current buffer. A prefix argument restricts the download to only the message. diff --git a/piem.el b/piem.el index 56f0b54..fd0a41b 100644 --- a/piem.el +++ b/piem.el @@ -80,6 +80,8 @@ (defcustom piem-inboxes nil Local path of the code repository associated with the inbox. :url A URL hosting HTTPS archives. This value must end with a slash. + :maildir + A Maildir directory to inject messages into. Here's an example for the public-inbox project itself: @@ -87,7 +89,8 @@ (defcustom piem-inboxes nil :coderepo \"~/src/public-inbox/\" :address \"meta@public-inbox.org\" :listid \"meta.public-inbox.org\" - :url \"https://public-inbox.org/meta/\")" + :url \"https://public-inbox.org/meta/\" + :maildir \"~/.local/share/mail/.lists.mail.public-inbox\")" :type '(alist :key-type string :value-type (plist :value-type string))) @@ -169,8 +172,11 @@ (defcustom piem-am-read-worktree-function #'piem-am-read-worktree :type 'function) (defcustom piem-maildir-directory nil - "Inject public-inbox threads into this directory. -If non-nil, this must be an existing Maildir directory." + "Default directory to inject public-inbox threads into. + +If non-nil, this must be an existing Maildir directory. This can be +overriden on a per-list basis by using the \":maildir\" keyword in +`piem-inboxes'." :type 'string) (defcustom piem-mail-injection-skipif-predicate nil @@ -373,6 +379,15 @@ (defun piem-inbox-coderepo (&optional inbox) (when-let ((repo (piem-inbox-get :coderepo inbox))) (expand-file-name repo))) +(defun piem-inbox-maildir-directory (&optional inbox) + "Return the maildir for INBOX's entry in `piem-inboxes'. + +If INBOX is nil, use the inbox returned by `piem-inbox'. If the +INBOX doesn't have a maildir configured, return the value of +`piem-maildir-directory'." + (or (piem-inbox-get :maildir inbox) + piem-maildir-directory)) + (defun piem-inbox-by-url-match (url) "Return inbox based on matching URL against `:url'." (setq url (piem--ensure-trailing-slash url)) @@ -547,7 +562,7 @@ (defun piem-download-and-decompress (url) \f ;;;; Maildir injection -(defun piem--write-mbox-to-maildir () +(defun piem--write-mbox-to-maildir (maildir-directory) (let ((n-added 0) (n-skipped 0)) (while (and (not (eobp)) @@ -558,7 +573,7 @@ (defun piem--write-mbox-to-maildir () (point-marker))) (point-max-marker))) (basename (piem-maildir-make-uniq-maildir-id)) - (tmpfile (concat piem-maildir-directory "/tmp/" basename))) + (tmpfile (concat maildir-directory "/tmp/" basename))) (goto-char beg) (if (and (functionp piem-mail-injection-skipif-predicate) (save-excursion @@ -579,7 +594,7 @@ (defun piem--write-mbox-to-maildir () end t) (replace-match "\\1" t))) (write-region beg end tmpfile nil nil nil 'excl) - (piem-maildir-move-tmp-to-new piem-maildir-directory + (piem-maildir-move-tmp-to-new maildir-directory basename) (delete-file tmpfile) (cl-incf n-added)) @@ -588,7 +603,7 @@ (defun piem--write-mbox-to-maildir () ;;;###autoload (defun piem-inject-thread-into-maildir (mid &optional message-only) - "Inject thread containing MID into `piem-maildir-directory'. + "Inject thread containing MID into `piem-inbox-maildir-directory'. If prefix argument MESSAGE-ONLY is non-nil, inject just the message for MID, not the entire thread. @@ -599,36 +614,37 @@ (defun piem-inject-thread-into-maildir (mid &optional message-only) (list (or (piem-mid) (user-error "No message ID found for the current buffer")) current-prefix-arg)) - (cond - ((not piem-maildir-directory) - (user-error "`piem-maildir-directory' is not configured")) - ((not (piem-maildir-dir-is-maildir-p piem-maildir-directory)) - (user-error - "`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 (piem-mid-url mid) - (if message-only "/raw" "/t.mbox.gz"))) - (buffer (url-retrieve-synchronously url 'silent))) - (unwind-protect - (with-current-buffer buffer - (if (/= url-http-response-status 200) - (error "Download of %s failed" url) - (piem--url-remove-header) - (unless message-only - (piem--url-decompress)) - (pcase-let ((`(,added-count . ,skipped-count) - (piem--write-mbox-to-maildir))) - (message "Added %d message%s%s for %s to %s" - added-count - (if (= added-count 1) "" "s") - (if (> skipped-count 0) - (format " (skipping %d)" skipped-count) - "") - mid - (abbreviate-file-name piem-maildir-directory))) - (run-hook-with-args 'piem-after-mail-injection-functions mid))) - (kill-buffer buffer)))) + (let ((maildir-directory (piem-inbox-maildir-directory))) + (cond + ((not maildir-directory) + (user-error "`maildir-directory' is not configured")) + ((not (piem-maildir-dir-is-maildir-p maildir-directory)) + (user-error + "`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 (piem-mid-url mid) + (if message-only "/raw" "/t.mbox.gz"))) + (buffer (url-retrieve-synchronously url 'silent))) + (unwind-protect + (with-current-buffer buffer + (if (/= url-http-response-status 200) + (error "Download of %s failed" url) + (piem--url-remove-header) + (unless message-only + (piem--url-decompress)) + (pcase-let ((`(,added-count . ,skipped-count) + (piem--write-mbox-to-maildir maildir-directory))) + (message "Added %d message%s%s for %s to %s" + added-count + (if (= added-count 1) "" "s") + (if (> skipped-count 0) + (format " (skipping %d)" skipped-count) + "") + mid + (abbreviate-file-name maildir-directory))) + (run-hook-with-args 'piem-after-mail-injection-functions mid))) + (kill-buffer buffer))))) \f ;;;; Patch handling base-commit: 26c8103eaa9df3ebaf873a7cd477efaa48cec140 -- 2.30.0
Xinglu Chen writes: > Previously the user was able to configure a maildir to inject threads > into, but the user might want the maildir to be different depending > on which mailing list the threads was coming from. Yeah, I suspect that'd be a pretty common preference. (From the current limitation, you probably guessed that I'm a lumper when it comes to mailing list messages.) > With the `:maildir` keyword, users can configure the maildir on a > per-list basis. If there is not `:maildir` configured for a mailing > list, it will fallback to the value of `piem-maildir-directory`. Sounds good. > --- > I made `piem--write-mbox-to-maildir` take the maildir as an argument, > otherwise `piem-inbox-maildir-directory` would return nil because it is > not in a Gnus och Notmuch buffer when being called in > `piem--write-mbox-to-maildir`. Make sense. > Documentation/piem.texi | 3 +- > piem.el | 90 ++++++++++++++++++++++++----------------- > 2 files changed, 55 insertions(+), 38 deletions(-) > > diff --git a/Documentation/piem.texi b/Documentation/piem.texi > index ebd756d..9f8779f 100644 > --- a/Documentation/piem.texi > +++ b/Documentation/piem.texi > @@ -410,7 +410,8 @@ > regular mail but are following a project via NNTP in Gnus). In this > case, you can use the command @code{piem-inject-thread-into-maildir} to > move the thread's messages into a local Maildir directory > -(@code{piem-maildir-directory}). By default the command downloads the > +(@code{piem-maildir-directory}, or the @code{:maildir} for the > +current inbox). By default the command downloads the Hmm, with :maildir coming second, I think I'd interpret the priority to be opposite of what it is. Perhaps something closer to the phrasing you used in the commit message: ... into a local Maildir directory specified by the current inbox's @code{:maildir} value in @code{piem-inboxes}, falling back to @code{piem-maildir-directory}. > entire thread for the message ID associated with the current buffer. A > prefix argument restricts the download to only the message. > > diff --git a/piem.el b/piem.el > index 56f0b54..fd0a41b 100644 > --- a/piem.el > +++ b/piem.el [...] > @@ -588,7 +603,7 @@ (defun piem--write-mbox-to-maildir () > > ;;;###autoload > (defun piem-inject-thread-into-maildir (mid &optional message-only) > - "Inject thread containing MID into `piem-maildir-directory'. > + "Inject thread containing MID into `piem-inbox-maildir-directory'. > > If prefix argument MESSAGE-ONLY is non-nil, inject just the > message for MID, not the entire thread. > @@ -599,36 +614,37 @@ (defun piem-inject-thread-into-maildir (mid &optional message-only) > (list (or (piem-mid) > (user-error "No message ID found for the current buffer")) > current-prefix-arg)) > - (cond > - ((not piem-maildir-directory) > - (user-error "`piem-maildir-directory' is not configured")) > - ((not (piem-maildir-dir-is-maildir-p piem-maildir-directory)) > - (user-error > - "`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 (piem-mid-url mid) > - (if message-only "/raw" "/t.mbox.gz"))) > - (buffer (url-retrieve-synchronously url 'silent))) [...] > - (kill-buffer buffer)))) > + (let ((maildir-directory (piem-inbox-maildir-directory))) > + (cond > + ((not maildir-directory) > + (user-error "`maildir-directory' is not configured")) Unlike the previous piem-maildir-directory, maildir-directory is local to this function, so a user that sees this message can't learn more with `C-h v' and friends. How about "No directory returned by `piem-inbox-maildir-directory'" instead? > + ((not (piem-maildir-dir-is-maildir-p maildir-directory)) > + (user-error > + "`maildir-directory' does not look like a Maildir directory")) Similar comment here. Perhaps (user-error "Does not look like a Maildir directory: %s" maildir-directory) Anyway, those are all minor comments. Looking very nice. Thanks.
On Thu, Mar 11 2021, Kyle Meyer wrote: >> diff --git a/Documentation/piem.texi b/Documentation/piem.texi >> index ebd756d..9f8779f 100644 >> --- a/Documentation/piem.texi >> +++ b/Documentation/piem.texi >> @@ -410,7 +410,8 @@ >> regular mail but are following a project via NNTP in Gnus). In this >> case, you can use the command @code{piem-inject-thread-into-maildir} to >> move the thread's messages into a local Maildir directory >> -(@code{piem-maildir-directory}). By default the command downloads the >> +(@code{piem-maildir-directory}, or the @code{:maildir} for the >> +current inbox). By default the command downloads the > > Hmm, with :maildir coming second, I think I'd interpret the priority to > be opposite of what it is. Perhaps something closer to the phrasing you > used in the commit message: > > ... into a local Maildir directory specified by the current inbox's > @code{:maildir} value in @code{piem-inboxes}, falling back to > @code{piem-maildir-directory}. Good point. >> + (let ((maildir-directory (piem-inbox-maildir-directory))) >> + (cond >> + ((not maildir-directory) >> + (user-error "`maildir-directory' is not configured")) > > Unlike the previous piem-maildir-directory, maildir-directory is local > to this function, so a user that sees this message can't learn more with > `C-h v' and friends. How about > > "No directory returned by `piem-inbox-maildir-directory'" > > instead? Sounds good to me.
Previously the user was able to configure a maildir to inject threads into, but the user might want the maildir to be different depending on which mailing list the threads was coming from. With the `:maildir` keyword, users can configure the maildir on a per-list basis. If there is not `:maildir` configured for a mailing list, it will fallback to the value of `piem-maildir-directory`. --- Changes since v1: - Update wording of docs. - Improve error message when no maildir is found. Also, I prefixed the commit summary with "piem: ". Documentation/piem.texi | 12 +++--- piem.el | 90 ++++++++++++++++++++++++----------------- 2 files changed, 60 insertions(+), 42 deletions(-) diff --git a/Documentation/piem.texi b/Documentation/piem.texi index ebd756d..0f5235c 100644 --- a/Documentation/piem.texi +++ b/Documentation/piem.texi @@ -408,11 +408,13 @@ can be inconvenient to start participating in a thread that you aren't reading in your regular MUA (e.g., if you use notmuch.el to read your regular mail but are following a project via NNTP in Gnus). In this -case, you can use the command @code{piem-inject-thread-into-maildir} to -move the thread's messages into a local Maildir directory -(@code{piem-maildir-directory}). By default the command downloads the -entire thread for the message ID associated with the current buffer. A -prefix argument restricts the download to only the message. +case, you can use the command @code{piem-inject-thread-into-maildir} +to move the thread's messages into a local Maildir directory specified +by the current inbox's @code{:maildir} value in @code{piem-inboxes}, +falling back to @code{piem-maildir-directory}. By default the command +downloads the entire thread for the message ID associated with the +current buffer. A prefix argument restricts the download to only the +message. @vindex piem-after-mail-injection-functions After the messages are injected, each function in diff --git a/piem.el b/piem.el index 56f0b54..78536f1 100644 --- a/piem.el +++ b/piem.el @@ -80,6 +80,8 @@ (defcustom piem-inboxes nil Local path of the code repository associated with the inbox. :url A URL hosting HTTPS archives. This value must end with a slash. + :maildir + A Maildir directory to inject messages into. Here's an example for the public-inbox project itself: @@ -87,7 +89,8 @@ (defcustom piem-inboxes nil :coderepo \"~/src/public-inbox/\" :address \"meta@public-inbox.org\" :listid \"meta.public-inbox.org\" - :url \"https://public-inbox.org/meta/\")" + :url \"https://public-inbox.org/meta/\" + :maildir \"~/.local/share/mail/.lists.mail.public-inbox\")" :type '(alist :key-type string :value-type (plist :value-type string))) @@ -169,8 +172,11 @@ (defcustom piem-am-read-worktree-function #'piem-am-read-worktree :type 'function) (defcustom piem-maildir-directory nil - "Inject public-inbox threads into this directory. -If non-nil, this must be an existing Maildir directory." + "Default directory to inject public-inbox threads into. + +If non-nil, this must be an existing Maildir directory. This can be +overriden on a per-list basis by using the \":maildir\" keyword in +`piem-inboxes'." :type 'string) (defcustom piem-mail-injection-skipif-predicate nil @@ -373,6 +379,15 @@ (defun piem-inbox-coderepo (&optional inbox) (when-let ((repo (piem-inbox-get :coderepo inbox))) (expand-file-name repo))) +(defun piem-inbox-maildir-directory (&optional inbox) + "Return the maildir for INBOX's entry in `piem-inboxes'. + +If INBOX is nil, use the inbox returned by `piem-inbox'. If the +INBOX doesn't have a maildir configured, return the value of +`piem-maildir-directory'." + (or (piem-inbox-get :maildir inbox) + piem-maildir-directory)) + (defun piem-inbox-by-url-match (url) "Return inbox based on matching URL against `:url'." (setq url (piem--ensure-trailing-slash url)) @@ -547,7 +562,7 @@ (defun piem-download-and-decompress (url) \f ;;;; Maildir injection -(defun piem--write-mbox-to-maildir () +(defun piem--write-mbox-to-maildir (maildir-directory) (let ((n-added 0) (n-skipped 0)) (while (and (not (eobp)) @@ -558,7 +573,7 @@ (defun piem--write-mbox-to-maildir () (point-marker))) (point-max-marker))) (basename (piem-maildir-make-uniq-maildir-id)) - (tmpfile (concat piem-maildir-directory "/tmp/" basename))) + (tmpfile (concat maildir-directory "/tmp/" basename))) (goto-char beg) (if (and (functionp piem-mail-injection-skipif-predicate) (save-excursion @@ -579,7 +594,7 @@ (defun piem--write-mbox-to-maildir () end t) (replace-match "\\1" t))) (write-region beg end tmpfile nil nil nil 'excl) - (piem-maildir-move-tmp-to-new piem-maildir-directory + (piem-maildir-move-tmp-to-new maildir-directory basename) (delete-file tmpfile) (cl-incf n-added)) @@ -588,7 +603,7 @@ (defun piem--write-mbox-to-maildir () ;;;###autoload (defun piem-inject-thread-into-maildir (mid &optional message-only) - "Inject thread containing MID into `piem-maildir-directory'. + "Inject thread containing MID into `piem-inbox-maildir-directory'. If prefix argument MESSAGE-ONLY is non-nil, inject just the message for MID, not the entire thread. @@ -599,36 +614,37 @@ (defun piem-inject-thread-into-maildir (mid &optional message-only) (list (or (piem-mid) (user-error "No message ID found for the current buffer")) current-prefix-arg)) - (cond - ((not piem-maildir-directory) - (user-error "`piem-maildir-directory' is not configured")) - ((not (piem-maildir-dir-is-maildir-p piem-maildir-directory)) - (user-error - "`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 (piem-mid-url mid) - (if message-only "/raw" "/t.mbox.gz"))) - (buffer (url-retrieve-synchronously url 'silent))) - (unwind-protect - (with-current-buffer buffer - (if (/= url-http-response-status 200) - (error "Download of %s failed" url) - (piem--url-remove-header) - (unless message-only - (piem--url-decompress)) - (pcase-let ((`(,added-count . ,skipped-count) - (piem--write-mbox-to-maildir))) - (message "Added %d message%s%s for %s to %s" - added-count - (if (= added-count 1) "" "s") - (if (> skipped-count 0) - (format " (skipping %d)" skipped-count) - "") - mid - (abbreviate-file-name piem-maildir-directory))) - (run-hook-with-args 'piem-after-mail-injection-functions mid))) - (kill-buffer buffer)))) + (let ((maildir-directory (piem-inbox-maildir-directory))) + (cond + ((not maildir-directory) + (user-error "No directory returned by `piem-inbox-maildir-directory'")) + ((not (piem-maildir-dir-is-maildir-p maildir-directory)) + (user-error + "Does not look like a Maildir directory: %s" maildir-directory)) + ((not (or message-only (piem-check-gunzip))) + (user-error "gunzip executable not found"))) + (when-let ((url (concat (piem-mid-url mid) + (if message-only "/raw" "/t.mbox.gz"))) + (buffer (url-retrieve-synchronously url 'silent))) + (unwind-protect + (with-current-buffer buffer + (if (/= url-http-response-status 200) + (error "Download of %s failed" url) + (piem--url-remove-header) + (unless message-only + (piem--url-decompress)) + (pcase-let ((`(,added-count . ,skipped-count) + (piem--write-mbox-to-maildir maildir-directory))) + (message "Added %d message%s%s for %s to %s" + added-count + (if (= added-count 1) "" "s") + (if (> skipped-count 0) + (format " (skipping %d)" skipped-count) + "") + mid + (abbreviate-file-name maildir-directory))) + (run-hook-with-args 'piem-after-mail-injection-functions mid))) + (kill-buffer buffer))))) \f ;;;; Patch handling base-commit: 26c8103eaa9df3ebaf873a7cd477efaa48cec140 -- 2.30.2
Thanks, pushed (2333819).