[RFC PATCH 00/14] am, b4-am: Add commands to extend an existing branch

[RFC PATCH 00/14] am, b4-am: Add commands to extend an existing branch

[Kyle Meyer]

Every now and then I think that it'd be nice to have commands that
extend an existing branch rather than create a new one.  But then I
end up concluding that most of the time I really do want to create a
new branch; for the other times, it's pretty easy to instead apply to
a detached head and then merge that into the target branch (e.g., with
magit-merge-into).

I had the thought again recently and decided to see how going in this
direction looked.  The result is this somewhat raw series (at the very
least, names and key bindings probably need more thought, and the
manual needs an update).  Patches 7, 12, 13, and 14 are the main
user-facing changes.

I'm still undecided about the overall direction at this point, though
I plan to at least extract and modify some patches from this series.
I have the feeling that the piem-am functionality is bound to be moved
under a transient at some point to give provide real estate for new
functionality.  That might be a good reason to take some of the prep
patches, though that shouldn't be the sole reason for adding these new
"extend existing branch" variants.

Note that, in piem's pre-1.0 release state, I am not considering
backwards compatibility as a reason to avoid these changes.

  [ 1/14] am: Give better name to default piem-am-read-worktree-function value
  [ 2/14] am: Add dedicated function for reading worktree
  [ 3/14] am: Extract git-am logic to dedicated function
  [ 4/14] am: Add function for reading piem-am's arguments
  [ 5/14] am: Add comment header for patch editing subsection
  [ 6/14] edit patch: Inject values via interactive arguments
  [ 7/14] am, b4-am: Rename piem-am to piem-am-create
  [ 8/14] am, b4-am: Rewrite -create docstrings to emphasize branch creation
  [ 9/14] piem-am-create: Move info argument to the end
  [10/14] piem-am--arguments: Make info extraction optional
  [11/14] b4: Move logic for checking arguments to a dedicated function
  [12/14] am, b4-am: Add commands that extend an existing branch
  [13/14] am: Move functionality under a dedicated transient
  [14/14] b4: Add piem-b4-am-from-mid-extend to transient

 Documentation/piem.texi |  51 +++++++------
 piem-b4.el              |  65 ++++++++++------
 piem.el                 | 159 ++++++++++++++++++++++++++--------------
 3 files changed, 174 insertions(+), 101 deletions(-)


base-commit: afa9e05e5bb42d88b0c5cf79bdcb8fbd14fdd800
-- 
2.34.0

[RFC PATCH 01/14] am: Give better name to default piem-am-read-worktree-function value

[Kyle Meyer]

piem-am-read-worktree isn't a good name because it provide any
information to distinguish this function from any other function used
for piem-am-read-worktree-function.
---
 piem.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/piem.el b/piem.el
