GNU bug report logs - #35362
26.2; [debbugs.el] Automate commit -> debbugs flow (posting patches, closing bugs after pushing)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35362 in the body.
You can then email your comments to 35362 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.2; [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sun, 21 Apr 2019 10:56:11 -0400

Severity: wishlist

As I said in Bug#33225 (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33225#35)

    I have in mind next to bring in my commands which produce control
    messages from git commits (both for attaching proposed patches from
    an unpushed commit, and closing from a pushed one).

There was a bit more delay (as always) due to having more hardcoded
things in my code than I had realized, but I think it's just about ready
now for other people to play with.

Patch to follow when I get a bug number (so that I can use the new code
to post itself, of course).

Message #8 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sun, 21 Apr 2019 11:23:54 -0400

[Message part 1 (text/plain, inline)]

tags 35362 + patch
quit

> Patch to follow when I get a bug number (so that I can use the new code
> to post itself, of course).

Here it is.

[0001-Automate-commit-debbugs-workflow-Bug-35362.patch (text/x-diff, inline)]

From 9b3b3b79d257850279bec2c35dd255f25765eb56 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> users.sourceforge.net>
Date: Sun, 21 Apr 2019 11:11:30 -0400
Subject: [PATCH] Automate commit -> debbugs workflow (Bug#35362)

* packages/debbugs/debbugs-gnu.el (debbugs-gnu-current-id): Return nil
in buffers not in debbugs-gnu-mode.
(debbugs-gnu-send-control-message): Don't remove bugs cache info here...
(debbugs-gnu-make-control-message): ...do it here instead (using
message-send-actions to wait for successful message send).
(debbugs-gnus-jump-to-bug, debbugs-gnu-git-remote-info-alist)
(debbugs-gnu-commit-description-format, debbugs-gnu--git-insert)
(debbugs-gnu--git-remote-info, debbugs-gnu--git-get-pushed-to)
(debbugs-gnu-announce-commit, debbugs-gnu-post-patch)
(debbugs-gnu-read-commit-range-hook)
(debbugs-gnu-read-commit-range-from-vc-log)
(debbugs-gnu-picked-commits, debbugs-gnu-pick-commits)
(debbugs-gnu-maybe-use-picked-commits)
(debbugs-gnu--prepare-to-use-picked-commits): New commands, functions
and variables.
* packages/debbugs/debbugs-ug.texi (Posting Patches): New section.
---
 packages/debbugs/debbugs-gnu.el  | 284 ++++++++++++++++++++++++++++++++++++++-
 packages/debbugs/debbugs-ug.texi |  26 ++++
 2 files changed, 304 insertions(+), 6 deletions(-)

diff --git a/packages/debbugs/debbugs-gnu.el b/packages/debbugs/debbugs-gnu.el
index 997d367b0..8bdecd9fc 100644
--- a/packages/debbugs/debbugs-gnu.el
+++ b/packages/debbugs/debbugs-gnu.el
@@ -180,12 +180,16 @@ (autoload 'rmail-summary "rmailsum")
 (autoload 'vc-dir-hide-up-to-date "vc-dir")
 (autoload 'vc-dir-mark "vc-dir")
 
+(declare-function log-view-current-entry "log-view" (&optional pos move))
+(declare-function log-view-current-tag "log-view" (&optional pos))
+
 (defvar compilation-in-progress)
 (defvar diff-file-header-re)
 (defvar gnus-article-buffer)
 (defvar gnus-posting-styles)
 (defvar gnus-save-duplicate-list)
 (defvar gnus-suppress-duplicates)
+(defvar message-sent-message-via)
 (defvar rmail-current-message)
 (defvar rmail-mode-map)
 (defvar rmail-summary-mode-map)
@@ -1282,7 +1286,8 @@ (defun debbugs-gnu-current-id (&optional noerror)
 	   (error "No bug on the current line"))))
 
 (defun debbugs-gnu-current-status ()
-  (get-text-property (line-beginning-position) 'tabulated-list-id))
+  (when (derived-mode-p 'debbugs-gnu-mode)
+    (get-text-property (line-beginning-position) 'tabulated-list-id)))
 
 (defun debbugs-gnu-display-status (query filter status)
   "Display the query, filter and status of the report on the current line."
@@ -1357,8 +1362,10 @@ (defun debbugs-read-emacs-bug-with-gnus (id status merged)
   ;; Use Gnus.
   (gnus-read-ephemeral-emacs-bug-group
    (cons id (if (listp merged) merged (list merged)))
-   (cons (current-buffer)
-	 (current-window-configuration)))
+   ;; This doesn't work, gives wrong-type-argument listp when quitting.
+   ;; (cons (current-buffer)
+   ;;       (current-window-configuration))
+   )
   (with-current-buffer (window-buffer (selected-window))
     (set (make-local-variable 'debbugs-gnu-bug-number) id)
     (set (make-local-variable 'debbugs-gnu-subject)
@@ -1534,7 +1541,6 @@ (defun debbugs-gnu-send-control-message (message &optional reverse)
       (debbugs-gnu-make-control-message
        message id reverse (current-buffer))
       (funcall (or debbugs-gnu-send-mail-function send-mail-function))
-      (remhash id debbugs-cache-data)
       (message-goto-body)
       (message "Control message sent:\n%s"
                (buffer-substring-no-properties (point) (1- (point-max)))))))
@@ -1720,8 +1726,274 @@ (defun debbugs-gnu-make-control-message (message bugid &optional reverse buffer)
                  bugid (if reverse ?- ?+)
                  message)))))
     (unless (looking-at-p debbugs-gnu-control-message-end-regexp)
-      (insert "quit\n\n"))))
-
+      (insert "quit\n\n"))
+    (add-hook 'message-send-actions
+              (lambda () (remhash bugid debbugs-cache-data))
+              nil t)))
+
+(defun debbugs-gnus-jump-to-bug (bugid)
+  "Display buffer associated with BUGID with `pop-to-buffer'.
+Use `gnus-read-ephemeral-emacs-bug-group' instead if there is no such buffer."
+  (let ((bug-buf nil)
+        ;; By reverse order of preference.
+        (preferred-modes '(gnus-summary-mode gnus-article-mode message-mode)))
+    (save-current-buffer
+      (cl-loop
+       for buf in (buffer-list)
+       while preferred-modes do
+       (set-buffer buf)
+       (when-let (((memql bugid (debbugs-gnus-implicit-ids)))
+                  (mode (cl-loop
+                         for mode in preferred-modes
+                         thereis (and (derived-mode-p mode)
+                                      ;; Don't choose sent message buffers.
+                                      (or (not (eq mode 'message-mode))
+                                          (not message-sent-message-via))
+                                      mode))))
+         (setq preferred-modes (cdr (memq mode preferred-modes)))
+         (setq bug-buf buf))))
+    (if bug-buf
+        (pop-to-buffer bug-buf '(display-buffer-reuse-window
+                                 . ((reusable-frames . visible))))
+      (gnus-read-ephemeral-emacs-bug-group bugid (current-window-configuration)))))
+
+(defcustom debbugs-gnu-git-remote-info-alist
+  '(("git.sv.gnu.org\\(?::/srv/git\\)/emacs.git" .
+     ((commit-url
+       . "https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=%H")
+      (ref-globs . ("/emacs-*" "/master"))))
+    ("git.sv.gnu.org\\(?::/srv/git\\)/emacs/elpa" .
+     ((commit-url
+       . "https://git.savannah.gnu.org/cgit/emacs/elpa.git/commit/?id=%H"))))
+  "Nest alist for repository-specific information.
+Each element has the form (REMOTE-REGEXP . INFO-ALIST), where
+INFO-ALIST is an alist containing the repository attributes.
+
+Supported keys of INFO-ALIST are
+
+* `commit-url': Format of a URL for a given commit hash, using
+  format specifiers supported by `git show'.  Used by
+  `debbugs-gnu-announce-commit' as a supplement to
+  `debbugs-gnu-commit-description-format'.
+
+* `ref-globs': List of glob patterns matching branches of
+  interest, used by `debbugs-gnu-announce-commit' to make the
+  \"Pushed to X\" message."
+  :type '(alist :key-type string :value-type (alist :key-type symbol)))
+
+(defcustom debbugs-gnu-commit-description-format
+  "%h %cI \"%s\""
+  "Format used for describing commits in `debbugs-gnu-announce-commit'.
+It is passed as --format argument to `git show', see its manual
+page for formatting specifier meanings."
+  :type 'string)
+
+(defcustom debbugs-gnu-git-program (or (bound-and-true-p vc-git-program) "git")
+  "Name of the Git executable (excluding any arguments)."
+  :type 'string)
+
+(defun debbugs-gnu--git-insert (&rest args)
+  "Insert output of running git with ARGS.
+Throws error if git returns non-zero.  Uses `debbugs-gnu-git-program'."
+  (unless (eql 0 (apply #'process-file
+                        debbugs-gnu-git-program nil '(t t) nil
+                        args))
+    (error "git %s failed: %s" (car args) (buffer-string))))
+
+(defun debbugs-gnu--git-remote-info ()
+  "Return (REMOTE . INFO-ALIST)."
+  (with-temp-buffer
+    (debbugs-gnu--git-insert "remote" "-v")
+    (catch 'found-remote
+      (dolist (remote-info debbugs-gnu-git-remote-info-alist)
+        (goto-char (point-min))
+        (and (re-search-forward (car remote-info) nil t)
+             (progn (beginning-of-line)
+                    (looking-at "[^ \t]+"))
+             (throw 'found-remote
+                    (cons (match-string 0) (cdr remote-info))))))))
+
+(defun debbugs-gnu--git-get-pushed-to (commit-range remote-info)
+  "Return the branch name which COMMIT-RANGE was pushed to.
+REMOTE-INFO is return value of `debbugs-gnu--git-remote-info'."
+  (let* ((last-commit
+          (with-temp-buffer
+            (debbugs-gnu--git-insert
+             ;; %H: commit hash.
+             "log" "-1" "--format=%H" commit-range)
+            (goto-char (point-min))
+            (buffer-substring (point-min) (line-end-position))))
+         (remote (pop remote-info)))
+    (let ((ref-globs (cdr (assq 'ref-globs remote-info))))
+      (with-temp-buffer
+        (apply
+         #'debbugs-gnu--git-insert
+         "branch" "--remote" "--contains" last-commit
+         (mapcar (lambda (glob) (concat remote glob))
+                 ref-globs))
+        ;; First 2 characters are current branch indicator.
+        (goto-char (+ (point-min) 2))
+        (and (looking-at (concat (regexp-quote remote) "/\\(.+\\)$"))
+             (match-string 1))))))
+
+(defun debbugs-gnu-announce-commit (commit-range bugnum &optional _args)
+  "Insert info about COMMIT-RANGE into message.
+Optionally call `debbugs-gnu-make-control-message' to close BUGNUM."
+  (let* ((status (car (debbugs-get-status bugnum)))
+         (packages (cdr (assq 'package status)))
+         (remote-info (debbugs-gnu--git-remote-info)))
+    (insert "\nPushed to "
+            (or (debbugs-gnu--git-get-pushed-to commit-range remote-info) "")
+            ".\n\n")
+    (debbugs-gnu--git-insert
+     "show" "--no-patch"
+     (concat "--format=" debbugs-gnu-commit-description-format
+             "\n" (cdr (assq 'commit-url remote-info)) "\n")
+     commit-range)
+    (when (y-or-n-p "Close bug? ")
+      (let ((emacs-version
+             (or (and (member "emacs" packages)
+                      (file-exists-p "configure.ac")
+                      (with-temp-buffer
+                        (insert-file-contents "configure.ac")
+                        (and (re-search-forward "\
+^ *AC_INIT(GNU Emacs, *\\([0-9.]+\\), *bug-gnu-emacs <at> gnu.org"
+                                                nil t)
+                             (match-string 1))))
+                 "")))
+        (debbugs-gnu-make-control-message
+         "done" bugnum nil (current-buffer))))))
+
+(defun debbugs-gnu-post-patch (commit-range bugnum &optional format-patch-args)
+  "Attach COMMIT-RANGE as patches into current message.
+Optionally call `debbugs-gnu-make-control-message'' to tag BUGNUM
+with `patch'."
+  (letrec ((disposition (completing-read "disposition: " '("inline" "attachment")))
+           ;; Make attachments text/plain for better compatibility
+           ;; (e.g., opening in browser instead of downloading).
+           (type (if (equal disposition "inline") "text/x-diff" "text/plain"))
+           (dir (make-temp-file (format "patches-for-bug%d-" bugnum) t))
+           (deldir (lambda ()
+                     (delete-directory dir t)
+                     (remove-hook 'message-exit-actions deldir t)
+                     (remove-hook 'kill-buffer-hook deldir t))))
+    (add-hook 'message-send-actions deldir nil t)
+    (add-hook 'kill-buffer-hook deldir nil t)
+    (with-temp-buffer
+      (apply #'debbugs-gnu--git-insert
+             "format-patch" (concat "--output-directory=" dir)
+             (append format-patch-args
+                     (list commit-range))))
+    (dolist (patch (directory-files dir t "\\`[^.]"))
+      (mml-attach-file patch type "patch" disposition))
+    (when (and (not (member
+                     "patch" (assq 'tags (car (debbugs-get-status
+                                               bugnum)))))
+               (y-or-n-p "Tag + patch? "))
+      (debbugs-gnu-make-control-message
+       "patch" bugnum nil (current-buffer)))))
+
+(defvar debbugs-gnu-read-commit-range-hook nil
+  "Used by `debbugs-gnu-pick-commits'.
+Each function receives no arguments, and should return an
+argument compatible with `debbugs-gnu-pick-commits'.  If the
+function can't function in the current buffer, it should return
+nil to let the next function try.")
+
+(defun debbugs-gnu-read-commit-range-from-vc-log ()
+  "Read commit range from a VC log buffer.
+Return commit at point, or commit range in region if it is
+active.  This function is suitable for use in
+`debbugs-gnu-read-commit-range-hook'."
+  (when (derived-mode-p 'vc-git-log-view-mode)
+    (list (if (use-region-p)
+              (let ((beg (log-view-current-entry (region-beginning)))
+                    (end (log-view-current-entry (region-end))))
+                (if (= (car beg) (car end))
+                    ;; Region spans only a single entry.
+                    (cadr beg)
+                  ;; Later revs are at the top of buffer.
+                  (format "%s~1..%s" (cadr end) (cadr beg))))
+            (log-view-current-tag)))))
+(add-hook 'debbugs-gnu-read-commit-range-hook
+          #'debbugs-gnu-read-commit-range-from-vc-log)
+
+(defvar debbugs-gnu-picked-commits nil
+  "List of commits selected in `debbugs-gnu-pick-commits'.
+Format of each element is (BUGNUMBERS REPO-DIR COMMIT-RANGE).")
+
+(defun debbugs-gnu-pick-commits (commit-range)
+  "Select COMMIT-RANGE to post as patches or announce as pushed.
+COMMIT-RANGE is read using `debbugs-gnu-read-commit-range-hook',
+or `read-string' if none of its functions apply.  Add entry to
+`debbugs-gnu-pick-commits' and jump to read bug in prepration for
+user to call `debbugs-gnu-maybe-use-picked-commits'."
+  (interactive
+   (or (run-hook-with-args-until-success
+        'debbugs-gnu-read-commit-range-hook)
+       (list (read-string "Commit (or range): "))))
+  (let ((bugnum nil)
+        (repo-dir default-directory))
+    (with-temp-buffer
+      ;; %B = raw body (unwrapped subject and body)
+      (debbugs-gnu--git-insert
+       ;; %B: raw body (unwrapped subject and body).
+       "show" "--no-patch" "--format=%B" commit-range)
+      (goto-char (point-min))
+      (while (re-search-forward "[bB]ug ?#\\([0-9]+\\)" nil t)
+        (push (match-string 1) bugnum)))
+    (let ((read-bugnum
+           (string-to-number
+            (completing-read
+             (if bugnum
+                 (format "Bug # (default %s): " (car bugnum))
+               "Bug #: ")
+             debbugs-gnu-completion-table nil t nil nil bugnum))))
+      (debbugs-gnus-jump-to-bug read-bugnum)
+      (cl-callf2 mapcar #'string-to-number bugnum)
+      (unless (memql read-bugnum bugnum)
+        (push read-bugnum bugnum)))
+    (push (list bugnum repo-dir commit-range)
+          debbugs-gnu-picked-commits)
+    (if (derived-mode-p 'message-mode)
+        (debbugs-gnu-maybe-use-picked-commits)
+      (message "Reply to a message to continue"))))
+
+(defun debbugs-gnu-maybe-use-picked-commits ()
+  "Add commit corresponding to current message's bug number.
+Calls `debbugs-gnu-announce-commit' or `debbugs-gnu-post-patch'
+on an entry with a matching bug number from
+`debbugs-gnu-picked-commits'.  Remove entry after message is
+successfully sent."
+  (interactive)
+  (when (derived-mode-p 'message-mode)
+    (cl-loop with id = (car (debbugs-gnus-implicit-ids))
+             for pcomm-entry in debbugs-gnu-picked-commits
+             for (bugnum repo-dir commit-range) = pcomm-entry
+             when (memql id bugnum)
+             do
+             (goto-char (point-max))
+             (let ((default-directory repo-dir))
+               (pcase (read-char-choice
+                       (format "[a]nnounce commit, or [p]ost patch? (%s)"
+                               commit-range)
+                       '(?a ?p))
+                 (?a (debbugs-gnu-announce-commit commit-range id) )
+                 (?p (debbugs-gnu-post-patch commit-range id))))
+             (add-hook 'message-send-actions
+                       (lambda ()
+                         (cl-callf2 delq pcomm-entry
+                                    debbugs-gnu-picked-commits))
+                       nil t)
+             (remove-hook 'post-command-hook
+                          #'debbugs-gnu-maybe-use-picked-commits)
+             (cl-return))))
+
+;; We need to daisy chain the hooks because `message-setup-hook' runs
+;; too early (before `message-yank-original').
+(defun debbugs-gnu--prepare-to-use-picked-commits ()
+  (add-hook 'post-command-hook #'debbugs-gnu-maybe-use-picked-commits))
+(add-hook 'message-setup-hook #'debbugs-gnu--prepare-to-use-picked-commits)
 
 (defvar debbugs-gnu-usertags-mode-map
   (let ((map (make-sparse-keymap)))
diff --git a/packages/debbugs/debbugs-ug.texi b/packages/debbugs/debbugs-ug.texi
index 2bb01d2c2..0ea7ca521 100644
--- a/packages/debbugs/debbugs-ug.texi
+++ b/packages/debbugs/debbugs-ug.texi
@@ -313,6 +313,7 @@ in @code{org-mode}.
 * TODO Items::                  TODO Items.
 * Control Messages::            Control Messages.
 * Applying Patches::            Applying Patches in the Emacs Repository.
+* Posting Patches::             Posting Patches to Debbugs from the Emacs Repository.
 @end menu
 
 
@@ -674,6 +675,31 @@ creates a ChangeLog entry with all needed information.  A final
 @kbd{M-m} in the @samp{ChangeLog} buffer commits the patch via
 @samp{*vc-log*}.
 
+@node Posting Patches
+@section Posting Patches to Debbugs from the Emacs Repository
+
+Once you have committed a patch fixing a bug you usually want to post
+it to the bug thread for review and testing.  And when the patch is
+deemed satisfactory and pushed to the official GNU Emacs repository,
+the bug should be marked closed.
+
+@findex debbugs-gnu-pick-commits
+The command @code{debbugs-gnu-pick-commits} helps automate both these
+processes: it queries for a commit (or commit range), and a bug number
+(defaulting to the bug number mentioned in the commit message).  It
+then jumps you to a buffer associated with the bug.  When you reply to
+a message in the bug thread, you are asked whether to post the commits
+as patches (optionally tagging the bug with @code{"patch"}), or
+announce that the bug has been fixed by the selected commits
+(optionally closing the bug and marking as closed in the Emacs version
+corresponding to the patch).
+
+@vindex debbugs-gnu-read-commit-range-hook
+The query for commit (or commit range) to use is controlled by
+@code{debbugs-gnu-read-commit-range-hook}.  Initially it has an entry
+which operates in @samp{*vc-change-log*} buffers: the default will be
+the commit under point, or the commit range contained in the region if
+it is active.
 
 @node Minor Mode
 @chapter Minor Mode
-- 
2.11.0

Message #13 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sun, 21 Apr 2019 11:27:57 -0400

[Message part 1 (text/plain, inline)]

> -   (cons (current-buffer)
> -	 (current-window-configuration)))
> +   ;; This doesn't work, gives wrong-type-argument listp when quitting.
> +   ;; (cons (current-buffer)
> +   ;;       (current-window-configuration))
> +   )

Ooops, sorry, I was confused about this.  This part is fine.

> +      (gnus-read-ephemeral-emacs-bug-group bugid (current-window-configuration)))))

It's this one that is missing the cons.  Patch updated.

[0001-Automate-commit-debbugs-workflow-Bug-35362.patch (text/plain, attachment)]

Message #16 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sun, 21 Apr 2019 21:58:51 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

Hi Noam,

I haven't tried your patch yet. Just some first comments while reading it:

All your defcustoms miss a :version tag. I guess, "27.1" is proper.

>    ;; Use Gnus.
>    (gnus-read-ephemeral-emacs-bug-group
>     (cons id (if (listp merged) merged (list merged)))
> +   This doesn't work, gives wrong-type-argument listp when quitting.

Comment chars are missing. Leftover from editing?

> +(defun debbugs-gnus-jump-to-bug (bugid)
> +  "Display buffer associated with BUGID with `pop-to-buffer'.
> +Use `gnus-read-ephemeral-emacs-bug-group' instead if there is no such buffer."

debbugs-gnu.el supports both gnus and rmail. Do we miss something for
rmail then?

(I'm not an rmail user, but Eli is.)

> +(defun debbugs-gnu--git-insert (&rest args)
> +  "Insert output of running git with ARGS.
> +Throws error if git returns non-zero.  Uses `debbugs-gnu-git-program'."
> +  (unless (eql 0 (apply #'process-file
> +                        debbugs-gnu-git-program nil '(t t) nil
> +                        args))
> +    (error "git %s failed: %s" (car args) (buffer-string))))

Why not `vc-git--call'?

> +    (debbugs-gnu--git-insert "remote" "-v")

> +            (debbugs-gnu--git-insert
> +             ;; %H: commit hash.
> +             "log" "-1" "--format=%H" commit-range)

> +        (apply
> +         #'debbugs-gnu--git-insert
> +         "branch" "--remote" "--contains" last-commit
> +         (mapcar (lambda (glob) (concat remote glob))
> +                 ref-globs))

> +    (debbugs-gnu--git-insert
> +     "show" "--no-patch"
> +     (concat "--format=" debbugs-gnu-commit-description-format
> +             "\n" (cdr (assq 'commit-url remote-info)) "\n")
> +     commit-range)


> +      (apply #'debbugs-gnu--git-insert
> +             "format-patch" (concat "--output-directory=" dir)
> +             (append format-patch-args
> +                     (list commit-range))))

> +      (debbugs-gnu--git-insert
> +       ;; %B: raw body (unwrapped subject and body).
> +       "show" "--no-patch" "--format=%B" commit-range)

Aren't there respective vc-git functions we could reuse?

> +@node Posting Patches
> +@section Posting Patches to Debbugs from the Emacs Repository
> +
> +Once you have committed a patch fixing a bug you usually want to post
> +it to the bug thread for review and testing.  And when the patch is
> +deemed satisfactory and pushed to the official GNU Emacs repository,
> +the bug should be marked closed.

Currently, you are supporting both Emacs and ELPA repositories, aren't
you? Please sday so.

> +@findex debbugs-gnu-pick-commits
> +The command @code{debbugs-gnu-pick-commits} helps automate both these
> +processes: it queries for a commit (or commit range), and a bug number
> +(defaulting to the bug number mentioned in the commit message).  It
> +then jumps you to a buffer associated with the bug.  When you reply to
> +a message in the bug thread, you are asked whether to post the commits
> +as patches (optionally tagging the bug with @code{"patch"}), or
> +announce that the bug has been fixed by the selected commits
> +(optionally closing the bug and marking as closed in the Emacs version
> +corresponding to the patch).
> +
> +@vindex debbugs-gnu-read-commit-range-hook
> +The query for commit (or commit range) to use is controlled by
> +@code{debbugs-gnu-read-commit-range-hook}.  Initially it has an entry
> +which operates in @samp{*vc-change-log*} buffers: the default will be
> +the commit under point, or the commit range contained in the region if
> +it is active.

Reading this doesn't give me a clear picture what happens. Could you add
an example?

Also, pls describe at least `debbugs-gnu-git-remote-info-alist'. Other
projects but Emacs might profit from.

Best regards, Michael.

Message #19 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Mon, 22 Apr 2019 15:42:31 -0400

Michael Albinus <michael.albinus <at> gmx.de> writes:

> All your defcustoms miss a :version tag. I guess, "27.1" is proper.

Oh, hmm.  Shouldn't the version correspond to the debbugs.el package
version, since it's not tied to the Emacs version as such?

>>    ;; Use Gnus.
>>    (gnus-read-ephemeral-emacs-bug-group
>>     (cons id (if (listp merged) merged (list merged)))
>> +   This doesn't work, gives wrong-type-argument listp when quitting.
>
> Comment chars are missing. Leftover from editing?

Sorry, this hunk was supposed to be dropped completely.

>> +(defun debbugs-gnus-jump-to-bug (bugid)
>> +  "Display buffer associated with BUGID with `pop-to-buffer'.
>> +Use `gnus-read-ephemeral-emacs-bug-group' instead if there is no such buffer."
>
> debbugs-gnu.el supports both gnus and rmail. Do we miss something for
> rmail then?
>
> (I'm not an rmail user, but Eli is.)

Possibly yes, I haven't used rmail so I'm not sure how to test that out.

>> +(defun debbugs-gnu--git-insert (&rest args)
>> +  "Insert output of running git with ARGS.
>> +Throws error if git returns non-zero.  Uses `debbugs-gnu-git-program'."
>> +  (unless (eql 0 (apply #'process-file
>> +                        debbugs-gnu-git-program nil '(t t) nil
>> +                        args))
>> +    (error "git %s failed: %s" (car args) (buffer-string))))
>
> Why not `vc-git--call'?

I thought relying on a function marked as internal would not be a good
idea for an ELPA package which should potentially work across multiple
versions of Emacs.  Otherwise vc-git--call should work fine too.

>> +    (debbugs-gnu--git-insert "remote" "-v")
>
>> +            (debbugs-gnu--git-insert
>> +             ;; %H: commit hash.
>> +             "log" "-1" "--format=%H" commit-range)
>
>> +        (apply
>> +         #'debbugs-gnu--git-insert
>> +         "branch" "--remote" "--contains" last-commit
>> +         (mapcar (lambda (glob) (concat remote glob))
>> +                 ref-globs))
>
>> +    (debbugs-gnu--git-insert
>> +     "show" "--no-patch"
>> +     (concat "--format=" debbugs-gnu-commit-description-format
>> +             "\n" (cdr (assq 'commit-url remote-info)) "\n")
>> +     commit-range)
>
>
>> +      (apply #'debbugs-gnu--git-insert
>> +             "format-patch" (concat "--output-directory=" dir)
>> +             (append format-patch-args
>> +                     (list commit-range))))
>
>> +      (debbugs-gnu--git-insert
>> +       ;; %B: raw body (unwrapped subject and body).
>> +       "show" "--no-patch" "--format=%B" commit-range)
>
> Aren't there respective vc-git functions we could reuse?

No, I don't see any.

>> +@node Posting Patches
>> +@section Posting Patches to Debbugs from the Emacs Repository
>> +
>> +Once you have committed a patch fixing a bug you usually want to post
>> +it to the bug thread for review and testing.  And when the patch is
>> +deemed satisfactory and pushed to the official GNU Emacs repository,
>> +the bug should be marked closed.
>
> Currently, you are supporting both Emacs and ELPA repositories, aren't
> you? Please sday so.

Yes.

>> +@findex debbugs-gnu-pick-commits
>> +The command @code{debbugs-gnu-pick-commits} helps automate both these
[...]
> Reading this doesn't give me a clear picture what happens. Could you add
> an example?
>
> Also, pls describe at least `debbugs-gnu-git-remote-info-alist'. Other
> projects but Emacs might profit from.

Okay, I've expanded the manual section (inlined below) maybe it gives
you a more clear picture (or maybe a less clear one :/).

@node Posting Patches
@section Posting Patches to Debbugs from the Emacs Repository

Once you have committed a patch fixing a bug you usually want to post
it to the bug thread for review and testing.  And when the patch is
deemed satisfactory and pushed to the official repository, the bug
should be marked closed.

@findex debbugs-gnu-pick-commits
The command @code{debbugs-gnu-pick-commits} helps automate both these
processes: it queries for a commit (or commit range), and a bug number
(defaulting to the bug number mentioned in the commit message).  It
then jumps you to a buffer associated with the bug.  When you reply to
a message in the bug thread, you are asked whether to post the commits
as patches (optionally tagging the bug with @code{"patch"}), or
announce that the bug has been fixed by the selected commits
(optionally closing the bug and marking as closed in the Emacs version
corresponding to the patch).

For example, suppose you are reading the message of ``Bug#1234:
foo-mode fails to call frobnicate on startup'' in a message buffer.
You decide to fix it, so you switch to the source code, add in the
missing call and commit locally, with the commit message ``*
lisp/foo-mode.el (foo-mode): Call frobnicate (Bug#1234).''  Use
@kbd{C-x v l} to run @code{vc-print-log}, and navigate to the new
commit.  Invoke the command @code{debbugs-gnu-pick-commits} and press
@kbd{RET} to accept the default bug number (which will be 1234 since
it's in the commit message) in response to the prompt.  You are then
popped to the message buffer, and when you reply to the message, press
@kbd{p} to post the git formatted patch as an attachment for review,
and then answer @kbd{y} to tag the bug with @code{"patch"} when the
message is sent.  Assuming you get favorable reviews, you then push
it, and again call @code{debbugs-gnu-pick-commits} but this time press
@kbd{a} (for ``announce'') after replying to the relevant bug thread
message.  This will insert some text describing the commit and where
it was pushed to, and answering @kbd{y} will arrange for the bug to be
closed when the message is sent.

@node Customizing debbugs-gnu-pick-commits
@subsection Customizing debbugs-gnu-pick-commits

@vindex debbugs-gnu-read-commit-range-hook
The query for commit (or commit range) to use is controlled by
@code{debbugs-gnu-read-commit-range-hook}.  Initially it has an entry
which operates in @samp{*vc-change-log*} buffers: the default will be
the commit under point, or the commit range contained in the region if
it is active.

@vindex debbugs-gnu-commit-description-format
@vindex debbugs-gnu-git-remote-info-alist
The string inserted to describe an announced commit is controlled by
the variable @code{debbugs-gnu-commit-description-format}, it is a
format string passed to the @code{--format} argument of @code{git
show}.  Additionally, if the remote url matches an entry in
@code{debbugs-gnu-git-remote-info-alist}, then its @code{commit-url}
subitem is appended to the commit description.  By default this
variable is configured for the GNU Emacs and GNU ELPA repositories.

Message #22 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Tue, 23 Apr 2019 10:03:39 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

Hi Noam,

>> All your defcustoms miss a :version tag. I guess, "27.1" is proper.
>
> Oh, hmm.  Shouldn't the version correspond to the debbugs.el package
> version, since it's not tied to the Emacs version as such?

I usually take the Emacs version for the :version attribute. Debbug's
own version would be better tagged with :package-version; I haven't used
this attribute, 'tho.

>> debbugs-gnu.el supports both gnus and rmail. Do we miss something for
>> rmail then?
>>
>> (I'm not an rmail user, but Eli is.)
>
> Possibly yes, I haven't used rmail so I'm not sure how to test that out.

Me neither. Let's keep it as it is, waiting for protest ... or patches.

>>> +(defun debbugs-gnu--git-insert (&rest args)
>>> +  "Insert output of running git with ARGS.
>>> +Throws error if git returns non-zero.  Uses `debbugs-gnu-git-program'."
>>> +  (unless (eql 0 (apply #'process-file
>>> +                        debbugs-gnu-git-program nil '(t t) nil
>>> +                        args))
>>> +    (error "git %s failed: %s" (car args) (buffer-string))))
>>
>> Why not `vc-git--call'?
>
> I thought relying on a function marked as internal would not be a good
> idea for an ELPA package which should potentially work across multiple
> versions of Emacs.  Otherwise vc-git--call should work fine too.

Right. However, this function is stable for years. And its
implementation looks more robust than your version.

> Once you have committed a patch fixing a bug you usually want to post
> it to the bug thread for review and testing.  And when the patch is
> deemed satisfactory and pushed to the official repository, the bug
> should be marked closed.

Well, this is about local commits, which haven't been pushed yet. Or
maybe pushed already, but in a branch. Could you pls say?

> For example, suppose you are reading the message of ``Bug#1234:

All other examples in this manual use 12345. Do we want to follow?

> Invoke the command @code{debbugs-gnu-pick-commits} and press

Should we give this command a key binding?

> @kbd{RET} to accept the default bug number (which will be 1234 since

This should be @kbd{@key{RET}}, like at other places in this manual.

> @vindex debbugs-gnu-read-commit-range-hook
> The query for commit (or commit range) to use is controlled by
> @code{debbugs-gnu-read-commit-range-hook}.  Initially it has an entry
> which operates in @samp{*vc-change-log*} buffers: the default will be
> the commit under point, or the commit range contained in the region if
> it is active.

I don't understand (yet), how the description of this variable helps. Is
the user expected to change the value herself, somehow? Otherwise, we
might to keep this description out.

> Additionally, if the remote url matches an entry in
> @code{debbugs-gnu-git-remote-info-alist}, then its @code{commit-url}
> subitem is appended to the commit description.  By default this
> variable is configured for the GNU Emacs and GNU ELPA repositories.

Maybe you add a short sentence, that it is expected that people adapt
this user option if they run an own Emacs fork, located somewhere else,
or if they host a repository for own packages.

Best regards, Michael.

Message #25 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Wed, 24 Apr 2019 22:10:52 -0400

[Message part 1 (text/plain, inline)]

Michael Albinus <michael.albinus <at> gmx.de> writes:

> I usually take the Emacs version for the :version attribute. Debbug's
> own version would be better tagged with :package-version; I haven't used
> this attribute, 'tho.

Alright.

>>> debbugs-gnu.el supports both gnus and rmail. Do we miss something for
>>> rmail then?
>>>
>>> (I'm not an rmail user, but Eli is.)
>>
>> Possibly yes, I haven't used rmail so I'm not sure how to test that out.
>
> Me neither. Let's keep it as it is, waiting for protest ... or patches.

Right, maybe later.

>>> Why not `vc-git--call'?
>>
>> I thought relying on a function marked as internal would not be a good
>> idea for an ELPA package which should potentially work across multiple
>> versions of Emacs.  Otherwise vc-git--call should work fine too.
>
> Right. However, this function is stable for years. And its
> implementation looks more robust than your version.

Yes, that's true.  Okay, switched to using it.

>> Once you have committed a patch fixing a bug you usually want to post
>> it to the bug thread for review and testing.  And when the patch is
>> deemed satisfactory and pushed to the official repository, the bug
>> should be marked closed.
>
> Well, this is about local commits, which haven't been pushed yet. Or
> maybe pushed already, but in a branch. Could you pls say?

ok

>> For example, suppose you are reading the message of ``Bug#1234:
>
> All other examples in this manual use 12345. Do we want to follow?

Ah, I suppose we may as well stay consistent.

>> Invoke the command @code{debbugs-gnu-pick-commits} and press
>
> Should we give this command a key binding?

Maybe, but then the question is what map to bind it in.  Is it okay to
start adding things into vc-git-log-view-mode-map?

>> @kbd{RET} to accept the default bug number (which will be 1234 since
>
> This should be @kbd{@key{RET}}, like at other places in this manual.

ok.

>> @vindex debbugs-gnu-read-commit-range-hook
>> The query for commit (or commit range) to use is controlled by
>> @code{debbugs-gnu-read-commit-range-hook}.  Initially it has an entry
>> which operates in @samp{*vc-change-log*} buffers: the default will be
>> the commit under point, or the commit range contained in the region if
>> it is active.
>
> I don't understand (yet), how the description of this variable helps. Is
> the user expected to change the value herself, somehow? Otherwise, we
> might to keep this description out.

The idea is you can add entries that can work with other packages, e.g.,
magit.

>> Additionally, if the remote url matches an entry in
>> @code{debbugs-gnu-git-remote-info-alist}, then its @code{commit-url}
>> subitem is appended to the commit description.  By default this
>> variable is configured for the GNU Emacs and GNU ELPA repositories.
>
> Maybe you add a short sentence, that it is expected that people adapt
> this user option if they run an own Emacs fork, located somewhere else,
> or if they host a repository for own packages.

Ok.  I didn't mention an Emacs fork as an expected variation, as that
would mean a different debbugs server too, which I really just wouldn't
expect to happen.

[0001-Automate-commit-debbugs-workflow-Bug-35362.patch (text/plain, attachment)]

Message #28 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Thu, 25 Apr 2019 14:16:33 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

Hi Noam,

I believe your patch is almost OK, so let's apply it. But of course, I
still have some few remarks :-)

>>> Invoke the command @code{debbugs-gnu-pick-commits} and press
>>
>> Should we give this command a key binding?
>
> Maybe, but then the question is what map to bind it in.  Is it okay to
> start adding things into vc-git-log-view-mode-map?

Yep. Maybe we bind "c" for it, because other candidates (d/D/p/P) seem
to be bound already.

Furthermore, we might mention debbugs-gnu-pick-commits in the
;;; Commentary section of debbugs-gnu.el.

>> I don't understand (yet), how the description of this variable helps. Is
>> the user expected to change the value herself, somehow? Otherwise, we
>> might to keep this description out.
>
> The idea is you can add entries that can work with other packages, e.g.,
> magit.

OK, this part I don't understand yet (I'm not a magit user, for
example). But I don't mind.

> --- a/packages/debbugs/debbugs-gnu.el
> +++ b/packages/debbugs/debbugs-gnu.el

> +    (add-hook 'message-send-actions
> +              (lambda () (remhash bugid debbugs-cache-data))
> +              nil t)))

Do we  need to declare message-send-actions?

> +(defun debbugs-gnus-jump-to-bug (bugid)
> +  "Display buffer associated with BUGID with `pop-to-buffer'.
> +Use `gnus-read-ephemeral-emacs-bug-group' instead if there is no such buffer."

Maybe you could add at least a FIXME for the lack of rmail support.

> +(defun debbugs-gnu--git-insert (&rest args)
> +  "Insert output of running git with ARGS.
> +Throws error if git returns non-zero.  Uses `vc-git-program'."

Don't mention vc-git-program.

> +  (unless (eql 0 (apply #'vc-git--call '(t t) args))

Declare vc-git--call.

> +(defun debbugs-gnu--git-remote-info ()
> +  "Return (REMOTE . INFO-ALIST)."

Say, that REMOTE is a string, which must be matched by REMOTE-REGEXP of
debbugs-gnu-git-remote-info-alist. Refer to the docstring of that user option.

> +           (dir (make-temp-file (format "patches-for-bug%d-" bugnum) t))

I'm curious: Why a trailing "-" in the buffer name?

> +           (deldir (lambda ()
> +                     (delete-directory dir t)
> +                     (remove-hook 'message-exit-actions deldir t)

Do we  need to declare message-exit-actions?

> +    (dolist (patch (directory-files dir t "\\`[^.]"))
> +      (mml-attach-file patch type "patch" disposition))

Do we  need to declare mml-attach-file?

> +`debbugs-gnu-pick-commits' and jump to read bug in prepration for

Typo.

> --- a/packages/debbugs/debbugs-ug.texi
> +++ b/packages/debbugs/debbugs-ug.texi

> +The string inserted to describe an announced commit is controlled by
> +the variable @code{debbugs-gnu-commit-description-format}, it is a

It is a user option.

> +subitem is appended to the commit description.  By default this
> +variable is configured for the GNU Emacs and GNU ELPA repositories,

That's also a user option.

Best regards, Michael.

Message #31 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sat, 27 Apr 2019 09:44:24 -0400

tags 35362 fixed
close 35362 
quit

Michael Albinus <michael.albinus <at> gmx.de> writes:

> I believe your patch is almost OK, so let's apply it. But of course, I
> still have some few remarks :-)

Okay, pushed to master.

35f0db153 2019-04-27T09:28:21-04:00 "Automate commit -> debbugs workflow (Bug#35362)"
https://git.savannah.gnu.org/cgit/emacs/elpa.git/commit/?id=35f0db1532612e8b6b4bcc5ecd2f7e479735110a

>>>> Invoke the command @code{debbugs-gnu-pick-commits} and press
>>>
>>> Should we give this command a key binding?
>>
>> Maybe, but then the question is what map to bind it in.  Is it okay to
>> start adding things into vc-git-log-view-mode-map?
>
> Yep. Maybe we bind "c" for it, because other candidates (d/D/p/P) seem
> to be bound already.

Actually I noticed there are some minor modes that exist just to bind
some dwim command in a few different modes, so I added a
debbugs-gnu-pick-vc-log-commit-mode with the "c" binding (otherwise we
would need eval-after-load to change vc-git-log-view-mode-map which
package code isn't supposed to use).

> Furthermore, we might mention debbugs-gnu-pick-commits in the
> ;;; Commentary section of debbugs-gnu.el.
>
>
>> --- a/packages/debbugs/debbugs-gnu.el
>> +++ b/packages/debbugs/debbugs-gnu.el
>
>> +    (add-hook 'message-send-actions
>> +              (lambda () (remhash bugid debbugs-cache-data))
>> +              nil t)))
>
> Do we  need to declare message-send-actions?

No, add-hook operates on a symbol, not a variable.

>> +(defun debbugs-gnus-jump-to-bug (bugid)
>> +  "Display buffer associated with BUGID with `pop-to-buffer'.
>> +Use `gnus-read-ephemeral-emacs-bug-group' instead if there is no such buffer."
>
> Maybe you could add at least a FIXME for the lack of rmail support.

Ok.  Actually I see there is a debbugs-read-emacs-bug-with-rmail
function, so I think it shouldn't be too hard to figure out rmail
support, but I'll do it in a separate patch.

>> +(defun debbugs-gnu--git-insert (&rest args)
>> +  "Insert output of running git with ARGS.
>> +Throws error if git returns non-zero.  Uses `vc-git-program'."
>
> Don't mention vc-git-program.

ok

>> +  (unless (eql 0 (apply #'vc-git--call '(t t) args))
>
> Declare vc-git--call.

right.

>> +(defun debbugs-gnu--git-remote-info ()
>> +  "Return (REMOTE . INFO-ALIST)."
>
> Say, that REMOTE is a string, which must be matched by REMOTE-REGEXP of
> debbugs-gnu-git-remote-info-alist. Refer to the docstring of that user option.

right

>> +           (dir (make-temp-file (format "patches-for-bug%d-" bugnum) t))
>
> I'm curious: Why a trailing "-" in the buffer name?

That was a typo.

>> +           (deldir (lambda ()
>> +                     (delete-directory dir t)
>> +                     (remove-hook 'message-exit-actions deldir t)
>
> Do we  need to declare message-exit-actions?

no (as for add-hook)

>> +    (dolist (patch (directory-files dir t "\\`[^.]"))
>> +      (mml-attach-file patch type "patch" disposition))
>
> Do we  need to declare mml-attach-file?

It seems to already be autoloaded.

>> +`debbugs-gnu-pick-commits' and jump to read bug in prepration for
>
> Typo.

oops.

>> --- a/packages/debbugs/debbugs-ug.texi
>> +++ b/packages/debbugs/debbugs-ug.texi
>
>> +The string inserted to describe an announced commit is controlled by
>> +the variable @code{debbugs-gnu-commit-description-format}, it is a
>
> It is a user option.
>
>> +subitem is appended to the commit description.  By default this
>> +variable is configured for the GNU Emacs and GNU ELPA repositories,
>
> That's also a user option.

Right, I don't use customize much, so the distinction is lost on me.

Message #38 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Sun, 28 Apr 2019 09:53:46 +0200

Noam Postavsky <npostavs <at> gmail.com> writes:

> Okay, pushed to master.

Thanks. I propose we'll wait for some few days, before releasing 0.18.

Best regards, Michael.

Message #41 received at 35362 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35362 <at> debbugs.gnu.org
Subject: Re: bug#35362: 26.2;
 [debbugs.el] Automate commit -> debbugs flow (posting patches,
 closing bugs after pushing)
Date: Mon, 06 May 2019 14:17:42 +0200

Michael Albinus <michael.albinus <at> gmx.de> writes:

>> Okay, pushed to master.
>
> Thanks. I propose we'll wait for some few days, before releasing 0.18.

Done.

Best regards, Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.