From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kyle@kyleam.com>
Received: from mp1 ([2001:41d0:2:863f::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by ms12 with LMTPS
	id qFsiOVR6uWH4fgAAsNZ9tg
	(envelope-from <kyle@kyleam.com>)
	for <larch@inbox.kyleam.com>; Wed, 15 Dec 2021 05:17:08 +0000
Received: from out1.migadu.com ([2001:41d0:2:863f::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by mp1 with LMTPS
	id aP7wNFR6uWHRUAAAbx9fmQ
	(envelope-from <kyle@kyleam.com>)
	for <larch@inbox.kyleam.com>; Wed, 15 Dec 2021 05:17:08 +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=1639545428;
	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:
	 content-transfer-encoding:content-transfer-encoding:
	 in-reply-to:in-reply-to:references:references;
	bh=fmp/RlvogBUr3F0uOcVCahdWHUpOKpdF8IXJZhH263o=;
	b=Qu4Ov/K7txTegvgZa8AwtBGbK5/0hIKonxiwEKvAO6dMLx50tfbEThHif4zXL7I5YRSWn7
	aX3jcRPV7atIdeqDxjd5CsyvYXBiI/LCHX6Yc2U0rFXWTzUcSoQ8UCReYRoh0DF1YB8i4T
	StucvJINTo0JQN3neI/cEXaGSph52DHmia6S/YI9BZHpJEzIH6bnURpMXKrHm/7DptwBpC
	aznwTo7S2A5WIHENPRDea3VxFWJkD4dXjKVuBUW5kl5f6GKN72HIgbPL9c/UJkQSQm7PQI
	APiXVvLk23rnTL3kqy02q+V3KBEuRibJumCNwkd3+qtFaaMjPBqmwe5S5cv3QQ==
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
In-Reply-To: <20211214150959.624-1-leo@relevant-information.com>
References: <20211214150959.624-1-leo@relevant-information.com>
Date: Wed, 15 Dec 2021 00:16:53 -0500
Message-ID: <87y24mwipm.fsf@kyleam.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Migadu-Flow: FLOW_OUT
X-Migadu-Auth-User: kyleam.com
X-TUID: KnHnrAfoLkbc

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=
=20
> added clarification on where the copied comment came from.  I'm not sure=
=20
> how the copyright notice should be formatted though.=20

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 (=3D 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
=20=20
  In toplevel form:
  piem-notmuch.el:109:1:Warning: Unused lexical variable =E2=80=98id=E2=80=
=99
  piem-notmuch.el:109:1:Warning: Unused lexical variable =E2=80=98patches=
=E2=80=99

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

  (when (and (=3D 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-subjec=
t))
             (=3D (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.