index b839942e..9e5b0622 100644
--- a/piem.el
+++ b/piem.el
@@ -192,7 +192,8 @@ (defcustom piem-am-create-worktree nil
   "Whether to create a dedicated worktree for applying patches."
   :type 'boolean)
 
-(defcustom piem-am-read-worktree-function #'piem-am-read-worktree
+(defcustom piem-am-read-worktree-function
+  #'piem-am-read-worktree-sibling-named-by-branch
   "Function that reads a to-be-created worktree from the user.
 This function is called with two arguments, the directory of the
 code repository that the worktree will be created from and the
@@ -890,7 +891,7 @@ (defun piem-name-branch-who-what-v (info)
 
 (defvar piem-am-args (list "--scissors" "--3way"))
 
-(defun piem-am-read-worktree (coderepo branch)
+(defun piem-am-read-worktree-sibling-named-by-branch (coderepo branch)
   "Read a worktree to create for applying patches.
 This function is intended to be used as a value of
 `piem-am-read-worktree-function'.  The worktree directory is
-- 
2.34.0

Re: [RFC PATCH 01/14] am: Give better name to default piem-am-read-worktree-function value

[Kyle Meyer]

Kyle Meyer writes:

> piem-am-read-worktree isn't a good name because it provide any

                                                    ^ doesn't

> information to distinguish this function from any other function used
> for piem-am-read-worktree-function.
> ---
>  piem.el | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Pushed (f55a49ca).

[RFC PATCH 02/14] am: Add dedicated function for reading worktree

[Kyle Meyer]

piem-am calls the value of the variable piem-am-read-worktree-function
and does a bit of error handling outside.  Another command will need
to do the same, so extract the logic to a dedicated function.
---
 piem.el | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/piem.el b/piem.el
index 9e5b0622..ac81c217 100644
--- a/piem.el
+++ b/piem.el
@@ -905,6 +905,14 @@ (defun piem-am-read-worktree-sibling-named-by-branch (coderepo branch)
           (concat (file-name-nondirectory fname) "-"
                   (replace-regexp-in-string "/" "-" branch))))))
 
+(defun piem-am--read-worktree (coderepo branch)
+  (let ((dir (expand-file-name
+              (funcall piem-am-read-worktree-function
+                       coderepo branch))))
+    (when (file-exists-p dir)
+      (user-error "Worktree directory already exists"))
+    dir))
+
 ;;;###autoload
 (defun piem-am (mbox &optional format info coderepo toggle-worktree)
   "Feed an am-ready mbox to `git am'.
@@ -958,11 +966,7 @@ (defun piem-am (mbox &optional format info coderepo toggle-worktree)
                    (if base (cons base cands) cands)))))
       (when use-worktree
         (setq am-directory
-              (expand-file-name
-               (funcall piem-am-read-worktree-function
-                        default-directory new-branch)))
-        (when (file-exists-p am-directory)
-          (user-error "Worktree directory already exists")))
+              (piem-am--read-worktree default-directory new-branch)))
       (apply #'piem-process-call nil piem-git-executable
              (append (if use-worktree
                          (list "worktree" "add")
-- 
2.34.0

[RFC PATCH 03/14] am: Extract git-am logic to dedicated function

[Kyle Meyer]

Another command will need the same code.
---
 piem.el | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/piem.el b/piem.el
index ac81c217..4b0457a2 100644
--- a/piem.el
+++ b/piem.el
@@ -913,6 +913,26 @@ (defun piem-am--read-worktree (coderepo branch)
       (user-error "Worktree directory already exists"))
     dir))
 
+(defun piem--git-am (mbox format dir)
+  (setq dir (or dir default-directory))
+  (let ((interactivep (eq (car-safe mbox) :interactive))
+        (args (cons (concat "--patch-format=" (or format "mboxrd"))
+                    piem-am-args)))
+    (when interactivep
+      (setq mbox (cdr mbox)))
+    (if (bufferp mbox)
+        (unwind-protect
+            (apply #'piem-process-call-with-buffer-input
+                   dir mbox piem-git-executable "am" args)
+          (when interactivep
+            (kill-buffer mbox)))
+      (apply #'piem-process-call dir piem-git-executable "am"
+             (append args (list mbox)))))
+  (if (and piem-use-magit
+           (fboundp 'magit-status-setup-buffer))
+      (magit-status-setup-buffer dir)
+    (dired dir)))
+
 ;;;###autoload
 (defun piem-am (mbox &optional format info coderepo toggle-worktree)
   "Feed an am-ready mbox to `git am'.
@@ -975,20 +995,7 @@ (defun piem-am (mbox &optional format info coderepo toggle-worktree)
                      (and use-worktree (list am-directory))
                      (and (not (string-blank-p base))
                           (list base)))))
-    (let ((args (cons (concat "--patch-format=" format)
-                      piem-am-args)))
-      (if (bufferp mbox)
-          (unwind-protect
-              (apply #'piem-process-call-with-buffer-input
-                     am-directory mbox piem-git-executable "am" args)
-            (when interactivep
-              (kill-buffer mbox)))
-        (apply #'piem-process-call am-directory piem-git-executable "am"
-               (append args (list mbox)))))
-    (if (and piem-use-magit
-             (fboundp 'magit-status-setup-buffer))
-        (magit-status-setup-buffer am-directory)
-      (dired am-directory))))
+    (piem--git-am mbox format am-directory)))
 
 (defvar-local piem-edit-patch--coderepo nil)
 (defvar-local piem-edit-patch--format nil)
-- 
2.34.0

[RFC PATCH 04/14] am: Add function for reading piem-am's arguments

[Kyle Meyer]

Another command will need to do very similar processing.
---
 piem.el | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/piem.el b/piem.el
index 4b0457a2..dc93f89d 100644
--- a/piem.el
+++ b/piem.el
@@ -933,6 +933,20 @@ (defun piem--git-am (mbox format dir)
       (magit-status-setup-buffer dir)
     (dired dir)))
 
