From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:267::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms12 with LMTPS id wP3fBxqrSmBgPgAAsNZ9tg (envelope-from ) for ; Thu, 11 Mar 2021 23:43:22 +0000 Received: from out0.migadu.com ([2001:41d0:2:267::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id AJAJORWrSmBgbgAA1q6Kng (envelope-from ) for ; Thu, 11 Mar 2021 23:43:17 +0000 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kyleam.com; s=key1; t=1615506197; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BOZhUOabSrtCNzen3DQzlbxwCsonkMwfu2LSfL5qRDA=; b=r3jyXjIhkBD4DuwFm1go7IjJuLTcfose2C79EY/S0ZElhmY61bHjByV9oQWkAtfJLqzu7Q m7yj/78FAbX0S/XKDcQw1uGBU1gj7tko9BSNIuKlbcz7mpmPuDhciJYVR0uUXRreyt53v3 LgQwxFRd2p8ENHDyzxw4M4UPvbHgn5UnOtm9s8sCtuKsl/ZDaW203O8DI1BfHrdep6BHR2 AsQj/bTwEuyOD8DON9pO4XXYboJW6ReoTu1L0rzba5978z2tSjnb2fxK7D7QXTaIeGw+/g 6hzaq0M0Yg2pfY6ICadHwVfrtN5j+/SXJVn0ZYMUHrUYMPQJ8oLCgw4yB7eY2A== From: Kyle Meyer To: Xinglu Chen Cc: piem@inbox.kyleam.com Subject: Re: [RFC PATCH] Add :maildir keyword to piem-inboxes In-Reply-To: <9ff4c94928b0cd0d223799fa8a978d94e545522e.1615460587.git.public@yoctocell.xyz> References: <9ff4c94928b0cd0d223799fa8a978d94e545522e.1615460587.git.public@yoctocell.xyz> Date: Thu, 11 Mar 2021 18:43:14 -0500 Message-ID: <87im5x9rzh.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: kyle@kyleam.com X-TUID: bnVmXeJy9R0r 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.