This series is prompted by simon's patch workflow described at <https://yhetil.org/guix-patches/86361cys9h.fsf@tournier.info>: > Yes, somehow. Generally, I create a new checked out worktree with a new > branch starting at base-commit, then I apply the patches in. Well, my > sequence looks like: > > M-C-SPC M-w <base-commmit> > C-x b <magit-guix-master> > % c /path/to/wk/foo RET C-y RET foo RET > C-x b <message-with-patch> > | git gam foo > > and I am not enough annoyed (yet!) to write some Emacs helper > functions. However, if b4+piem could do that for me, should be > awesome. :-) With patch 4, something like that should be possible with both piem-am and piem-b4-am-from-mid. When the new piem-am-create-worktree option is set to a non-nil value, the caller will be prompted for a directory where the new worktree should be created. Patch 5 makes it possible to trigger this in a one-off fashion with a prefix argument because, while I don't want this behavior most of the time, I could see myself using it every now and then. [1/6] piem-am: Rephrase CODEREPO description [2/6] piem-am: Store "empty string" branch check [3/6] am: Support creating a new worktree [4/6] am: Add option to configure how worktree is read [5/6] am: Allow flipping worktree creation with prefix argument [6/6] manual: Document worktree-related options piem-b4.el | 15 ++++++++--- piem.el | 77 ++++++++++++++++++++++++++++++++++++++++++------------ piem.texi | 15 ++++++++++- 3 files changed, 85 insertions(+), 22 deletions(-) base-commit: b54ad663fe22811dde804721d4db528ea18a433c -- 2.29.2
Describing CODEREPO in terms of where git-am is called is a bit confusing because piem-am does other things here as well (e.g. reading the base and checking out a branch). And it won't necessarily be where git-am is called once worktree support is added. Give a more generic description. --- piem.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/piem.el b/piem.el index 5ad2b94..8ee5b0b 100644 --- a/piem.el +++ b/piem.el @@ -650,8 +650,8 @@ (defun piem-am (mbox &optional format info coderepo) branch name or starting point (see `piem-default-branch-function' for a list of possible properties). -If CODEREPO is given, switch to this directory before calling -`git am'." +CODEREPO, if given, indicates the code repository to operate +within. If not specified, the default directory is used." (interactive (pcase-let ((`(,mbox . ,format) (or (piem-am-ready-mbox) -- 2.29.2
This will be needed in another spot. --- piem.el | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/piem.el b/piem.el index 8ee5b0b..a42ab3d 100644 --- a/piem.el +++ b/piem.el @@ -669,9 +669,10 @@ (defun piem-am (mbox &optional format info coderepo) (interactivep (eq (car-safe mbox) :interactive))) (when interactivep (setq mbox (cdr mbox))) - (let ((new-branch (read-string - "New branch (empty for detached): " - (funcall piem-default-branch-function info))) + (let ((new-branch + (let ((b (read-string "New branch (empty for detached): " + (funcall piem-default-branch-function info)))) + (and (not (string-empty-p b)) b))) (base (completing-read "Base commit: " (let ((cands (and piem-use-magit @@ -680,9 +681,7 @@ (defun piem-am (mbox &optional format info coderepo) (base (plist-get info :base-commit))) (if base (cons base cands) cands))))) (apply #'piem-process-call nil piem-git-executable "checkout" - (append (if (string-empty-p new-branch) - (list "--detach") - (list "-b" new-branch)) + (append (if new-branch (list "-b" new-branch) (list "--detach")) (and (not (string-blank-p base)) (list base))))) (let ((args (cons (concat "--patch-format=" format) -- 2.29.2
On the guix-patches list, simon described a workflow in which a new worktree is used to apply patches. Such a workflow is fairly straightforward to support in piem-am (and thus piem-b4-am-from-mid). Aside form reading the worktree from the caller, the main change needed is to replace 'git checkout (-b BRANCH|--detatched) [base]' with 'git worktree add (-b BRANCH|--detatched) PATH [base]'. Teach piem-am to use a worktree when piem-am-create-worktree is non-nil. Suggested-by: zimoun <zimon.toutoune@gmail.com> Ref: https://yhetil.org/guix-patches/86361cys9h.fsf@tournier.info --- piem.el | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/piem.el b/piem.el index a42ab3d..da7f9f8 100644 --- a/piem.el +++ b/piem.el @@ -154,6 +154,10 @@ (defcustom piem-default-branch-function The reported base commit of the patch, if any." :type 'function) +(defcustom piem-am-create-worktree nil + "Whether to create a dedicated worktree for applying patches." + :type 'boolean) + (defcustom piem-maildir-directory nil "Inject public-inbox threads into this directory. If non-nil, this must be an existing Maildir directory." @@ -636,6 +640,20 @@ (defun piem-name-branch-who-what-v (info) (defvar piem-am-args (list "--scissors" "--3way")) +(defun piem-am-read-worktree (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 +completed from the parent directory of CODEREPO. If BRANCH is +non-nil, it is used as for construct the default completion." + (let ((fname (directory-file-name coderepo))) + (read-directory-name + "Create worktree: " + (file-name-directory fname) nil nil + (and branch + (concat (file-name-nondirectory fname) "-" + (replace-regexp-in-string "/" "-" branch)))))) + ;;;###autoload (defun piem-am (mbox &optional format info coderepo) "Feed an am-ready mbox to `git am'. @@ -665,8 +683,9 @@ (defun piem-am (mbox &optional format info coderepo) (piem-extract-mbox-info mbox) (piem-inbox-coderepo-maybe-read)))) (setq format (or format "mboxrd")) - (let ((default-directory (or coderepo default-directory)) - (interactivep (eq (car-safe mbox) :interactive))) + (let* ((default-directory (or coderepo default-directory)) + (am-directory default-directory) + (interactivep (eq (car-safe mbox) :interactive))) (when interactivep (setq mbox (cdr mbox))) (let ((new-branch @@ -680,8 +699,18 @@ (defun piem-am (mbox &optional format info coderepo) (magit-list-local-branch-names))) (base (plist-get info :base-commit))) (if base (cons base cands) cands))))) - (apply #'piem-process-call nil piem-git-executable "checkout" - (append (if new-branch (list "-b" new-branch) (list "--detach")) + (when piem-am-create-worktree + (setq am-directory + (expand-file-name + (piem-am-read-worktree default-directory new-branch))) + (when (file-exists-p am-directory) + (user-error "Worktree directory already exists"))) + (apply #'piem-process-call nil piem-git-executable + (append (if piem-am-create-worktree + (list "worktree" "add") + (list "checkout")) + (if new-branch (list "-b" new-branch) (list "--detach")) + (and piem-am-create-worktree (list am-directory)) (and (not (string-blank-p base)) (list base))))) (let ((args (cons (concat "--patch-format=" format) @@ -689,15 +718,15 @@ (defun piem-am (mbox &optional format info coderepo) (if (bufferp mbox) (unwind-protect (apply #'piem-process-call-with-buffer-input - nil mbox piem-git-executable "am" args) + am-directory mbox piem-git-executable "am" args) (when interactivep (kill-buffer mbox))) - (apply #'piem-process-call nil piem-git-executable "am" + (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) - (dired ".")))) + (magit-status-setup-buffer am-directory) + (dired am-directory)))) ;;;###autoload (autoload 'piem-dispatch "piem" nil t) (define-transient-command piem-dispatch () -- 2.29.2
It seems likely that piem-am-read-worktree won't quite behave as some callers want. Let users specify a custom function. --- piem.el | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/piem.el b/piem.el index da7f9f8..ac9812c 100644 --- a/piem.el +++ b/piem.el @@ -158,6 +158,14 @@ (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 + "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 +name of the branch that will be created. The branch may be nil +if the caller requested a detached HEAD." + :type 'function) + (defcustom piem-maildir-directory nil "Inject public-inbox threads into this directory. If non-nil, this must be an existing Maildir directory." @@ -702,7 +710,8 @@ (defun piem-am (mbox &optional format info coderepo) (when piem-am-create-worktree (setq am-directory (expand-file-name - (piem-am-read-worktree default-directory new-branch))) + (funcall piem-am-read-worktree-function + default-directory new-branch))) (when (file-exists-p am-directory) (user-error "Worktree directory already exists"))) (apply #'piem-process-call nil piem-git-executable -- 2.29.2
I tend to use a few dedicated worktrees for projects and am not interested in creating a worktree for each patch series I apply. However, I can imagine wanting to create one every now and then. Make it possible by adding a prefix argument to piem-am and piem-b4-am-from-mid that flips the meaning of piem-am-create-worktree. --- piem-b4.el | 15 +++++++++++---- piem.el | 18 ++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/piem-b4.el b/piem-b4.el index 27ecf19..172236f 100644 --- a/piem-b4.el +++ b/piem-b4.el @@ -130,15 +130,21 @@ (defun piem-b4-am-ready-from-mid (mid &optional args) (append args (list mid)))) ;;;###autoload -(defun piem-b4-am-from-mid (mid &optional args) +(defun piem-b4-am-from-mid (mid &optional args toggle-worktree) "Get the thread for MID, extract an am-ready mbox, and apply it. + Try to get a thread for the Message-Id MID with `piem-mid-to-thread-functions', falling back to letting b4 download it. After calling `b4 am' with ARGS to prepare an -am-ready mbox, feed the result to `git am'." +am-ready mbox, feed the result to `git am'. + +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 (list (or (piem-mid) (read-string "Message ID: ")) - (transient-args 'piem-b4-am))) + (transient-args 'piem-b4-am) + current-prefix-arg)) (when-let ((badopt (cl-some (lambda (arg) (and (string-match @@ -158,7 +164,8 @@ (defun piem-b4-am-from-mid (mid &optional args) (with-temp-buffer (insert-file-contents (or cover mbox-file)) (piem-extract-mbox-info)) - coderepo) + coderepo + toggle-worktree) (when clean-fn (funcall clean-fn))))) diff --git a/piem.el b/piem.el index ac9812c..0cb1093 100644 --- a/piem.el +++ b/piem.el @@ -663,7 +663,7 @@ (defun piem-am-read-worktree (coderepo branch) (replace-regexp-in-string "/" "-" branch)))))) ;;;###autoload -(defun piem-am (mbox &optional format info coderepo) +(defun piem-am (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 @@ -677,7 +677,11 @@ (defun piem-am (mbox &optional format info coderepo) for a list of possible properties). CODEREPO, if given, indicates the code repository to operate -within. If not specified, the default directory is used." +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." (interactive (pcase-let ((`(,mbox . ,format) (or (piem-am-ready-mbox) @@ -689,10 +693,12 @@ (defun piem-am (mbox &optional format info coderepo) (list (cons :interactive mbox) format (piem-extract-mbox-info mbox) - (piem-inbox-coderepo-maybe-read)))) + (piem-inbox-coderepo-maybe-read) + current-prefix-arg))) (setq format (or format "mboxrd")) (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))) @@ -707,7 +713,7 @@ (defun piem-am (mbox &optional format info coderepo) (magit-list-local-branch-names))) (base (plist-get info :base-commit))) (if base (cons base cands) cands))))) - (when piem-am-create-worktree + (when use-worktree (setq am-directory (expand-file-name (funcall piem-am-read-worktree-function @@ -715,11 +721,11 @@ (defun piem-am (mbox &optional format info coderepo) (when (file-exists-p am-directory) (user-error "Worktree directory already exists"))) (apply #'piem-process-call nil piem-git-executable - (append (if piem-am-create-worktree + (append (if use-worktree (list "worktree" "add") (list "checkout")) (if new-branch (list "-b" new-branch) (list "--detach")) - (and piem-am-create-worktree (list am-directory)) + (and use-worktree (list am-directory)) (and (not (string-blank-p base)) (list base))))) (let ((args (cons (concat "--patch-format=" format) -- 2.29.2
--- piem.texi | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/piem.texi b/piem.texi index cf39fe8..5f90055 100644 --- a/piem.texi +++ b/piem.texi @@ -251,6 +251,17 @@ Applying patches contained in a message be provided as the default completion candidate. Entering an empty base signals to use the current branch of the repository as the base. +@vindex piem-am-create-worktree +@vindex piem-am-read-worktree-function +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 +@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 +for a particular call. + @cindex magit @vindex piem-use-magit When piem loads, it detects whether Magit is loaded and sets @@ -320,7 +331,9 @@ Using b4 to apply patches 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}). +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. @findex piem-mid-to-thread-functions To generate the input thread, first any functions in -- 2.29.2
Hi Kyle,
Thank you.
On Sun, 15 Nov 2020 at 01:15, Kyle Meyer <kyle@kyleam.com> wrote:
> With patch 4, something like that should be possible with both piem-am
> and piem-b4-am-from-mid. When the new piem-am-create-worktree option
> is set to a non-nil value, the caller will be prompted for a directory
> where the new worktree should be created.
>
> Patch 5 makes it possible to trigger this in a one-off fashion with a
> prefix argument because, while I don't want this behavior most of the
> time, I could see myself using it every now and then.
That’s awesome! I am going to test all that. I am currently running
out of time because lockdown=extra-work+weird-feelings and the coming online
Guix Days (and release v1.2 even if Ludo did everything).
Well, I will report you my feedback soon and will try to add a section
to the Guix Cookbook; anyhow, I have to add one after my experience as
Outreach mentor. :-)
Cheers,
simon
zimoun writes:
> That’s awesome! I am going to test all that. I am currently running
> out of time because lockdown=extra-work+weird-feelings and the coming online
> Guix Days (and release v1.2 even if Ludo did everything).
>
> Well, I will report you my feedback soon and will try to add a section
> to the Guix Cookbook; anyhow, I have to add one after my experience as
> Outreach mentor. :-)
Sure, no rush. I'll probably review this series with fresh eyes later
today or tomorrow and merge it in, but everything will of course still
be open for discussion after that point, and any feedback is very
welcome.
Thanks for the nice idea.
Kyle Meyer writes:
> Sure, no rush. I'll probably review this series with fresh eyes later
> today or tomorrow and merge it in, but everything will of course still
> be open for discussion after that point, and any feedback is very
> welcome.
I've taken another look and was more or less happy with it (and in
general think worktree creation should be supported in some way). The
text added to the manual feels a bit off, but I think that may be due to
a wider need to rearrange things. Dunno.
Pushed with a couple of minor tweaks.
1: 613b625 = 1: 613b625 piem-am: Rephrase CODEREPO description
2: 291878e = 2: 291878e piem-am: Store "empty string" branch check
3: a8b3342 ! 3: 060055d am: Support creating a new worktree
@@ piem.el: (defun piem-name-branch-who-what-v (info)
+This function is intended to be used as a value of
+`piem-am-read-worktree-function'. The worktree directory is
+completed from the parent directory of CODEREPO. If BRANCH is
-+non-nil, it is used as for construct the default completion."
++non-nil, it is used to construct the default completion value."
+ (let ((fname (directory-file-name coderepo)))
+ (read-directory-name
+ "Create worktree: "
4: 6250619 = 4: 251f9a2 am: Add option to configure how worktree is read
5: 92e1d7d = 5: 688c878 am: Allow flipping worktree creation with prefix argument
6: d089154 ! 6: 5db8c31 manual: Document worktree-related options
@@ piem.texi: Applying patches contained in a message
@vindex piem-use-magit
When piem loads, it detects whether Magit is loaded and sets
@@ piem.texi: Using b4 to apply patches
+ @itemx M-x piem-b4-am-from-mid
+ @findex piem-b4-am-from-mid
+ @findex piem-mid
++@vindex piem-am-create-worktree
++@vindex piem-am-read-worktree-function
+ Generate or download a thread's mbox for the current buffer's message
+ ID, process it into an am-ready mbox with b4, and then feed it to
+ @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
Hi Kyle,
On Sun, 15 Nov 2020 at 17:32, Kyle Meyer <kyle@kyleam.com> wrote:
> I've taken another look and was more or less happy with it (and in
> general think worktree creation should be supported in some way). The
> text added to the manual feels a bit off, but I think that may be due to
> a wider need to rearrange things. Dunno.
I will try it in a couple of days, I guess. With completely fresh
eyes. Good test for the manual.
Cheers,
simon