[PATCH] emacs: Helpers for notmuch developers.

[PATCH] emacs: Helpers for notmuch developers.

[David Edmondson]

---

I've been using this for a few days and decided to share it to get
feedback. Reviewing patches can be tedious, so I tried to make things
a little simpler.

To use this, load the file and then from `notmuch-show-mode' invoke
`notmuch-dev-show-review-patch'. It assumes that any open messages
contain patches and attempts to build a repository with the patches
applied.

General management (i.e. keeping up to date) of the repository it uses
is your responsibility, as is cleaning out old branches. You can, of
course, just delete the temporary repository after using it - the code
will re-create it next time.

If you have a slow network connection then a local copy of the main
repository can be specified by changing
`notmuch-dev-master-repository'.

Comments?

 emacs/notmuch-dev.el |  121 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 121 insertions(+), 0 deletions(-)
 create mode 100644 emacs/notmuch-dev.el

diff --git a/emacs/notmuch-dev.el b/emacs/notmuch-dev.el
new file mode 100644
index 0000000..63ee490
--- /dev/null
+++ b/emacs/notmuch-dev.el
@@ -0,0 +1,121 @@
+;; notmuch-dev.el --- help for notmuch developers
+;;
+;; Copyright © David Edmondson
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
+;;
+;; Authors: David Edmondson <dme@dme.org>
+
+(require 'notmuch-lib)
+(require 'notmuch-show)
+(require 'magit)
+
+(defgroup notmuch-dev nil
+  "Helpers for notmuch developers."
+  :group 'notmuch)
+
+(defcustom notmuch-dev-master-repository "git://notmuchmail.org/git/notmuch"
+  "The URI of the master notmuch repository."
+  :group 'notmuch-dev
+  :type 'string)
+
+(defcustom notmuch-dev-temporary-directory temporary-file-directory
+  "A directory in which to place temporary repositories."
+  :group 'notmuch-dev
+  :type 'string)
+
+;;
+
+(defvar notmuch-dev-temporary-repository-name (concat "notmuch-dev-" (user-login-name))
+  "The name of the temporary repository.")
+
+(defvar notmuch-dev-temporary-repository-path
+  (file-name-as-directory (file-truename (concat notmuch-dev-temporary-directory "/"
+						 notmuch-dev-temporary-repository-name)))
+  "The path of the temporary repository.")
+
+(defun notmuch-dev-make-temporary-repository ()
+  (unless (file-directory-p notmuch-dev-temporary-repository-path)
+    (message "Cloning %s into %s..." 
+	     notmuch-dev-master-repository notmuch-dev-temporary-repository-path)
+    (magit-run* (list magit-git-executable "clone"
+		      notmuch-dev-master-repository
+		      notmuch-dev-temporary-repository-path))
+
+    (unless (file-directory-p notmuch-dev-temporary-repository-path)
+      (error "git clone failed.")))
+
+  ;; Causes us to switch to the magit buffer - is that unfortunate in
+  ;; some situations?
+  (magit-status notmuch-dev-temporary-repository-path))
+
+(defun notmuch-dev-checkout-master ()
+  (magit-checkout "master"))
+
+(defun notmuch-dev-delete-branch (name)
+  (magit-delete-branch name))
+
+(defun notmuch-dev-create-branch (name)
+  ;; Switches to the new branch automatically.
+  (magit-create-branch name "master"))
+
+(defun notmuch-dev-title-to-branch (title)
+  (let* ((s (downcase title))
+	 (s (replace-regexp-in-string "[ \t]+" "-" s))
+	 (s (replace-regexp-in-string "[\]\[\":]" "" s))
+	 (s (replace-regexp-in-string ".$" "" s))
+	 )
+    s))
+
+;;
+
+(defun notmuch-dev-show-review-patch ()
+  "Call this from `notmuch-show-mode'."
+  (interactive)
+
+  (notmuch-dev-review-patch (notmuch-show-get-subject)
+			    (mapconcat 'identity
+				       (notmuch-show-get-message-ids-for-open-messages)
+				       " | ")))
+
+(defun notmuch-dev-review-patch (title search-terms)
+  (let ((patch-name (notmuch-dev-title-to-branch title)))
+
+    (notmuch-dev-make-temporary-repository)
+
+    ;; Switch to the repository directory.
+    (let ((default-directory notmuch-dev-temporary-repository-path))
+
+      (notmuch-dev-checkout-master)
+      ;; Delete the branch if it exists.
+      (condition-case nil
+	  (notmuch-dev-delete-branch patch-name)
+	(error nil))
+      (notmuch-dev-create-branch patch-name)
+
+      ;; Have notmuch generate mailbox format output for the search
+      ;; terms and feed that to git-am.
+      (shell-command
+       (concat
+	notmuch-command " show --format=mbox " (shell-quote-argument search-terms)
+	" | "
+	"git am --quiet"))
+
+      (magit-status notmuch-dev-temporary-repository-path))))
+
+;;
+
+(provide 'notmuch-dev)
-- 
1.7.7.3

Re: [PATCH] emacs: Helpers for notmuch developers.

[David Bremner]

On Wed,  4 Jan 2012 14:01:18 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> I've been using this for a few days and decided to share it to get
> feedback. Reviewing patches can be tedious, so I tried to make things
> a little simpler.
> 

I didn't have a very thorough look at the code, but I like the
functionality. Especially hitting q a few times from magit and being in
the right place to reply.

> General management (i.e. keeping up to date) of the repository it uses
> is your responsibility, as is cleaning out old branches. You can, of
> course, just delete the temporary repository after using it - the code
> will re-create it next time.

maybe the branches could be under some namespace that makes this easy?

> +(defvar notmuch-dev-temporary-repository-name (concat "notmuch-dev-" (user-login-name))
> +  "The name of the temporary repository.")

Do we care about security at all in this context? I'm always a bit
nervous about creating predictably named files/directories in publicly
writable places.

> +  ;; Causes us to switch to the magit buffer - is that unfortunate in
> +  ;; some situations?
> +  (magit-status notmuch-dev-temporary-repository-path))

