discussion and development of piem
 help / color / mirror / code / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: leo@relevant-information.com
Cc: piem@inbox.kyleam.com
Subject: Re: [PATCH v2] Use notmuch-extract-patch if available
Date: Wed, 15 Dec 2021 00:16:53 -0500	[thread overview]
Message-ID: <87y24mwipm.fsf@kyleam.com> (raw)
In-Reply-To: <20211214150959.624-1-leo@relevant-information.com>

Thanks for the update.

leo@relevant-information.com writes:

> From: Leo <sourcehut@relevant-information.com>

I think the patch subject needs an update for the current approach where
notmuch-extract-patch isn't used by default.  Perhaps something like

  Support preparing am-ready mbox via notmuch-extract-patch

?

> notmuch-extract-patch is a command line tool from the elpa-mailscripts
> package that extracts patches from a thread.  It's a useful way to
> extract the latest patch series from an email thread and filter out the
> replies and reviews.
> ---
> I've extracted the functionality into its own function as requested, and 
> added clarification on where the copied comment came from.  I'm not sure 
> how the copyright notice should be formatted though. 

Thanks.  For the copyright, please add it on its own line (i.e. repeat
";; Copyright (C) ...").

> +(defun piem-notmuch-extract-patch-am-ready-mbox ()
> +  "Return a function that inserts an am-ready mbox.
> +Use the message itself if it looks like a patch using
> +notmuch-extract-patch to get the latest patch series from the
> +notmuch thread."
> +  (when (derived-mode-p 'notmuch-show-mode)
> +    (let* ((handle (piem-notmuch--with-current-message
> +                     (mm-dissect-buffer)))
> +           (n-attachments (notmuch-count-attachments handle))
> +           patches)
> +      (when (= n-attachments 0)
> +          (when (string-match-p piem-patch-subject-re
> +                                (notmuch-show-get-subject))
> +            (let ((id (notmuch-show-get-message-id))
> +                  (thread-id notmuch-show-thread-id))

The `patches' and `id' bindings are unused and should be removed:

  $ make compile
  emacs --batch -Q -L . -L tests -f batch-byte-compile piem-notmuch.el
  
  In toplevel form:
  piem-notmuch.el:109:1:Warning: Unused lexical variable ‘id’
  piem-notmuch.el:109:1:Warning: Unused lexical variable ‘patches’

A purely cosmetic comment: you could compress the (when ... (when ...))
to

  (when (and (= n-attachments 0)
             (string-match-p piem-patch-subject-re
                             (notmuch-show-get-subject)))
   ....)

Or, rolling it all the way up to the top (and taking advantage of the
fact that, unlike in piem-notmuch-am-ready-mbox, `handle' isn't used
here):

  (when (and (derived-mode-p 'notmuch-show-mode)
             (string-match-p piem-patch-subject-re (notmuch-show-get-subject))
             (= (notmuch-count-attachments
                 (piem-notmuch--with-current-message
                   (mm-dissect-buffer)))
                0))
    ...)

Anyway, that's dealer's choice.

With the next iteration, I think this should be good to apply, adjusting
later as needed.  Once things sit for a bit, I can extend the
documentation to mention it.

  reply	other threads:[~2021-12-15  5:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-14 15:09 [PATCH v2] Use notmuch-extract-patch if available leo
2021-12-15  5:16 ` Kyle Meyer [this message]
2021-12-15  5:22   ` Kyle Meyer
2021-12-16 19:32   ` [PATCH v3 1/2] Support preparing am-ready mbox via notmuch-extract-patch sourcehut
2021-12-16 19:32     ` [PATCH v3 2/2] Add user option for specifying path to notmuch-extract-patch sourcehut
2021-12-17  5:22       ` Kyle Meyer
2021-12-21 19:15         ` [PATCH v4] " Leo
2021-12-24 18:31           ` Kyle Meyer
2021-12-17  5:15     ` [PATCH v3 1/2] Support preparing am-ready mbox via notmuch-extract-patch 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=87y24mwipm.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=leo@relevant-information.com \
    --cc=piem@inbox.kyleam.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).