From: Kyle Meyer <kyle@kyleam.com> To: Xinglu Chen <public@yoctocell.xyz> Cc: piem@inbox.kyleam.com Subject: Re: [RFC PATCH] Add :maildir keyword to piem-inboxes Date: Thu, 11 Mar 2021 18:43:14 -0500 [thread overview] Message-ID: <87im5x9rzh.fsf@kyleam.com> (raw) In-Reply-To: <9ff4c94928b0cd0d223799fa8a978d94e545522e.1615460587.git.public@yoctocell.xyz> 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.
next prev parent reply other threads:[~2021-03-11 23:43 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-11 11:06 Xinglu Chen 2021-03-11 23:43 ` Kyle Meyer [this message] 2021-03-12 16:43 ` Xinglu Chen 2021-03-12 16:55 ` [PATCH v2] piem: " Xinglu Chen 2021-03-13 1:18 ` Kyle Meyer
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://git.kyleam.com/ * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87im5x9rzh.fsf@kyleam.com \ --to=kyle@kyleam.com \ --cc=piem@inbox.kyleam.com \ --cc=public@yoctocell.xyz \ --subject='Re: [RFC PATCH] Add :maildir keyword to piem-inboxes' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://git.kyleam.com/piem/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).