Could not find am-ready mbox for current buffer

Could not find am-ready mbox for current buffer

[Leo]

Hello,

This is really just for future reference, in case someone else stumbles
upon this.  I've resolved it for myself and am just documenting what the
issue was.


I found that for a specific email with a patch piem would just refuse to
apply it with the message "Could not find am-ready mbox for current
buffer", even if just piping the email through `git am` would work.
Turns out the issue was with piem-patch-subject-re.  The patch was sent
with a different subject prefix (the --subject-prefix option in
git-send-email) than the default, which didn't match
piem-patch-subject-re.

For example: 

> Subject: [RFC] add some code

So if you, like me, just can't figure out why piem is angry with just
this specific patch, consider adjusting that variable.

Cheers,

Leo

[PATCH] *-am-ready-mbox: Don't require PATCH prefix

[Kyle Meyer]

Leo writes:

> This is really just for future reference, in case someone else stumbles
> upon this.  I've resolved it for myself and am just documenting what the
> issue was.

Thanks.

> I found that for a specific email with a patch piem would just refuse to
> apply it with the message "Could not find am-ready mbox for current
> buffer", even if just piping the email through `git am` would work.
> Turns out the issue was with piem-patch-subject-re.  The patch was sent
> with a different subject prefix (the --subject-prefix option in
> git-send-email) than the default, which didn't match
> piem-patch-subject-re.

It looks like piem-patch-subject-re is used in piem--shorten-subject and
*-am-ready-mbox functions:

  $ grep -p piem-patch-subject-re
  piem-gnus.el=(defun piem-gnus-am-ready-mbox ()
  piem-gnus.el:                                  piem-patch-subject-re
  piem-notmuch.el=(defun piem-notmuch-am-ready-mbox ()
  piem-notmuch.el:          (when (string-match-p piem-patch-subject-re
  piem-notmuch.el=(defun piem-notmuch-extract-patch-am-ready-mbox ()
  piem-notmuch.el:             (string-match-p piem-patch-subject-re
  piem.el=(defun piem-extract-mbox-info (&optional buffer)
  piem.el:(defvar piem-patch-subject-re
  piem.el=(defun piem--shorten-subject (subject)
  piem.el:                     piem-patch-subject-re ""))

Perhaps it was a mistake to add the piem-patch-subject-re guard to the
*-am-ready-mbox functions.  If we know we're in the right buffer type
and the caller is asking to extract the am-ready-mbox from the current
buffer, is there any value being strict about the subject?  I'm thinking
of dropping the guards like so...

-- >8 --
Subject: [PATCH] *-am-ready-mbox: Don't require PATCH prefix

All of the *-am-ready-mbox functions expect inline patches to have a
'[... PATCH ...]' in the subject.  In cases where a patch is sent
without that prefix, a piem-am caller gets an unhelpful "Could not
find am-ready mbox for current buffer" in response to
piem-am-ready-mbox returning nil.

To support these (hopefully rare) cases, drop the "has patch prefix?"
guard from all the *-am-ready-mbox functions.  A caller is requesting
to extract a patch from the current buffer, and the "is the current
buffer in the right mode?" check should be sufficient for an
am-ready-mbox function to decide whether it should be responsible for
generating the mbox.

This change makes it more likely that a piem-am caller invokes 'git
am' on something that's not a valid patch, but in that case 1) the
output buffer gives a reasonable error and 2) no other function in
piem-am-ready-mbox-functions could be expected to successfully apply
it, so there's no harm in claiming it.
---
 piem-gnus.el    |  8 ++------
 piem-notmuch.el | 12 ++++--------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/piem-gnus.el b/piem-gnus.el
index 7120512..b6dc672 100644
--- a/piem-gnus.el
+++ b/piem-gnus.el
@@ -130,12 +130,8 @@ (defun piem-gnus-am-ready-mbox ()
       (when-let ((patch (with-current-buffer gnus-article-buffer
                           (save-restriction
                             (widen)
-                            (and (string-match-p
-                                  piem-patch-subject-re
-                                  (mail-decode-encoded-word-string
-                                   (message-field-value "subject")))
-                                 (buffer-substring-no-properties
-                                  (point-min) (point-max)))))))
+                            (buffer-substring-no-properties
+                             (point-min) (point-max))))))
         (cons (lambda () (insert patch))
               "mbox"))))))
 
diff --git a/piem-notmuch.el b/piem-notmuch.el
index 493b724..9077ec8 100644
--- a/piem-notmuch.el
+++ b/piem-notmuch.el
@@ -100,12 +100,10 @@ (defun piem-notmuch-am-ready-mbox ()
            (n-attachments (notmuch-count-attachments handle))
            patches)
       (if (= n-attachments 0)
-          (when (string-match-p piem-patch-subject-re
-                                (notmuch-show-get-subject))
-            (let ((id (notmuch-show-get-message-id)))
-              (lambda ()
-                (call-process notmuch-command nil t nil
-                              "show" "--format=mbox" id))))
+          (let ((id (notmuch-show-get-message-id)))
+            (lambda ()
+              (call-process notmuch-command nil t nil
+                            "show" "--format=mbox" id)))
         (notmuch-foreach-mime-part
          (lambda (p)
            (when-let ((patch (piem-am-extract-attached-patch p)))
@@ -125,8 +123,6 @@ (defun piem-notmuch-extract-patch-am-ready-mbox ()
 notmuch-extract-patch to get the latest patch series from the
 notmuch thread."
   (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)))

base-commit: 4256eb8b62f81f1f755184bbe245b3155f723a5a
-- 
2.41.0

Re: [PATCH] *-am-ready-mbox: Don't require PATCH prefix

[Kyle Meyer]

Kyle Meyer writes:

> Subject: [PATCH] *-am-ready-mbox: Don't require PATCH prefix
> [...]

I meant to add a Reported-by trailer; will fix on apply (unless reroll
is needed for other reasons).

> ---
>  piem-gnus.el    |  8 ++------
>  piem-notmuch.el | 12 ++++--------
>  2 files changed, 6 insertions(+), 14 deletions(-)

Re: [PATCH] *-am-ready-mbox: Don't require PATCH prefix

[Kyle Meyer]

[ adding piem inbox back to cc  ]

Leo writes:

> Kyle Meyer <kyle@kyleam.com> writes:
>>
>> This change makes it more likely that a piem-am caller invokes 'git
>> am' on something that's not a valid patch, but in that case 1) the
>> output buffer gives a reasonable error
>>
> The only comment I have is that running piem-am with this patch on a
> non-patch buffer (and selecting a repo to apply it on) will create a new
> branch and put git in a state of failing to apply a patch.  You need to
> clean it up by deleting the branch and running `git am --abort` in the
> repository.  This seems reasonable to me.  Both are easy to do, and is
> what you would expect to happen if you run `git-am` with a broken patch
> (well not really the branch, but I think that's fine).

Right, that is worth spelling out.  Thanks.

Re: [PATCH] *-am-ready-mbox: Don't require PATCH prefix

[Kyle Meyer]

Kyle Meyer writes:

> Leo writes:
>
>> The only comment I have is that running piem-am with this patch on a
>> non-patch buffer (and selecting a repo to apply it on) will create a new
>> branch and put git in a state of failing to apply a patch.  You need to
>> clean it up by deleting the branch and running `git am --abort` in the
>> repository.  This seems reasonable to me.  Both are easy to do, and is
>> what you would expect to happen if you run `git-am` with a broken patch
>> (well not really the branch, but I think that's fine).
>
> Right, that is worth spelling out.  Thanks.

Pushed (8abe70c) with a revised commit message.