+(defun piem-am--arguments ()
+  (pcase-let ((`(,mbox . ,format)
+               (or (piem-am-ready-mbox)
+                   (user-error
+                    "Could not find am-ready mbox for current buffer"))))
+    ;; We're responsible for cleaning up the buffer created by
+    ;; `piem-am-ready-mbox'; sneak in an indication to let the
+    ;; downstream code know.
+    (list (cons :interactive mbox)
+          format
+          (piem-extract-mbox-info mbox)
+          (piem-inbox-coderepo-maybe-read)
+          current-prefix-arg)))
+
 ;;;###autoload
 (defun piem-am (mbox &optional format info coderepo toggle-worktree)
   "Feed an am-ready mbox to `git am'.
@@ -953,26 +967,10 @@ (defun piem-am (mbox &optional format info coderepo toggle-worktree)
 When prefix argument TOGGLE-WORKTREE is non-nil, invert the
 meaning of `piem-am-create-worktree'.  With the default value,
 this triggers the creation of a new worktree."
-  (interactive
-   (pcase-let ((`(,mbox . ,format)
-                (or (piem-am-ready-mbox)
-                    (user-error
-                     "Could not find am-ready mbox for current buffer"))))
-     ;; We're responsible for cleaning up the buffer created by
-     ;; `piem-am-ready-mbox'; sneak in an indication to let the
-     ;; downstream code know.
-     (list (cons :interactive mbox)
-           format
-           (piem-extract-mbox-info mbox)
-           (piem-inbox-coderepo-maybe-read)
-           current-prefix-arg)))
-  (setq format (or format "mboxrd"))
+  (interactive (piem-am--arguments))
   (let* ((default-directory (or coderepo default-directory))
          (am-directory default-directory)
-         (use-worktree (xor piem-am-create-worktree toggle-worktree))
-         (interactivep (eq (car-safe mbox) :interactive)))
-    (when interactivep
-      (setq mbox (cdr mbox)))
+         (use-worktree (xor piem-am-create-worktree toggle-worktree)))
     (let ((new-branch
            (let ((b (read-string "New branch (empty for detached): "
                                  (funcall piem-default-branch-function info))))
-- 
2.34.0

[RFC PATCH 05/14] am: Add comment header for patch editing subsection

[Kyle Meyer]

---
 piem.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/piem.el b/piem.el
index dc93f89d..6241cb1f 100644
--- a/piem.el
+++ b/piem.el
@@ -995,6 +995,9 @@ (defun piem-am (mbox &optional format info coderepo toggle-worktree)
                           (list base)))))
     (piem--git-am mbox format am-directory)))
 
+\f
+;;;;; Patch editing
+
 (defvar-local piem-edit-patch--coderepo nil)
 (defvar-local piem-edit-patch--format nil)
 
-- 
2.34.0

[RFC PATCH 06/14] edit patch: Inject values via interactive arguments

[Kyle Meyer]

piem-edit stores values in buffer-local variables that
piem-edit-patch-am then uses in a non-interactive call to piem-am.
That works, but the caller doesn't have access to piem-am's
toggle-worktree argument.  And this functionality gap will grow bigger
when an upcoming commit moves transition piem-am to a transient,
moving the current piem-am to a suffix.

Instead teach piem-am--arguments how to pull the mbox and coderepo
from these buffer-local values so that piem-edit-patch-am can just
call piem-am interactively.
---
 piem.el | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/piem.el b/piem.el
index 6241cb1f..eea81a8e 100644
--- a/piem.el
+++ b/piem.el
@@ -933,9 +933,14 @@ (defun piem--git-am (mbox format dir)
       (magit-status-setup-buffer dir)
     (dired dir)))
 
+(defvar-local piem-edit-patch--coderepo nil)
+(defvar-local piem-edit-patch--format nil)
+
 (defun piem-am--arguments ()
   (pcase-let ((`(,mbox . ,format)
-               (or (piem-am-ready-mbox)
+               (or (and (derived-mode-p 'piem-edit-patch-mode)
+                        (cons (current-buffer) piem-edit-patch--format))
+                   (piem-am-ready-mbox)
                    (user-error
                     "Could not find am-ready mbox for current buffer"))))
     ;; We're responsible for cleaning up the buffer created by
@@ -944,7 +949,8 @@ (defun piem-am--arguments ()
     (list (cons :interactive mbox)
           format
           (piem-extract-mbox-info mbox)
-          (piem-inbox-coderepo-maybe-read)
+          (or piem-edit-patch--coderepo
+              (piem-inbox-coderepo-maybe-read))
           current-prefix-arg)))
 
 ;;;###autoload
@@ -998,9 +1004,6 @@ (defun piem-am (mbox &optional format info coderepo toggle-worktree)
 \f
 ;;;;; Patch editing
 
-(defvar-local piem-edit-patch--coderepo nil)
-(defvar-local piem-edit-patch--format nil)
-
 (defun piem-edit ()
   "Edit an am-ready mbox before feeding it to `git am'."
   (interactive)
@@ -1017,12 +1020,7 @@ (defun piem-edit ()
 (defun piem-edit-patch-am ()
   "Apply the patch that is currently edited."
   (interactive)
-  (let ((buf (current-buffer)))
-    (piem-am buf
-             piem-edit-patch--format
-             (piem-extract-mbox-info (current-buffer))
-             piem-edit-patch--coderepo)
-    (kill-buffer buf)))
+  (call-interactively #'piem-am))
 
 (defvar piem-edit-patch-mode-map
   (let ((map (make-sparse-keymap)))
-- 
2.34.0

Re: [RFC PATCH 06/14] edit patch: Inject values via interactive arguments

[Kyle Meyer]

Kyle Meyer writes:

> piem-edit stores values in buffer-local variables that
> piem-edit-patch-am then uses in a non-interactive call to piem-am.
> That works, but the caller doesn't have access to piem-am's
> toggle-worktree argument.  And this functionality gap will grow bigger
> when an upcoming commit moves transition piem-am to a transient,

s/transition //

> moving the current piem-am to a suffix.
>
> Instead teach piem-am--arguments how to pull the mbox and coderepo
> from these buffer-local values so that piem-edit-patch-am can just
> call piem-am interactively.

I don't think I'm going to revisit this series today, but as a note for
later ...

> [...]
>  (defun piem-edit ()
>    "Edit an am-ready mbox before feeding it to `git am'."
>    (interactive)
> @@ -1017,12 +1020,7 @@ (defun piem-edit ()
>  (defun piem-edit-patch-am ()
>    "Apply the patch that is currently edited."
>    (interactive)
> -  (let ((buf (current-buffer)))
> -    (piem-am buf
> -             piem-edit-patch--format
> -             (piem-extract-mbox-info (current-buffer))
> -             piem-edit-patch--coderepo)
> -    (kill-buffer buf)))
> +  (call-interactively #'piem-am))

... I'm not sure why I kept piem-edit around if it's reduced to an
interactive call to piem-am.  Why not just bind piem-am in
piem-edit-patch-mode-map?

[RFC PATCH 07/14] am, b4-am: Rename piem-am to piem-am-create

[Kyle Meyer]

Make way for a variant the extends an existing branch.
---
 Documentation/piem.texi | 51 ++++++++++++++++++++++-------------------
 piem-b4.el              | 20 ++++++++--------
 piem.el                 |  4 ++--
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/Documentation/piem.texi b/Documentation/piem.texi
index 73d277ea..73a0b1c6 100644
--- a/Documentation/piem.texi
+++ b/Documentation/piem.texi
@@ -259,8 +259,8 @@ Applying patches
 
 @table @code
 
-@item piem-am
-@findex piem-am
+@item piem-am-create
+@findex piem-am-create
 This command tries to extract a patch from the current Notmuch or Gnus
 message buffer and can handle an inline patch as well as one or more
 patch attachments.
@@ -278,8 +278,8 @@ Applying patches contained in a message
 @section Applying patches contained in a message
 
 @table @kbd
-@findex piem-am
-@item M-x piem-am @key{RET} @var{branch} @key{RET} @var{base}
+@findex piem-am-create
+@item M-x piem-am-create @key{RET} @var{branch} @key{RET} @var{base}
 Apply the patch or patches in the current buffer to the associated code
 repository.  Before applying, checkout a new branch @var{branch}
 starting at @var{base}.
@@ -304,7 +304,7 @@ Applying patches contained in a message
 Rather than applying the patches directly to the associated code
 repository, you can create a dedicated worktree by setting
 @code{piem-am-create-worktree} to a non-nil value.  Giving a prefix
-argument to @code{piem-am} inverts the meaning of
+argument to @code{piem-am-create} inverts the meaning of
 @code{piem-am-create-worktree}; that is, by default a prefix argument is
 useful if you generally prefer to work within the configured code
 repository but would like to trigger the one-off creation of a worktree
@@ -318,10 +318,11 @@ Applying patches contained in a message
 a patch.
 
 @findex piem-am-ready-mbox
-Note that the @code{piem-am} command works only for buffers from which
-@code{piem-am-ready-mbox} can generate an am-ready mbox, which depends
-on the enabled integration libraries.  Currently @code{piem-notmuch} and
-@code{piem-gnus} implement the necessary functionality.
+Note that the @code{piem-am-create} command works only for buffers from
+which @code{piem-am-ready-mbox} can generate an am-ready mbox, which
+depends on the enabled integration libraries.  Currently
+@code{piem-notmuch} and @code{piem-gnus} implement the necessary
+functionality.
 
 @node Using b4 to apply patches
 @section Using b4 to apply patches
@@ -369,8 +370,8 @@ Using b4 to apply patches
 @table @kbd
 
 @item a
-@itemx M-x piem-b4-am-from-mid
-@findex piem-b4-am-from-mid
+@itemx M-x piem-b4-am-from-mid-create
+@findex piem-b4-am-from-mid-create
 @findex piem-mid
 @vindex piem-am-create-worktree
 @vindex piem-am-read-worktree-function
@@ -379,10 +380,11 @@ Using b4 to apply patches
 @code{git am} called within an associated Git repository.  If a message
 ID of the current buffer is not known (i.e. @code{piem-mid} returns
 nil), one is read from the caller.  The caller is also queried for the
-branch name and base, as described for @code{piem-am} (@pxref{Applying
-patches contained in a message}).  And, as with @code{piem-am}, a
-worktree can be created by configuring @code{piem-am-create-worktree} to
-a non-nil value or by giving a prefix argument.
+branch name and base, as described for @code{piem-am-create}
+(@pxref{Applying patches contained in a message}).  And, as with
+@code{piem-am-create}, a worktree can be created by configuring
+@code{piem-am-create-worktree} to a non-nil value or by giving a prefix
+argument.
 
 @findex piem-mid-to-thread-functions
 To generate the input thread, first any functions in
@@ -390,17 +392,17 @@ Using b4 to apply patches
 to be retrieved from a local store (e.g., the Notmuch database).  If
 that fails, try to download the thread from the @code{piem-inboxes} URL
 associated with the current buffer, provided the current buffer's
-message ID matches the one @code{piem-b4-am-from-mid} was called with.
-As a last resort, call @code{b4 am} without a local mbox to let it
-download the thread according to its own configuration.
+message ID matches the one @code{piem-b4-am-from-mid-create} was called
+with.  As a last resort, call @code{b4 am} without a local mbox to let
+it download the thread according to its own configuration.
 
 @item i
 @itemx M-x piem-b4-am-ready-from-mid
 @findex piem-b4-am-ready-from-mid
 Call @code{b4 am} with a given message ID.  This differs from
-@code{piem-b4-am-from-mid} in that it is a direct wrapper around a
-command-line call to @code{b4 am}.  The caller is always queried for the
-message ID, and the final product is an am-ready mbox.  @code{b4} is
+@code{piem-b4-am-from-mid-create} in that it is a direct wrapper around
+a command-line call to @code{b4 am}.  The caller is always queried for
+the message ID, and the final product is an am-ready mbox.  @code{b4} is
 responsible for downloading the thread, so the caller must point b4's
 configuration option @code{b4.midmask} to the appropriate public-inbox
 URL.
@@ -418,9 +420,10 @@ Applying patches without a public-inbox archive
 
 Much of the functionality described in the previous sections can work
 even if messages aren't available in a public-inbox archive.
-@code{piem-am} and @code{piem-b4-am-from-mid} try to generate the
-am-ready mbox from a local source (e.g., via Notmuch or Gnus) before
-falling back to downloading the thread from a public-inbox archive.
+@code{piem-am-create} and @code{piem-b4-am-from-mid-create} try to
+generate the am-ready mbox from a local source (e.g., via Notmuch or
+Gnus) before falling back to downloading the thread from a public-inbox
+archive.
 
 @cindex mailscripts
 Also, for those not working with public-inbox archives, it's worth
diff --git a/piem-b4.el b/piem-b4.el
index 18a68d95..d29527d9 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -51,7 +51,7 @@ (defcustom piem-b4-b4-executable "b4"
 ;;;; Internals
 
 (defvar piem-b4-keep-temp-directory nil
-  "Don't clean up the directory created by `piem-b4-am-from-mid'.
+  "Don't clean up the directory created by `piem-b4--get-am-files'.
 This is intended to be used for debugging purposes.")
 
 (defun piem-b4--get-am-files (mid coderepo args)
@@ -128,7 +128,7 @@ (defun piem-b4-am-ready-from-mid (mid &optional args)
          (append args (list mid))))
 
 ;;;###autoload
-(defun piem-b4-am-from-mid (mid &optional args toggle-worktree)
+(defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
   "Get the thread for MID, extract an am-ready mbox, and apply it.
 
 Try to generate a thread for the Message-Id MID with
@@ -162,13 +162,13 @@ (defun piem-b4-am-from-mid (mid &optional args toggle-worktree)
                 (piem-b4--get-am-files mid coderepo args))
                (default-directory coderepo))
     (unwind-protect
-        (piem-am mbox-file
-                 nil
-                 (with-temp-buffer
-                   (insert-file-contents (or cover mbox-file))
-                   (piem-extract-mbox-info))
-                 coderepo
-                 toggle-worktree)
+        (piem-am-create mbox-file
+                        nil
+                        (with-temp-buffer
+                          (insert-file-contents (or cover mbox-file))
+                          (piem-extract-mbox-info))
+                        coderepo
+                        toggle-worktree)
       (when clean-fn
         (funcall clean-fn)))))
 
@@ -253,7 +253,7 @@ (transient-define-prefix piem-b4-am ()
    ;; command-line piping to `git am'.
    (5 "-V" "Do not save cover letter" "--no-cover")]
   ["Actions"
-   [("a" "Message ID -> mbox -> git-am" piem-b4-am-from-mid)]
+   [("a" "Message ID -> mbox -> git-am" piem-b4-am-from-mid-create)]
    [("b" "Local mbox -> am-ready mbox" piem-b4-am-ready-from-mbox)
     ("i" "Message ID -> am-ready mbox" piem-b4-am-ready-from-mid)]])
 
diff --git a/piem.el b/piem.el
index eea81a8e..1a3725cc 100644
--- a/piem.el
+++ b/piem.el
@@ -954,7 +954,7 @@ (defun piem-am--arguments ()
           current-prefix-arg)))
 
 ;;;###autoload
-(defun piem-am (mbox &optional format info coderepo toggle-worktree)
+(defun piem-am-create (mbox &optional format info coderepo toggle-worktree)
   "Feed an am-ready mbox to `git am'.
 
 MBOX is a buffer whose contents are an am-ready mbox (obtained
@@ -1039,7 +1039,7 @@ (define-derived-mode piem-edit-patch-mode text-mode "piem-edit-patch"
 ;;;###autoload (autoload 'piem-dispatch "piem" nil t)
 (transient-define-prefix piem-dispatch ()
   "Invoke a piem command."
-  [[("a" "apply patch" piem-am)
+  [[("a" "apply patch" piem-am-create)
     ("b" "call b4-am" piem-b4-am)]
    [("i" "inject thread into maildir" piem-inject-thread-into-maildir)
     ("l" "copy public-inbox link" piem-copy-mid-url)]
-- 
2.34.0

[RFC PATCH 08/14] am, b4-am: Rewrite -create docstrings to emphasize branch creation

[Kyle Meyer]

The manual does a pretty good job of describing branch creation for
piem-am-create and piem-b4-am-from-mid-create, but the docstrings
don't mention it directly, which will become more confusing once there
are am variants that don't create a new branch.

Modify the summary lines to mention branch creation.
---
 piem-b4.el | 2 +-
 piem.el    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/piem-b4.el b/piem-b4.el
index d29527d9..01004c4b 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -129,7 +129,7 @@ (defun piem-b4-am-ready-from-mid (mid &optional args)
 
 ;;;###autoload
 (defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
-  "Get the thread for MID, extract an am-ready mbox, and apply it.
+  "Create a new branch with patches extracted from MID's thread.
 
 Try to generate a thread for the Message-Id MID with
 `piem-mid-to-thread-functions'.  If that fails, try to download
diff --git a/piem.el b/piem.el
index 1a3725cc..1c00cab8 100644
--- a/piem.el
+++ b/piem.el
@@ -955,7 +955,7 @@ (defun piem-am--arguments ()
 
 ;;;###autoload
 (defun piem-am-create (mbox &optional format info coderepo toggle-worktree)
-  "Feed an am-ready mbox to `git am'.
+  "Create a new branch by feeding an am-ready mbox to `git am'.
 
 MBOX is a buffer whose contents are an am-ready mbox (obtained
 via `piem-am-ready-mbox' when called interactively).  FORMAT
-- 
2.34.0

[RFC PATCH 09/14] piem-am-create: Move info argument to the end

[Kyle Meyer]

The new -extend variant will share all the same arguments except for
INFO, so move that to the end.
---
 piem-b4.el |  6 +++---
 piem.el    | 16 ++++++++--------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/piem-b4.el b/piem-b4.el
index 01004c4b..d2370124 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -164,11 +164,11 @@ (defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
     (unwind-protect
         (piem-am-create mbox-file
                         nil
+                        coderepo
+                        toggle-worktree
                         (with-temp-buffer
                           (insert-file-contents (or cover mbox-file))
-                          (piem-extract-mbox-info))
-                        coderepo
-                        toggle-worktree)
+                          (piem-extract-mbox-info)))
       (when clean-fn
         (funcall clean-fn)))))
 
diff --git a/piem.el b/piem.el
index 1c00cab8..cb931489 100644
--- a/piem.el
+++ b/piem.el
@@ -948,13 +948,13 @@ (defun piem-am--arguments ()
     ;; downstream code know.
     (list (cons :interactive mbox)
           format
-          (piem-extract-mbox-info mbox)
           (or piem-edit-patch--coderepo
               (piem-inbox-coderepo-maybe-read))
-          current-prefix-arg)))
+          current-prefix-arg
+          (piem-extract-mbox-info mbox))))
 
 ;;;###autoload
-(defun piem-am-create (mbox &optional format info coderepo toggle-worktree)
+(defun piem-am-create (mbox &optional format coderepo toggle-worktree info)
   "Create a new branch by feeding an am-ready mbox to `git am'.
 
 MBOX is a buffer whose contents are an am-ready mbox (obtained
@@ -963,16 +963,16 @@ (defun piem-am-create (mbox &optional format info coderepo toggle-worktree)
 am'.  \"mbox\" and \"mboxrd\" are valid values, and \"mboxrd\" is
 the default.
 
-INFO is a plist that with information to help choose a default
-branch name or starting point (see `piem-default-branch-function'
-for a list of possible properties).
-
 CODEREPO, if given, indicates the code repository to operate
 within.  If not specified, the default directory is used.
 
 When prefix argument TOGGLE-WORKTREE is non-nil, invert the
 meaning of `piem-am-create-worktree'.  With the default value,
-this triggers the creation of a new worktree."
+this triggers the creation of a new worktree.
+
+INFO is a plist that with information to help choose a default
+branch name or starting point (see `piem-default-branch-function'
+for a list of possible properties)."
   (interactive (piem-am--arguments))
   (let* ((default-directory (or coderepo default-directory))
          (am-directory default-directory)
-- 
2.34.0

[RFC PATCH 10/14] piem-am--arguments: Make info extraction optional

[Kyle Meyer]

The new -extend variant won't need it.
---
 piem.el | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/piem.el b/piem.el
index cb931489..c3c2dfdf 100644
--- a/piem.el
+++ b/piem.el
@@ -936,22 +936,24 @@ (defun piem--git-am (mbox format dir)
 (defvar-local piem-edit-patch--coderepo nil)
 (defvar-local piem-edit-patch--format nil)
 
-(defun piem-am--arguments ()
+(defun piem-am--arguments (&optional with-info)
   (pcase-let ((`(,mbox . ,format)
                (or (and (derived-mode-p 'piem-edit-patch-mode)
                         (cons (current-buffer) piem-edit-patch--format))
                    (piem-am-ready-mbox)
                    (user-error
                     "Could not find am-ready mbox for current buffer"))))
-    ;; We're responsible for cleaning up the buffer created by
-    ;; `piem-am-ready-mbox'; sneak in an indication to let the
-    ;; downstream code know.
-    (list (cons :interactive mbox)
-          format
-          (or piem-edit-patch--coderepo
-              (piem-inbox-coderepo-maybe-read))
-          current-prefix-arg
-          (piem-extract-mbox-info mbox))))
+    (nconc
+     ;; We're responsible for cleaning up the buffer created by
+     ;; `piem-am-ready-mbox'; sneak in an indication to let the
+     ;; downstream code know.
+     (list (cons :interactive mbox)
+           format
+           (or piem-edit-patch--coderepo
+               (piem-inbox-coderepo-maybe-read))
+           current-prefix-arg)
+     (and with-info
+          (list (piem-extract-mbox-info mbox))))))
 
 ;;;###autoload
 (defun piem-am-create (mbox &optional format coderepo toggle-worktree info)
@@ -973,7 +975,7 @@ (defun piem-am-create (mbox &optional format coderepo toggle-worktree info)
 INFO is a plist that with information to help choose a default
 branch name or starting point (see `piem-default-branch-function'
 for a list of possible properties)."
-  (interactive (piem-am--arguments))
+  (interactive (piem-am--arguments 'with-info))
   (let* ((default-directory (or coderepo default-directory))
          (am-directory default-directory)
          (use-worktree (xor piem-am-create-worktree toggle-worktree)))
-- 
2.34.0

[RFC PATCH 11/14] b4: Move logic for checking arguments to a dedicated function

[Kyle Meyer]

The new -extend variant will need to do the same check.
---
 piem-b4.el | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/piem-b4.el b/piem-b4.el
index d2370124..48836eb2 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -127,6 +127,17 @@ (defun piem-b4-am-ready-from-mid (mid &optional args)
   (apply #'piem-process-start nil piem-b4-b4-executable "am"
          (append args (list mid))))
 
+(defun piem-b4-am--check-args (args)
+  (when-let ((badopt (cl-some
+                      (lambda (arg)
+                        (and (string-match
+                              (rx string-start
+                                  (group (or "--outdir" "--mbox-name")) "=")
+                              arg)
+                             (match-string 1 arg)))
+                      args)))
+    (user-error "%s is incompatible with this command" badopt)))
+
 ;;;###autoload
 (defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
   "Create a new branch with patches extracted from MID's thread.
@@ -148,15 +159,7 @@ (defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
                          (read-string "Message ID: "))
                      (transient-args 'piem-b4-am)
                      current-prefix-arg))
-  (when-let ((badopt (cl-some
-                      (lambda (arg)
-                        (and (string-match
-                              (rx string-start
-                                  (group (or "--outdir" "--mbox-name")) "=")
-                              arg)
-                             (match-string 1 arg)))
-                      args)))
-    (user-error "%s is incompatible with this command" badopt))
+  (piem-b4-am--check-args args)
   (pcase-let* ((coderepo (piem-inbox-coderepo-maybe-read))
                (`(,cover ,mbox-file ,clean-fn)
                 (piem-b4--get-am-files mid coderepo args))
-- 
2.34.0

[RFC PATCH 12/14] am, b4-am: Add commands that extend an existing branch

[Kyle Meyer]

piem-am-create and piem-b4-am-from-mid-create require the caller to
create a new branch or to work on top of a detached head.  Sometimes
it's useful to instead apply a patch to an existing branch (e.g.,
because the caller wants a patch to go directly to the mainline or
because a new patch came in that goes on top of a current topic
branch).  In these cases, an alternative approach is to apply to a
detached head or a temporary branch and then merge into the target.

Although that approach is fairly straightforward (particularly with
the help of magit-merge-into), it might not be obvious to callers.
Add dedicated commands to expose this mode of operation.
---
 piem-b4.el | 14 ++++++++++++++
 piem.el    | 28 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/piem-b4.el b/piem-b4.el
index 48836eb2..f949f963 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -175,6 +175,20 @@ (defun piem-b4-am-from-mid-create (mid &optional args toggle-worktree)
       (when clean-fn
         (funcall clean-fn)))))
 
+(defun piem-b4-am-from-mid-extend (mid &optional args toggle-worktree)
+  "Like `piem-b4-am-from-mid-create', but extend an existing branch."
+  (interactive (list (or (piem-mid)
+                         (read-string "Message ID: "))
+                     (transient-args 'piem-b4-am)))
+  (piem-b4-am--check-args args)
+  (pcase-let* ((coderepo (piem-inbox-coderepo-maybe-read))
+               (`(,_ ,mbox-file ,clean-fn)
+                (piem-b4--get-am-files mid coderepo args)))
+    (unwind-protect
+        (piem-am-extend mbox-file nil coderepo toggle-worktree)
+      (when clean-fn
+        (funcall clean-fn)))))
+
 (transient-define-argument piem-b4-am:--outdir ()
   :description "Output directory"
   :class 'transient-option
diff --git a/piem.el b/piem.el
index c3c2dfdf..73e331cc 100644
--- a/piem.el
+++ b/piem.el
@@ -1003,6 +1003,34 @@ (defun piem-am-create (mbox &optional format coderepo toggle-worktree info)
                           (list base)))))
     (piem--git-am mbox format am-directory)))
 
+(defun piem-am-extend (mbox &optional format coderepo toggle-worktree)
+  "Like `piem-am-create', but extend an existing branch."
+  (interactive (piem-am--arguments))
+  (let* ((default-directory (or coderepo default-directory))
+         (am-directory default-directory)
+         (use-worktree (xor piem-am-create-worktree toggle-worktree))
+         (branch (if (and piem-use-magit
+                          (fboundp 'magit-get-current-branch)
+                          (fboundp 'magit-read-branch-or-commit))
+                     (let ((current-branch (magit-get-current-branch))
+                           (b (magit-read-branch-or-commit "Branch")))
+                       (and (not (equal b current-branch)) b))
+                   (read-string "Branch (empty for current): "))))
+    (cond
+     ((and branch (not (string-empty-p branch)))
+      (when use-worktree
+        (setq am-directory
+              (piem-am--read-worktree default-directory branch)))
+      (apply #'piem-process-call nil piem-git-executable
+             (append (if use-worktree
+                         (list "worktree" "add")
+                       (list "checkout"))
+                     (and use-worktree (list am-directory))
+                     (list branch))))
+     (use-worktree
+      (user-error "Worktree can't be created with coderepo's current branch")))
+    (piem--git-am mbox format am-directory)))
+
 \f
 ;;;;; Patch editing
 
-- 
2.34.0

[RFC PATCH 13/14] am: Move functionality under a dedicated transient

[Kyle Meyer]

This provides a place to bind piem-am-extend (and perhaps other
things, like piem-edit).  At least for my use, I expect piem-am-create
to the dominant variant, so make the prefix default to calling that
without showing the popup.
---
 piem.el | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/piem.el b/piem.el
index 73e331cc..2b1d3095 100644
--- a/piem.el
+++ b/piem.el
@@ -1031,6 +1031,16 @@ (defun piem-am-extend (mbox &optional format coderepo toggle-worktree)
       (user-error "Worktree can't be created with coderepo's current branch")))
     (piem--git-am mbox format am-directory)))
 
+;;;###autoload (autoload 'piem-am "piem" nil t)
+(transient-define-prefix piem-am (&optional transient)
+  "Feed an am-ready mbox to `git am'."
+  [[("a" "create new branch" piem-am-create)
+    ("A" "extend existing branch" piem-am-extend)]]
+  (interactive "P")
+  (if transient
+      (transient-setup 'piem-am)
+    (call-interactively #'piem-am-create)))
+
 \f
 ;;;;; Patch editing
 
@@ -1069,7 +1079,7 @@ (define-derived-mode piem-edit-patch-mode text-mode "piem-edit-patch"
 ;;;###autoload (autoload 'piem-dispatch "piem" nil t)
 (transient-define-prefix piem-dispatch ()
   "Invoke a piem command."
-  [[("a" "apply patch" piem-am-create)
+  [[("a" "apply patch" piem-am)
     ("b" "call b4-am" piem-b4-am)]
    [("i" "inject thread into maildir" piem-inject-thread-into-maildir)
     ("l" "copy public-inbox link" piem-copy-mid-url)]
-- 
2.34.0

[RFC PATCH 14/14] b4: Add piem-b4-am-from-mid-extend to transient

[Kyle Meyer]

---
 piem-b4.el | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/piem-b4.el b/piem-b4.el
index f949f963..edaed7b6 100644
--- a/piem-b4.el
+++ b/piem-b4.el
@@ -269,10 +269,12 @@ (transient-define-prefix piem-b4-am ()
    ;; Hide because this is unlikely to be useful outside of
    ;; command-line piping to `git am'.
    (5 "-V" "Do not save cover letter" "--no-cover")]
-  ["Actions"
-   [("a" "Message ID -> mbox -> git-am" piem-b4-am-from-mid-create)]
-   [("b" "Local mbox -> am-ready mbox" piem-b4-am-ready-from-mbox)
-    ("i" "Message ID -> am-ready mbox" piem-b4-am-ready-from-mid)]])
+  ["Message ID -> mbox -> git-am"
+   ("a" "... creating branch" piem-b4-am-from-mid-create)
+   ("A" "... extending existing branch" piem-b4-am-from-mid-extend)]
+  ["Create am-ready mbox"
+   [("b" "... from local mbox" piem-b4-am-ready-from-mbox)
+    ("i" "... from message ID" piem-b4-am-ready-from-mid)]])
 
 ;;; piem-b4.el ends here
 (provide 'piem-b4)
-- 
2.34.0