I found it to be a pleasant surprise.

> +      (shell-command
> +       (concat
> +	notmuch-command " show --format=mbox " (shell-quote-argument search-terms)
> +	" | "
> +	"git am --quiet"))
> +

Hmm. A knee jerk reaction is not to like this, like seeing system in C
code. But I don't have a better solution off hand.

d

Re: [PATCH] emacs: Helpers for notmuch developers.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

On Thu, 05 Jan 2012 23:39:30 -0400, David Bremner <david@tethera.net> wrote:
> > General management (i.e. keeping up to date) of the repository it uses
> > is your responsibility, as is cleaning out old branches. You can, of
> > course, just delete the temporary repository after using it - the code
> > will re-create it next time.
> 
> maybe the branches could be under some namespace that makes this easy?

Just a prefix? 'review/' or something?

> > +(defvar notmuch-dev-temporary-repository-name (concat "notmuch-dev-" (user-login-name))
> > +  "The name of the temporary repository.")
> 
> Do we care about security at all in this context? I'm always a bit
> nervous about creating predictably named files/directories in publicly
> writable places.

I presume that you'd already have set `temporary-file-directory' if you
worry about this generally, or `notmuch-dev-temporary-directory' if just
in this instance.

> > +  ;; Causes us to switch to the magit buffer - is that unfortunate in
> > +  ;; some situations?
> > +  (magit-status notmuch-dev-temporary-repository-path))
> 
> I found it to be a pleasant surprise.

Ending there when the whole sequence completes, I agree. The question is
really about whether displaying the magit-status buffer after simply
creating the repository is good.

> > +      (shell-command
> > +       (concat
> > +	notmuch-command " show --format=mbox " (shell-quote-argument search-terms)
> > +	" | "
> > +	"git am --quiet"))
> > +
> 
> Hmm. A knee jerk reaction is not to like this, like seeing system in C
> code. But I don't have a better solution off hand.

Agreed. I think that I'll split it into two parts:
  - running notmuch to create a mailbox (or perhaps directory of
    patches),
  - running git-am to apply the patches.

A bigger problem is that magit appears to be ignorant of git-am, with
the result that if applying the patches fails magit doesn't provide any
help (or even notification) that there are outstanding things to do. It
also blocks a future git-am.

Currently thinking about an alternative to using git-am that is more
magit friendly...

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH v2] emacs: Helpers for notmuch developers.

[David Edmondson]

---

- Prefix the branch name with 'review/'
- Avoid `shell-command', which also results in better error reporting
  when 'git-am' fails.

 emacs/notmuch-dev.el |  129 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 129 insertions(+), 0 deletions(-)
 create mode 100644 emacs/notmuch-dev.el

