[PATCH] piem-am: Order attached patches by file name prefix

[Kyle Meyer <kyle@kyleam.com>]

Ihor Radchenko writes:

> I just tried to use piem for patches in
> https://orgmode.org/list/87v8s8n1bm.fsf@heagren.com
>
> The patches are supposed to be applied in specific order 1/2 and then
> 2/2. However, the patches in the email are attached in the opposite
> order.
>
> piem-am on the above email disregards the patch order and simply applies
> the patches as they appear in the email attachments, which is not what
> the patch author intended.
>
> I consider the current behavior a bug.

Okay, though it's at least an intended bug :)  It is not particularly
user-facing, but piem is documented as taking the attachments in order
(although reading the wording now, it's probably a bit ambiguous).  The
sender attached the patches in the wrong order, forcing those reading it
and applying it manually to work from bottom to top.

Regardless of what you call it, I don't mind piem trying to be more
clever/helpful in these scenarios.  How about something like this?
(Tested on <87v8s8n1bm.fsf@heagren.com> from Notmuch and Gnus.)

-- >8 --
Subject: [PATCH] piem-am: Order attached patches by file name prefix

When extracting patches from attachments, piem-gnus-am-ready-mbox and
piem-notmuch-am-ready-mbox construct the mbox messages in the same
order as the attachments.  This depends on the sender attaching the
patches in ascending order.

Be a bit more helpful in situations where the sender attaches the
patches out of order by reordering patches according to the NNNN-
prefix that git-format-patch adds to the start of the patch file name.
This approach won't be able to reliably sort patches that aren't
generated by git-format-patch, but that's outside of any use case that
piem is intended to support.

Suggested-by: Ihor Radchenko <yantar92@gmail.com>
Link: https://inbox.kyleam.com/piem/87mtdj9dzt.fsf@localhost
---
 piem-gnus.el    | 13 ++++++++++---
 piem-notmuch.el | 15 +++++++++++----
 piem.el         | 26 +++++++++++++++-----------
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/piem-gnus.el b/piem-gnus.el
index c62a2e6e..4011fa2d 100644
--- a/piem-gnus.el
+++ b/piem-gnus.el
@@ -91,17 +91,24 @@ (defun piem-gnus-mid-to-thread (mid)
 
 (defun piem-gnus-am-ready-mbox ()
   "Return a function that inserts an am-ready mbox.
+
 If the buffer has any MIME parts that look like a patch, use
-those parts' contents (in order) as the mbox.  Otherwise, use the
-message itself if it looks like a patch."
+those parts' contents as the mbox, ordering the patches based on
+the number at the start of the file name.  If none of the file
+names start with a number, retain the original order of the
+attachments.
+
+If no MIME parts look like a patch, use the message itself if it
+looks like a patch."
   (when (derived-mode-p 'gnus-article-mode 'gnus-summary-mode)
     (cond
      (gnus-article-mime-handles
       (when-let ((patches (delq nil (mapcar #'piem-am-extract-attached-patch
                                             gnus-article-mime-handles))))
+        (setq patches (sort patches (lambda (x y) (< (car x) (car y)))))
         (cons (lambda ()
                 (dolist (patch patches)
-                  (insert patch)))
+                  (insert (cdr patch))))
               "mbox")))
      (gnus-article-buffer
       (when-let ((patch (with-current-buffer gnus-article-buffer
diff --git a/piem-notmuch.el b/piem-notmuch.el
index 41f2793a..493b7249 100644
--- a/piem-notmuch.el
+++ b/piem-notmuch.el
@@ -85,9 +85,15 @@ (defun piem-notmuch-mid-to-thread (mid)
 
 (defun piem-notmuch-am-ready-mbox ()
   "Return a function that inserts an am-ready mbox.
+
 If the buffer has any MIME parts that look like a patch, use
-those parts' contents (in order) as the mbox.  Otherwise, use the
-message itself if it looks like a patch."
+those parts' contents as the mbox, ordering the patches based on
+the number at the start of the file name.  If none of the file
+names start with a number, retain the original order of the
+attachments.
+
+If no MIME parts look like a patch, use the message itself if it
+looks like a patch."
   (when (derived-mode-p 'notmuch-show-mode)
     (let* ((handle (piem-notmuch--with-current-message
                      (mm-dissect-buffer)))
@@ -106,10 +112,11 @@ (defun piem-notmuch-am-ready-mbox ()
              (push patch patches)))
          handle)
         (when patches
-          (setq patches (nreverse patches))
+          (setq patches (sort (nreverse patches)
+                              (lambda (x y) (< (car x) (car y)))))
           (cons (lambda ()
                   (dolist (patch patches)
-                    (insert patch)))
+                    (insert (cdr patch))))
                 "mbox"))))))
 
 (defun piem-notmuch-extract-patch-am-ready-mbox ()
diff --git a/piem.el b/piem.el
index 47944e40..63c358a2 100644
--- a/piem.el
+++ b/piem.el
@@ -817,17 +817,21 @@ (defun piem-inject-thread-into-maildir (mid &optional inbox message-only)
 ;;;; Patch handling
 
 (defun piem-am-extract-attached-patch (handle)
-  "Return content for HANDLE if it looks like a patch."
-  (and (listp handle)
-       (let ((type (mm-handle-media-type handle))
-             (filename (mm-handle-filename handle)))
-         (or (member type '("text/x-diff" "text/x-patch"))
-             (and filename
-                  (equal type "text/plain")
-                  (string-suffix-p ".patch" filename t))))
-       (with-temp-buffer
-         (mm-display-inline handle)
-         (buffer-substring-no-properties (point-min) (point-max)))))
+  "Get the content for HANDLE if it looks like a patch.
+The return value is of the form (N . CONTENT), where N is the
+number at the start of the file name."
+  (when (listp handle)
+    (let ((type (mm-handle-media-type handle))
+          (filename (mm-handle-filename handle)))
+      (and (or (member type '("text/x-diff" "text/x-patch"))
+               (and filename
+                    (equal type "text/plain")
+                    (string-suffix-p ".patch" filename t)))
+           (with-temp-buffer
+             (mm-display-inline handle)
+             (cons
+              (string-to-number filename)
+              (buffer-substring-no-properties (point-min) (point-max))))))))
 
 (defun piem-extract-mbox-info (&optional buffer)
   "Collect information from message in BUFFER.

base-commit: 67411fadab680b01d9027a3f4833ef8892377874
-- 
2.36.1