diff --git a/emacs/notmuch-dev.el b/emacs/notmuch-dev.el
new file mode 100644
index 0000000..ac427ec
--- /dev/null
+++ b/emacs/notmuch-dev.el
@@ -0,0 +1,129 @@
+;; notmuch-dev.el --- help for notmuch developers
+;;
+;; Copyright © David Edmondson
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see <http://www.gnu.org/licenses/>.
+;;
+;; Authors: David Edmondson <dme@dme.org>
+
+(require 'notmuch-lib)
+(require 'notmuch-show)
+(require 'magit)
+
+(defgroup notmuch-dev nil
+  "Helpers for notmuch developers."
+  :group 'notmuch)
+
+(defcustom notmuch-dev-master-repository "git://notmuchmail.org/git/notmuch"
+  "The URI of the master notmuch repository."
+  :group 'notmuch-dev
+  :type 'string)
+
+(defcustom notmuch-dev-temporary-directory temporary-file-directory
+  "A directory in which to place temporary repositories."
+  :group 'notmuch-dev
+  :type 'string)
+
+;;
+
+(defvar notmuch-dev-temporary-repository-name (concat "notmuch-dev-" (user-login-name))
+  "The name of the temporary repository.")
+
+(defvar notmuch-dev-temporary-repository-path
+  (file-name-as-directory (file-truename (concat notmuch-dev-temporary-directory "/"
+						 notmuch-dev-temporary-repository-name)))
+  "The path of the temporary repository.")
+
+(defun notmuch-dev-make-temporary-repository ()
+  (unless (file-directory-p notmuch-dev-temporary-repository-path)
+    (message "Cloning %s into %s..." 
+	     notmuch-dev-master-repository notmuch-dev-temporary-repository-path)
+    (magit-run* (list magit-git-executable "clone"
+		      notmuch-dev-master-repository
+		      notmuch-dev-temporary-repository-path))
+    (message "Cloning %s into %s...done." 
+	     notmuch-dev-master-repository notmuch-dev-temporary-repository-path)
+
+    (unless (file-directory-p notmuch-dev-temporary-repository-path)
+      (error "git clone failed."))))
+
+(defun notmuch-dev-checkout-master ()
+  (magit-checkout "master"))
+
+(defun notmuch-dev-delete-branch (name)
+  (magit-delete-branch name))
+
+(defun notmuch-dev-create-branch (name)
+  ;; Switches to the new branch automatically.
+  (magit-create-branch name "master"))
+
+(defun notmuch-dev-flatten-title (title)
+  (let* ((s (downcase title))
+	 (s (replace-regexp-in-string "[ \t/]+" "-" s))
+	 (s (replace-regexp-in-string "[\]\[\":]" "" s))
+	 (s (replace-regexp-in-string "\\.$" "" s))
+	 )
+    s))
+
+(defun notmuch-dev-title-to-branch (title)
+  (concat "review/" (notmuch-dev-flatten-title title)))
+
+(defun notmuch-dev-title-to-mbox (title)
+  (concat notmuch-dev-temporary-directory "/"
+	  (notmuch-dev-flatten-title title) ".mbox"))
+
+;;
+
+(defun notmuch-dev-show-review-patch ()
+  "Call this from `notmuch-show-mode'."
+  (interactive)
+
+  (notmuch-dev-review-patch (notmuch-show-get-subject)
+			    (mapconcat 'identity
+				       (notmuch-show-get-message-ids-for-open-messages)
+				       " OR ")))
+
+(defun notmuch-dev-review-patch (title search-terms)
+  (let ((patch-name (notmuch-dev-title-to-branch title))
+	(mbox-path (notmuch-dev-title-to-mbox title)))
+
+    (notmuch-dev-make-temporary-repository)
+
+    ;; Switch to the repository directory.
+    (let ((default-directory notmuch-dev-temporary-repository-path))
+
+      (notmuch-dev-checkout-master)
+      ;; Delete the branch if it exists.
+      (condition-case nil
+	  (notmuch-dev-delete-branch patch-name)
+	(error nil))
+      (notmuch-dev-create-branch patch-name)
+
+      ;; Have notmuch generate mailbox format output for the search
+      ;; terms...
+      (with-temp-file mbox-path
+	(erase-buffer)
+	(call-process notmuch-command nil t nil
+		      "show" "--format=mbox" search-terms))
+
+      ;; ...and feed that to git-am.
+      (magit-run* (list magit-git-executable "am" mbox-path))
+
+      (magit-status notmuch-dev-temporary-repository-path))))
+
+;;
+
+(provide 'notmuch-dev)
-- 
1.7.7.3

Re: [PATCH] emacs: Helpers for notmuch developers.

[Tomi Ollila]

On Wed,  4 Jan 2012 14:01:18 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> I've been using this for a few days and decided to share it to get
> feedback. Reviewing patches can be tedious, so I tried to make things
> a little simpler.
> 
> To use this, load the file and then from `notmuch-show-mode' invoke
> `notmuch-dev-show-review-patch'. It assumes that any open messages
> contain patches and attempts to build a repository with the patches
> applied.
> 
> General management (i.e. keeping up to date) of the repository it uses
> is your responsibility, as is cleaning out old branches. You can, of
> course, just delete the temporary repository after using it - the code
> will re-create it next time.
> 
> If you have a slow network connection then a local copy of the main
> repository can be specified by changing
> `notmuch-dev-master-repository'.
> 
> Comments?

I think this is great. I've worked on something related but the emacs MUA
"integration" makes patching simpler (My thing worked on shell and would
require a 'recipe' file to describe HEAD commit and patch message id:s).

What I've done differently is that I already have cloned notmuch repository
somewhere which I attempt to keep up-to-date. For repository which I'm
going to patch I first do:

git clone --local --shared --no-checkout /path/to/notmuch/repo notmuch-dev-$USER
git --git-dir notmuch-dev-$USER/.git' config --unset remote.origin.url
cd notmuch-dev-$USER
git checkout -b patched $commit

I.e.

1) I created a local repository clone, sharing objects without initial checkout.
2) removed "remote" origin so accidental pushes are not possible
3) checkout out tree from some commit (possible not remote HEAD) 

And this is done every time before new patchset is applied -- always on top
of clean working directory.

What do you think of this approach related to your way cloning the repo and
then deleting/creating the branch. Just that developer may mess with the
repository contents and then there is tedious working tree cleanup to be
done (especially those who are not so fluent using git).


Tomi

Re: [PATCH v2] emacs: Helpers for notmuch developers.

[David Bremner]

On Fri,  6 Jan 2012 10:03:19 +0000, David Edmondson <dme@dme.org> wrote:
> ---
> 
> - Prefix the branch name with 'review/'
> - Avoid `shell-command', which also results in better error reporting
>   when 'git-am' fails.
> 

One thing I noticed is that reviewing a patch for the second time fails
because the branch already exists. I'm not sure if this is covered under
"upkeep of the repo" but it is a little inconvenient.

Re: [PATCH v2] emacs: Helpers for notmuch developers.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner <david@tethera.net> wrote:
> On Fri,  6 Jan 2012 10:03:19 +0000, David Edmondson <dme@dme.org> wrote:
> > ---
> > 
> > - Prefix the branch name with 'review/'
> > - Avoid `shell-command', which also results in better error reporting
> >   when 'git-am' fails.
> > 
> 
> One thing I noticed is that reviewing a patch for the second time fails
> because the branch already exists. I'm not sure if this is covered under
> "upkeep of the repo" but it is a little inconvenient.

Oh. It's supposed to delete the existing branch. It did in my test. What
happens for you?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] emacs: Helpers for notmuch developers.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

On Sat, 07 Jan 2012 00:56:19 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> What do you think of this approach related to your way cloning the
> repo and then deleting/creating the branch. Just that developer may
> mess with the repository contents and then there is tedious working
> tree cleanup to be done (especially those who are not so fluent using
> git).

This sounds good - I learn more about git all of the time. I'll try to
apply some of your suggestions.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2] emacs: Helpers for notmuch developers.

[David Bremner]

On Mon, 09 Jan 2012 08:23:53 +0000, David Edmondson <dme@dme.org> wrote:
> On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner <david@tethera.net> wrote:
> > On Fri,  6 Jan 2012 10:03:19 +0000, David Edmondson <dme@dme.org> wrote:
> > > ---
> > > 
> > > - Prefix the branch name with 'review/'
> > > - Avoid `shell-command', which also results in better error reporting
> > >   when 'git-am' fails.
> > > 
> > 
> > One thing I noticed is that reviewing a patch for the second time fails
> > because the branch already exists. I'm not sure if this is covered under
> > "upkeep of the repo" but it is a little inconvenient.
> 
> Oh. It's supposed to delete the existing branch. It did in my test. What
> happens for you?

FWIW, I loaded notmuch-dev.el on top of current master.

The magit buffer shows 

$ git --no-pager checkout -b review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results master
fatal: A branch named 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results' already exists.

The backtrace is 

  signal(error ("Git failed"))
  error("Git failed")
  magit-run*(("git" "--no-pager" "checkout" "-b" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master"))
  #[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4]()
  magit-refresh-wrapper(#[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4])
  magit-run-git("checkout" "-b" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master")
  magit-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master")
  notmuch-dev-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
  (let ((default-directory notmuch-dev-temporary-repository-path)) (notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch patch-name) (error nil)) (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path (erase-buffer) (call-process notmuch-command nil t nil "show" "--format=mbox" search-terms)) (magit-run* (list magit-git-executable "am" mbox-path)) (magit-status notmuch-dev-temporary-repository-path))
  (let ((patch-name ...) (mbox-path ...)) (notmuch-dev-make-temporary-repository) (let (...) (notmuch-dev-checkout-master) (condition-case nil ... ...) (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) (magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
  notmuch-dev-review-patch("[PATCH] emacs: Don't signal an error when reaching the end of the search results." "id:\"1324370714-28545-1-git-send-email-dme@dme.org\"")
  notmuch-dev-show-review-patch()
  call-interactively(notmuch-dev-show-review-patch t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)

Re: [PATCH v2] emacs: Helpers for notmuch developers.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]

On Mon, 09 Jan 2012 06:38:54 -0400, David Bremner <david@tethera.net> wrote:
> > Oh. It's supposed to delete the existing branch. It did in my test. What
> > happens for you?
> 
> FWIW, I loaded notmuch-dev.el on top of current master.
> 
> The magit buffer shows 
> 
> $ git --no-pager checkout -b review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results master
> fatal: A branch named 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results' already exists.
> 
> The backtrace is 
> 
>   signal(error ("Git failed"))
>   error("Git failed")
>   magit-run*(("git" "--no-pager" "checkout" "-b" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master"))
>   #[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4]()
>   magit-refresh-wrapper(#[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4])
>   magit-run-git("checkout" "-b" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master")
>   magit-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results" "master")
>   notmuch-dev-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
>   (let ((default-directory notmuch-dev-temporary-repository-path)) (notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch patch-name) (error nil)) (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path (erase-buffer) (call-process notmuch-command nil t nil "show" "--format=mbox" search-terms)) (magit-run* (list magit-git-executable "am" mbox-path)) (magit-status notmuch-dev-temporary-repository-path))
>   (let ((patch-name ...) (mbox-path ...)) (notmuch-dev-make-temporary-repository) (let (...) (notmuch-dev-checkout-master) (condition-case nil ... ...) (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) (magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
>   notmuch-dev-review-patch("[PATCH] emacs: Don't signal an error when reaching the end of the search results." "id:\"1324370714-28545-1-git-send-email-dme@dme.org\"")
>   notmuch-dev-show-review-patch()
>   call-interactively(notmuch-dev-show-review-patch t nil)
>   execute-extended-command(nil)
>   call-interactively(execute-extended-command nil nil)

Could you try running
      (notmuch-dev-delete-branch "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
when inside that repository please?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2] emacs: Helpers for notmuch developers.

[David Bremner]

On Mon, 09 Jan 2012 11:16:20 +0000, David Edmondson <dme@dme.org> wrote:
> On Mon, 09 Jan 2012 06:38:54 -0400, David Bremner <david@tethera.net> wrote:
> Could you try running
>       (notmuch-dev-delete-branch "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
> when inside that repository please?

Sorry for the delay.

zancas:/tmp/notmuch-dev-bremner  (git)-[master]-% git branch   
* master
  review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results

It looks likes somebody needs to use git branch -D?




Debugger entered--Lisp error: (error "The branch 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results' is not fully merged.")
  signal(error ("The branch 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results' is not fully merged."))
  error("The branch 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results' is not fully merged.")
  magit-run*(("git" "--no-pager" "branch" "-d" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"))
  #[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4]()
  magit-refresh-wrapper(#[nil "\303\304\b	B\n\"!\207" [magit-git-executable magit-git-standard-options args magit-run* append] 4])
  magit-run-git("branch" "-d" "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
  magit-delete-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
  notmuch-dev-delete-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
  eval((notmuch-dev-delete-branch "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"))
  eval-last-sexp-1(nil)

[PATCH] notmuch-dev: Forcibly delete branches.

[David Edmondson]

`magit-delete-branch' runs "git branch -d <branch>", which refuses to
delete branches that are not fully merged. Given that we don't care
and don't want to be stalled, use "git branch -D <branch>" directly.
---

This should address the problems removing un-merged branches.

 emacs/notmuch-dev.el |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-dev.el b/emacs/notmuch-dev.el
index ac427ec..871ce3b 100644
--- a/emacs/notmuch-dev.el
+++ b/emacs/notmuch-dev.el
@@ -64,7 +64,9 @@
   (magit-checkout "master"))
 
 (defun notmuch-dev-delete-branch (name)
-  (magit-delete-branch name))
+  ;; `magit-delete-branch' uses "-d", which is not sufficiently
+  ;; aggressive for us.
+  (magit-run-git "branch" "-D" name))
 
 (defun notmuch-dev-create-branch (name)
   ;; Switches to the new branch automatically.
-- 
1.7.7.3

Re: [PATCH] notmuch-dev: Forcibly delete branches.

[David Bremner]

On Fri, 13 Jan 2012 10:25:29 +0000, David Edmondson <dme@dme.org> wrote:
> `magit-delete-branch' runs "git branch -d <branch>", which refuses to
> delete branches that are not fully merged. Given that we don't care
> and don't want to be stalled, use "git branch -D <branch>" directly.
> ---

Yes, that seems to fix it. 

Two more questions/comments.

- Should n-d-s-review-patch update to current master? perhaps with
  prefix?

- I think more escaping of branch names is needed. In particular, branch
  names with {} in them get created ok, but then don't work very well in
  magit.  E.g. running log I get "Args out of range: "face}s", 5, 10"
  with the branch name

       review/patch-v2-emacs-logically-group-def{custom,face}s

d

[PATCH 1/2] notmuch-dev: Remove more characters from branch names.

[David Edmondson]

Remove {, } and ,.
---
 emacs/notmuch-dev.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-dev.el b/emacs/notmuch-dev.el
index 871ce3b..465846f 100644
--- a/emacs/notmuch-dev.el
+++ b/emacs/notmuch-dev.el
@@ -75,7 +75,7 @@
 (defun notmuch-dev-flatten-title (title)
   (let* ((s (downcase title))
 	 (s (replace-regexp-in-string "[ \t/]+" "-" s))
-	 (s (replace-regexp-in-string "[\]\[\":]" "" s))
+	 (s (replace-regexp-in-string "[\]\[\"{}:,]" "" s))
 	 (s (replace-regexp-in-string "\\.$" "" s))
 	 )
     s))
-- 
1.7.7.3

[PATCH 2/2] notmuch-dev: Update the master branch when requested.

[David Edmondson]

If `notmuch-dev-show-review-patch' is called with a prefix argument,
pull updates for the 'master' branch of the temporary repository.
---
 emacs/notmuch-dev.el |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-dev.el b/emacs/notmuch-dev.el
index 465846f..3a15bf4 100644
--- a/emacs/notmuch-dev.el
+++ b/emacs/notmuch-dev.el
@@ -61,7 +61,12 @@
       (error "git clone failed."))))
 
 (defun notmuch-dev-checkout-master ()
-  (magit-checkout "master"))
+  (magit-checkout "master")
+  (when current-prefix-arg
+    (message "Updating master...")
+    ;; Don't use `magit-pull' because it runs asynchronously.
+    (magit-run-git "pull" "-v")
+    (message "Updating master...done.")))
 
 (defun notmuch-dev-delete-branch (name)
   ;; `magit-delete-branch' uses "-d", which is not sufficiently
@@ -108,6 +113,7 @@
     (let ((default-directory notmuch-dev-temporary-repository-path))
 
       (notmuch-dev-checkout-master)
+
       ;; Delete the branch if it exists.
       (condition-case nil
 	  (notmuch-dev-delete-branch patch-name)
-- 
1.7.7.3

Re: [PATCH 2/2] notmuch-dev: Update the master branch when requested.

[Xavier Maillard]

Hi David,

Have you planned to cook something about this in notmuch wiki ?
That sounds like something I could use to test patches more easily than
I do currently.

Note: I am totally a dummy when I have to use git :/

Regards

/Xavier

Re: [PATCH 2/2] notmuch-dev: Update the master branch when requested.

[David Edmondson]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

On Fri, 13 Jan 2012 22:03:17 +0100, Xavier Maillard <xavier@maillard.im> wrote:
> Have you planned to cook something about this in notmuch wiki ?
> That sounds like something I could use to test patches more easily than
> I do currently.

I'll do so.

> Note: I am totally a dummy when I have to use git :/

Me too, hence the wrapper!

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/2] notmuch-dev: Update the master branch when requested.

[Xavier Maillard]

On Mon, 16 Jan 2012 09:07:52 +0000, David Edmondson <dme@dme.org> wrote:
> On Fri, 13 Jan 2012 22:03:17 +0100, Xavier Maillard <xavier@maillard.im> wrote:
> > Have you planned to cook something about this in notmuch wiki ?
> > That sounds like something I could use to test patches more easily than
> > I do currently.
> 
> I'll do so.

If I can help, just tell :D

/Xavier