[PATCH 0/5] Notmuch Pick (WIP or contrib)

[PATCH 0/5] Notmuch Pick (WIP or contrib)

[Mark Walters]

Hello

Notmuch pick is an emacs view which displays a threaded view of
messages: each message has its own line and the thread structure is
shown with UTF-8 graphics characters (so it looks vaguely similar to
mutt's threaded view): see http://kanelephant.com/screen.png for a screenshot.

Pick was originally written by David Edmondon and posted on irc and I
have been developing it on and off since.  Now that almost all the
backend changes it uses are in I would like to see about getting it
into mainline.

The code is not of the same standard as mainline code: in particular a
lot of the code is written by me and is working but unidiomatic
lisp. It has also not had widespread testing so I would expect it to
have several bugs.

We could try getting it into mainline using the normal review type
approach, but the patch is necessarily large (it implements a new view
similar in size to show or search) with the main pick.el file being
nearly 900 lines.

An alternative approach would be to accept it into contrib and then
reviewers/users could submit fixes for the problems directly.

This patch series implements the latter approach, but I am definitely
happy to try for the former or some other approach.

In its current form the user needs to copy (or link) the
notmuch-pick.el from contrib into the emacs directory and then build
notmuch as usual. There are two very small patches to "mainline" code:
one to compile and load the pick file if present and one small tweak
to notmuch-show.el. Then in contrib/notmuch-pick there are three
files: the notmuch-pick.el file itself, a README describing
documenting install and use, and a TODO which contains the main things
I think need doing (and I will try to update this in light of comments
received).

Any comments on the code or the correct approach, patches etc
gratefully received!

Best wishes

Mark

PS If it is in contrib then I am happy to maintain it (with the aim of
an eventual move to mainline).


Mark Walters (5):
  emacs: compile and load notmuch-pick.el if present.
  emacs: make notmuch-show return its buffer
  contrib: add notmuch-pick.el file itself
  contrib: Added pick README.
  contrib: add pick TODO file

 contrib/notmuch-pick/README          |   36 ++
 contrib/notmuch-pick/TODO            |   25 +
 contrib/notmuch-pick/notmuch-pick.el |  876 ++++++++++++++++++++++++++++++++++
 emacs/Makefile.local                 |    3 +-
 emacs/notmuch-show.el                |    3 +-
 emacs/notmuch.el                     |    5 +
 6 files changed, 946 insertions(+), 2 deletions(-)
 create mode 100644 contrib/notmuch-pick/README
 create mode 100644 contrib/notmuch-pick/TODO
 create mode 100644 contrib/notmuch-pick/notmuch-pick.el

-- 
1.7.9.1

[PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[Mark Walters]

Compile and load notmuch-pick.el if present.

All the actual setup of pick is done in the function notmuch-pick-init
so we call that in the notmuch init function if it is bound. This
function will setup all extra keybinding etc.
---
 emacs/Makefile.local |    3 ++-
 emacs/notmuch.el     |    5 +++++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index fb82247..9f4dba6 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -15,7 +15,8 @@ emacs_sources := \
 	$(dir)/notmuch-crypto.el \
 	$(dir)/notmuch-tag.el \
 	$(dir)/coolj.el \
-	$(dir)/notmuch-print.el
+	$(dir)/notmuch-print.el \
+	$(wildcard $(dir)/notmuch-pick.el)
 
 emacs_images := \
 	$(srcdir)/$(dir)/notmuch-logo.png
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fd1836f..4f3da4f 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -59,6 +59,9 @@
 (require 'notmuch-maildir-fcc)
 (require 'notmuch-message)
 
+;; Load notmuch-pick if available (but do not error if not present).
+(load "notmuch-pick" t)
+
 (defcustom notmuch-search-result-format
   `(("date" . "%12s ")
     ("count" . "%-7s ")
@@ -1088,6 +1091,8 @@ current search results AND that are tagged with the given tag."
 (defun notmuch ()
   "Run notmuch and display saved searches, known tags, etc."
   (interactive)
+  (when (fboundp 'notmuch-pick-init)
+    (notmuch-pick-init))
   (notmuch-hello))
 
 (defun notmuch-interesting-buffer (b)
-- 
1.7.9.1

[PATCH 2/5] emacs: make notmuch-show return its buffer

[Mark Walters]

notmuch-pick uses the returned buffer to try and make sure it does not
close the wrong buffer.
---
 emacs/notmuch-show.el |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6335d45..1cabd17 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1031,7 +1031,8 @@ function is used."
 	  notmuch-show-parent-buffer parent-buffer
 	  notmuch-show-query-context query-context)
     (notmuch-show-build-buffer)
-    (notmuch-show-goto-first-wanted-message)))
+    (notmuch-show-goto-first-wanted-message)
+    (current-buffer)))
 
 (defun notmuch-show-build-buffer ()
   (let ((inhibit-read-only t))
-- 
1.7.9.1

[PATCH 3/5] contrib: add notmuch-pick.el file itself

[Mark Walters]

This adds the main notmuch-pick.el file.
---
 contrib/notmuch-pick/notmuch-pick.el |  876 ++++++++++++++++++++++++++++++++++
 1 files changed, 876 insertions(+), 0 deletions(-)
 create mode 100644 contrib/notmuch-pick/notmuch-pick.el

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
new file mode 100644
index 0000000..ded0c42
--- /dev/null
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -0,0 +1,876 @@
+;; notmuch-pick.el --- displaying notmuch forests.
+;;
+;; Copyright © Carl Worth
+;; Copyright © David Edmondson
+;; Copyright © Mark Walters
+;;
+;; 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>
+;;          Mark Walters <markwalters1009@gmail.com>
+
+(require 'mail-parse)
+
+(require 'notmuch-lib)
+(require 'notmuch-query)
+(require 'notmuch-show)
+(eval-when-compile (require 'cl))
+
+(declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
+(declare-function notmuch-show "notmuch-show" (&rest args))
+(declare-function notmuch-tag "notmuch" (query &rest tags))
+(declare-function notmuch-show-strip-re "notmuch-show" (subject))
+(declare-function notmuch-show-clean-address "notmuch-show" (parsed-address))
+(declare-function notmuch-show-spaces-n "notmuch-show" (n))
+(declare-function notmuch-read-query "notmuch" (prompt))
+(declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
+(declare-function notmuch-update-tags "notmuch" (current-tags tag-changes))
+(declare-function notmuch-hello-trim "notmuch-hello" (search))
+(declare-function notmuch-search-find-thread-id "notmuch" ())
+(declare-function notmuch-search-find-subject "notmuch" ())
+
+;; the following variable is defined in notmuch.el
+(defvar notmuch-search-query-string)
+
+(defvar notmuch-pick-process-state nil
+  "Parsing state of the search process filter.")
+
+(defgroup notmuch-pick nil
+  "Showing message and thread structure."
+  :group 'notmuch)
+
+;; This is ugly. We can't run setup-show-out until it has been defined
+;; which needs the keymap to be defined. So we defer setting up to
+;; notmuch-pick-init.
+(defcustom notmuch-pick-show-out nil
+  "View selected messages in new window rather than split-pane."
+  :type 'boolean
+  :group 'notmuch-pick
+  :set (lambda (symbol value)
+	 (set-default symbol value)
+	 (when (fboundp 'notmuch-pick-setup-show-out)
+	   (notmuch-pick-setup-show-out))))
+
+(defcustom notmuch-pick-result-format
+  `(("date" . "%12s ")
+    ("authors" . "%21s ")
+    ("subject" . "%-54s ")
+    ("tags" . "(%s)"))
+  "Result formatting for Pick. Supported fields are:
+        date, authors, subject, tags
+Note subject includes the tree structure graphics.
+For example:
+        (setq notmuch-pick-result-format \(\(\"authors\" . \"%-40s\"\)
+                                             \(\"subject\" . \"%s\"\)\)\)"
+  :type '(alist :key-type (string) :value-type (string))
+  :group 'notmuch-pick)
+
+(defcustom notmuch-pick-asynchronous-parser t
+  "Use the asynchronous parser."
+  :type 'boolean
+  :group 'notmuch-pick)
+
+;; Faces for messages that match the query.
+(defface notmuch-pick-match-date-face
+  '((t :inherit default))
+  "Face used in pick mode for the date in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-match-author-face
+  '((((class color)
+      (background dark))
+     (:foreground "OliveDrab1"))
+    (((class color)
+      (background light))
+     (:foreground "dark blue"))
+    (t
+     (:bold t)))
+  "Face used in pick mode for the date in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-match-subject-face
+  '((t :inherit default))
+  "Face used in pick mode for the subject in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-match-tag-face
+  '((((class color)
+      (background dark))
+     (:foreground "OliveDrab1"))
+    (((class color)
+      (background light))
+     (:foreground "navy blue" :bold t))
+    (t
+     (:bold t)))
+  "Face used in pick mode for tags in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+;; Faces for messages that do not match the query.
+(defface notmuch-pick-no-match-date-face
+  '((t (:foreground "gray")))
+  "Face used in pick mode for non-matching dates."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-no-match-subject-face
+  '((t (:foreground "gray")))
+  "Face used in pick mode for non-matching subjects."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-no-match-author-face
+  '((t (:foreground "gray")))
+  "Face used in pick mode for the date in messages matching the query."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+(defface notmuch-pick-no-match-tag-face
+  '((t (:foreground "gray")))
+  "Face used in pick mode face for non-matching tags."
+  :group 'notmuch-pick
+  :group 'notmuch-faces)
+
+;; should this be done as a variable?
+(defvar notmuch-pick-previous-subject "")
+(make-variable-buffer-local 'notmuch-pick-previous-subject)
+
+;; The basic query i.e. the key part of the search request.
+(defvar notmuch-pick-basic-query nil)
+(make-variable-buffer-local 'notmuch-pick-basic-query)
+;; The context of the search: i.e., useful but can be dropped.
+(defvar notmuch-pick-query-context nil)
+(make-variable-buffer-local 'notmuch-pick-query-context)
+(defvar notmuch-pick-buffer-name nil)
+(make-variable-buffer-local 'notmuch-pick-buffer-name)
+(defvar notmuch-pick-message-window nil)
+(make-variable-buffer-local 'notmuch-pick-message-window)
+(put 'notmuch-pick-message-window 'permanent-local t)
+(defvar notmuch-pick-message-buffer nil)
+(make-variable-buffer-local 'notmuch-pick-message-buffer-name)
+(put 'notmuch-pick-message-buffer-name 'permanent-local t)
+
+(defvar notmuch-pick-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mouse-1] 'notmuch-pick-show-message)
+    (define-key map "q" 'notmuch-pick-quit)
+    (define-key map "x" 'notmuch-pick-quit)
+    (define-key map "?" 'notmuch-help)
+    (define-key map "a" 'notmuch-pick-archive-message)
+    (define-key map "=" 'notmuch-pick-refresh-view)
+    (define-key map "s" 'notmuch-search)
+    (define-key map "z" 'notmuch-pick)
+    (define-key map "m" 'notmuch-pick-new-mail)
+    (define-key map "f" 'notmuch-pick-forward-message)
+    (define-key map "r" 'notmuch-pick-reply-sender)
+    (define-key map "R" 'notmuch-pick-reply)
+    (define-key map "n" 'notmuch-pick-next-matching-message)
+    (define-key map "p" 'notmuch-pick-prev-matching-message)
+    (define-key map "N" 'notmuch-pick-next-message)
+    (define-key map "P" 'notmuch-pick-prev-message)
+    (define-key map "|" 'notmuch-pick-pipe-message)
+    (define-key map "-" 'notmuch-pick-remove-tag)
+    (define-key map "+" 'notmuch-pick-add-tag)
+    (define-key map " " 'notmuch-pick-scroll-or-next)
+    (define-key map "b" 'notmuch-pick-scroll-message-window-back)
+    map))
+(fset 'notmuch-pick-mode-map notmuch-pick-mode-map)
+
+(defun notmuch-pick-setup-show-out ()
+  (let ((map notmuch-pick-mode-map))
+    (if notmuch-pick-show-out
+	(progn
+	  (define-key map (kbd "M-RET") 'notmuch-pick-show-message)
+	  (define-key map (kbd "RET") 'notmuch-pick-show-message-out))
+      (progn
+	(define-key map (kbd "RET") 'notmuch-pick-show-message)
+	(define-key map (kbd "M-RET") 'notmuch-pick-show-message-out)))))
+
+(defun notmuch-pick-get-message-properties ()
+  "Return the properties of the current message as a plist.
+
+Some useful entries are:
+:headers - Property list containing the headers :Date, :Subject, :From, etc.
+:tags - Tags for this message"
+  (save-excursion
+    (beginning-of-line)
+    (get-text-property (point) :notmuch-message-properties)))
+
+(defun notmuch-pick-set-message-properties (props)
+  (save-excursion
+    (beginning-of-line)
+    (put-text-property (point) (+ (point) 1) :notmuch-message-properties props)))
+
+(defun notmuch-pick-set-prop (prop val &optional props)
+  (let ((inhibit-read-only t)
+	(props (or props
+		   (notmuch-pick-get-message-properties))))
+    (plist-put props prop val)
+    (notmuch-pick-set-message-properties props)))
+
+(defun notmuch-pick-get-prop (prop &optional props)
+  (let ((props (or props
+		   (notmuch-pick-get-message-properties))))
+    (plist-get props prop)))
+
+(defun notmuch-pick-set-tags (tags)
+  "Set the tags of the current message."
+  (notmuch-pick-set-prop :tags tags))
+
+(defun notmuch-pick-get-tags ()
+  "Return the tags of the current message."
+  (notmuch-pick-get-prop :tags))
+
+(defun notmuch-pick-refresh-result ()
+  (let ((init-point (point))
+	(end (line-end-position))
+	(msg (notmuch-pick-get-message-properties))
+	(inhibit-read-only t))
+    (beginning-of-line)
+    (delete-region (point) (1+ (line-end-position)))
+    (notmuch-pick-insert-msg msg)
+    (let ((new-end (line-end-position)))
+      (goto-char (if (= init-point end)
+		     new-end
+		   (min init-point (- new-end 1)))))))
+
+(defun notmuch-pick-tag-message (&rest tag-changes)
+  "Change tags for the current message.
+
+TAG-CHANGES is a list of tag operations for `notmuch-tag'."
+  (let* ((current-tags (notmuch-pick-get-tags))
+	 (new-tags (notmuch-update-tags current-tags tag-changes)))
+    (unless (equal current-tags new-tags)
+      (funcall 'notmuch-tag (notmuch-pick-get-message-id) tag-changes)
+      (notmuch-pick-set-tags new-tags)
+      (notmuch-pick-refresh-result))))
+
+(defun notmuch-pick-tag (&optional initial-input)
+  "Change tags for the current message, read input from the minibuffer."
+  (interactive)
+  (let ((tag-changes (notmuch-read-tag-changes
+		      initial-input (notmuch-pick-get-message-id))))
+    (apply 'notmuch-pick-tag-message tag-changes)))
+
+(defun notmuch-pick-add-tag ()
+  "Same as `notmuch-pick-tag' but sets initial input to '+'."
+  (interactive)
+  (notmuch-pick-tag "+"))
+
+(defun notmuch-pick-remove-tag ()
+  "Same as `notmuch-pick-tag' but sets initial input to '-'."
+  (interactive)
+  (notmuch-pick-tag "-"))
+
+(defun notmuch-pick-get-message-id ()
+  "Return the message id of the current message."
+  (concat "id:\"" (notmuch-pick-get-prop :id) "\""))
+
+(defun notmuch-pick-get-match ()
+  "Return whether the current message is a match."
+  (interactive)
+  (notmuch-pick-get-prop :match))
+
+;; This function should be in notmuch-hello.el but we are trying to
+;; minimise impact on the rest of the codebase.
+(defun notmuch-pick-from-hello (&optional search)
+  "Run a query and display results in experimental notmuch-pick mode"
+  (interactive)
+  (unless (null search)
+    (setq search (notmuch-hello-trim search))
+    (let ((history-delete-duplicates t))
+      (add-to-history 'notmuch-search-history search)))
+  (notmuch-pick search))
+
+;; This function should be in notmuch-show.el but be we trying to
+;; minimise impact on the rest of the codebase.
+(defun notmuch-pick-from-show-current-query ()
+  "Call notmuch pick with the current query"
+  (interactive)
+  (notmuch-pick notmuch-show-thread-id notmuch-show-query-context))
+
+;; This function should be in notmuch.el but be we trying to minimise
+;; impact on the rest of the codebase.
+(defun notmuch-pick-from-search-current-query ()
+  "Call notmuch pick with the current query"
+  (interactive)
+  (notmuch-pick notmuch-search-query-string))
+
+;; This function should be in notmuch.el but be we trying to minimise
+;; impact on the rest of the codebase.
+(defun notmuch-pick-from-search-thread ()
+  "Show the selected thread with notmuch-pick"
+  (interactive)
+  (notmuch-pick (notmuch-search-find-thread-id)
+                notmuch-search-query-string
+                (notmuch-prettify-subject (notmuch-search-find-subject)))
+  (notmuch-pick-show-match-message-with-wait))
+
+(defun notmuch-pick-show-message ()
+  "Show the current message (in split-pane)."
+  (interactive)
+  (let ((id (notmuch-pick-get-message-id))
+	(inhibit-read-only t)
+	buffer)
+    (when id
+      ;; We close and reopen the window to kill off un-needed buffers
+      ;; this might cause flickering but seems ok.
+      (notmuch-pick-close-message-window)
+      (setq notmuch-pick-message-window
+	    (split-window-vertically (/ (window-height) 4)))
+      (with-selected-window notmuch-pick-message-window
+	(setq current-prefix-arg '(4))
+	(setq buffer (notmuch-show id nil nil nil))))
+    (setq notmuch-pick-message-buffer buffer)))
+
+(defun notmuch-pick-show-message-out ()
+  "Show the current message (in whole window)."
+  (interactive)
+  (let ((id (notmuch-pick-get-message-id))
+	(inhibit-read-only t)
+	buffer)
+    (when id
+      ;; We close the window to kill off un-needed buffers.
+      (notmuch-pick-close-message-window)
+      (notmuch-show id nil nil nil))))
+
+(defun notmuch-pick-scroll-message-window ()
+  "Scroll the message window (if it exists)"
+  (interactive)
+  (when (window-live-p notmuch-pick-message-window)
+    (with-selected-window notmuch-pick-message-window
+      (if (pos-visible-in-window-p (point-max))
+	  t
+	(scroll-up)))))
+
+(defun notmuch-pick-scroll-message-window-back ()
+  "Scroll the message window back(if it exists)"
+  (interactive)
+  (when (window-live-p notmuch-pick-message-window)
+    (with-selected-window notmuch-pick-message-window
+      (if (pos-visible-in-window-p (point-min))
+	  t
+	(scroll-down)))))
+
+(defun notmuch-pick-scroll-or-next ()
+  "Scroll the message window. If it at end go to next message."
+  (interactive)
+  (when (notmuch-pick-scroll-message-window)
+    (notmuch-pick-next-matching-message)))
+
+(defun notmuch-pick-quit ()
+  "Close the split view or exit pick."
+  (interactive)
+  (unless (notmuch-pick-close-message-window)
+    (kill-buffer (current-buffer))))
+
+(defun notmuch-pick-close-message-window ()
+  "Close the message-window. Return t if close succeeds."
+  (interactive)
+  (when (window-live-p notmuch-pick-message-window)
+    (delete-window notmuch-pick-message-window)
+    (unless (get-buffer-window-list notmuch-pick-message-buffer)
+      (kill-buffer notmuch-pick-message-buffer))
+    t))
+
+(defun notmuch-pick-archive-message ()
+  "Archive the current message and move to next matching message."
+  (interactive)
+  (notmuch-pick-tag-message "-inbox")
+  (notmuch-pick-next-matching-message))
+
+(defun notmuch-pick-next-message ()
+  "Move to next message."
+  (interactive)
+  (forward-line)
+  (when (window-live-p notmuch-pick-message-window)
+    (notmuch-pick-show-message)))
+
+(defun notmuch-pick-prev-message ()
+  "Move to previous message."
+  (interactive)
+  (forward-line -1)
+  (when (window-live-p notmuch-pick-message-window)
+    (notmuch-pick-show-message)))
+
+(defun notmuch-pick-prev-matching-message ()
+  "Move to previous matching message."
+  (interactive)
+  (forward-line -1)
+  (while (and (not (bobp)) (not (notmuch-pick-get-match)))
+    (forward-line -1))
+  (when (window-live-p notmuch-pick-message-window)
+    (notmuch-pick-show-message)))
+
+(defun notmuch-pick-next-matching-message ()
+  "Move to next matching message."
+  (interactive)
+  (forward-line)
+  (while (and (not (eobp)) (not (notmuch-pick-get-match)))
+    (forward-line))
+  (when (window-live-p notmuch-pick-message-window)
+    (notmuch-pick-show-message)))
+
+(defun notmuch-pick-show-match-message-with-wait ()
+  "Show the first matching message but wait for it to appear or search to finish."
+  (interactive)
+  (unless (notmuch-pick-get-match)
+    (notmuch-pick-next-matching-message))
+  (while (and (not (notmuch-pick-get-match))
+	      (not (eq notmuch-pick-process-state 'end)))
+    (message "waiting for message")
+    (sit-for 0.1)
+    (goto-char (point-min))
+    (unless (notmuch-pick-get-match)
+      (notmuch-pick-next-matching-message)))
+  (message nil)
+  (when (notmuch-pick-get-match)
+    (notmuch-pick-show-message)))
+
+(defun notmuch-pick-refresh-view ()
+  "Refresh view."
+  (interactive)
+  (let ((inhibit-read-only t)
+	(basic-query notmuch-pick-basic-query)
+	(query-context notmuch-pick-query-context)
+	(buffer-name notmuch-pick-buffer-name))
+    (erase-buffer)
+    (notmuch-pick-worker basic-query query-context (get-buffer buffer-name))))
+
+(defun notmuch-pick-string-width (string width &optional right)
+  (let ((s (format (format "%%%s%ds" (if right "" "-") width)
+		   string)))
+    (if (> (length s) width)
+	(substring s 0 width)
+      s)))
+
+(defmacro with-current-notmuch-pick-message (&rest body)
+  "Evaluate body with current buffer set to the text of current message"
+  `(save-excursion
+     (let ((id (notmuch-pick-get-message-id)))
+       (let ((buf (generate-new-buffer (concat "*notmuch-msg-" id "*"))))
+         (with-current-buffer buf
+	    (call-process notmuch-command nil t nil "show" "--format=raw" id)
+           ,@body)
+	 (kill-buffer buf)))))
+
+(defun notmuch-pick-new-mail (&optional prompt-for-sender)
+  "Compose new mail."
+  (interactive "P")
+  (notmuch-pick-close-message-window)
+  (notmuch-mua-new-mail prompt-for-sender ))
+
+(defun notmuch-pick-forward-message (&optional prompt-for-sender)
+  "Forward the current message."
+  (interactive "P")
+  (notmuch-pick-close-message-window)
+  (with-current-notmuch-pick-message
+   (notmuch-mua-new-forward-message prompt-for-sender)))
+
+(defun notmuch-pick-reply (&optional prompt-for-sender)
+  "Reply to the sender and all recipients of the current message."
+  (interactive "P")
+  (notmuch-pick-close-message-window)
+  (notmuch-mua-new-reply (notmuch-pick-get-message-id) prompt-for-sender t))
+
+(defun notmuch-pick-reply-sender (&optional prompt-for-sender)
+  "Reply to the sender of the current message."
+  (interactive "P")
+  (notmuch-pick-close-message-window)
+  (notmuch-mua-new-reply (notmuch-pick-get-message-id) prompt-for-sender nil))
+
+;; Shamelessly stolen from notmuch-show.el: maybe should be unified.
+(defun notmuch-pick-pipe-message (command)
+  "Pipe the contents of the current message to the given command.
+
+The given command will be executed with the raw contents of the
+current email message as stdin. Anything printed by the command
+to stdout or stderr will appear in the *notmuch-pipe* buffer.
+
+When invoked with a prefix argument, the command will receive all
+open messages in the current thread (formatted as an mbox) rather
+than only the current message."
+  (interactive "sPipe message to command: ")
+  (let ((shell-command
+	 (concat notmuch-command " show --format=raw "
+		 (shell-quote-argument (notmuch-pick-get-message-id)) " | " command))
+	 (buf (get-buffer-create (concat "*notmuch-pipe*"))))
+    (with-current-buffer buf
+      (setq buffer-read-only nil)
+      (erase-buffer)
+      (let ((exit-code (call-process-shell-command shell-command nil buf)))
+	(goto-char (point-max))
+	(set-buffer-modified-p nil)
+	(setq buffer-read-only t)
+	(unless (zerop exit-code)
+	  (switch-to-buffer-other-window buf)
+	  (message (format "Command '%s' exited abnormally with code %d"
+			   shell-command exit-code)))))))
+
+;; Shamelessly stolen from notmuch-show.el: should be unified.
+(defun notmuch-pick-clean-address (address)
+  "Try to clean a single email ADDRESS for display.  Return
+unchanged ADDRESS if parsing fails."
+  (condition-case nil
+    (let (p-name p-address)
+      ;; It would be convenient to use `mail-header-parse-address',
+      ;; but that expects un-decoded mailbox parts, whereas our
+      ;; mailbox parts are already decoded (and hence may contain
+      ;; UTF-8). Given that notmuch should handle most of the awkward
+      ;; cases, some simple string deconstruction should be sufficient
+      ;; here.
+      (cond
+       ;; "User <user@dom.ain>" style.
+       ((string-match "\\(.*\\) <\\(.*\\)>" address)
+	(setq p-name (match-string 1 address)
+	      p-address (match-string 2 address)))
+
+       ;; "<user@dom.ain>" style.
+       ((string-match "<\\(.*\\)>" address)
+	(setq p-address (match-string 1 address)))
+
+       ;; Everything else.
+       (t
+	(setq p-address address)))
+
+      (when p-name
+	;; Remove elements of the mailbox part that are not relevant for
+	;; display, even if they are required during transport:
+	;;
+	;; Backslashes.
+	(setq p-name (replace-regexp-in-string "\\\\" "" p-name))
+
+	;; Outer single and double quotes, which might be nested.
+	(loop
+	 with start-of-loop
+	 do (setq start-of-loop p-name)
+
+	 when (string-match "^\"\\(.*\\)\"$" p-name)
+	 do (setq p-name (match-string 1 p-name))
+
+	 when (string-match "^'\\(.*\\)'$" p-name)
+	 do (setq p-name (match-string 1 p-name))
+
+	 until (string= start-of-loop p-name)))
+
+      ;; If the address is 'foo@bar.com <foo@bar.com>' then show just
+      ;; 'foo@bar.com'.
+      (when (string= p-name p-address)
+	(setq p-name nil))
+
+      ;; If we have a name return that otherwise return the address.
+      (if (not p-name)
+	  p-address
+	p-name))
+    (error address)))
+
+(defun notmuch-pick-insert-field (field format-string msg)
+  (let* ((headers (plist-get msg :headers))
+	(match (plist-get msg :match)))
+    (cond
+     ((string-equal field "date")
+      (let ((face (if match
+		      'notmuch-pick-match-date-face
+		    'notmuch-pick-no-match-date-face)))
+	(insert (propertize (format format-string (plist-get msg :date_relative))
+			    'face face))))
+
+     ((string-equal field "subject")
+      (let ((tree-status (plist-get msg :tree-status))
+	    (bare-subject (notmuch-show-strip-re (plist-get headers :Subject)))
+	    (face (if match
+		      'notmuch-pick-match-subject-face
+		    'notmuch-pick-no-match-subject-face)))
+	(insert (propertize (format format-string
+				    (concat
+				     (mapconcat #'identity (reverse tree-status) "")
+				     (if (string= notmuch-pick-previous-subject bare-subject)
+					 " ..."
+				       bare-subject)))
+			    'face face))
+	(setq notmuch-pick-previous-subject bare-subject)))
+
+     ((string-equal field "authors")
+      (let ((author (notmuch-pick-clean-address (plist-get headers :From)))
+	    (len (length (format format-string "")))
+	    (face (if match
+		      'notmuch-pick-match-author-face
+		    'notmuch-pick-no-match-author-face)))
+      (insert (propertize (format format-string
+				  (notmuch-pick-string-width author (- len 2)))
+			  'face face))))
+
+     ((string-equal field "tags")
+      (let ((tags (plist-get msg :tags))
+	    (face (if match
+			  'notmuch-pick-match-tag-face
+			'notmuch-pick-no-match-tag-face)))
+	(when tags
+	  (insert (propertize (format format-string
+				      (mapconcat #'identity tags ", "))
+			      'face face))))))))
+
+
+(defun notmuch-pick-insert-msg (msg)
+  "Insert the message MSG according to notmuch-pick-result-format"
+  (dolist (spec notmuch-pick-result-format)
+    (notmuch-pick-insert-field (car spec) (cdr spec) msg))
+  (notmuch-pick-set-message-properties msg)
+  (insert "\n"))
+
+
+(defun notmuch-pick-insert-tree (tree depth tree-status first last)
+  "Insert the message tree TREE at depth DEPTH in the current thread."
+  (let ((msg (car tree))
+	(replies (cadr tree)))
+
+      (cond
+       ((and (< 0 depth) (not last))
+	(push "├" tree-status))
+       ((and (< 0 depth) last)
+	(push "╰" tree-status))
+       ((and (eq 0 depth) first last)
+;;	  (push "─" tree-status)) choice between this and next line is matter of taste.
+	(push " " tree-status))
+       ((and (eq 0 depth) first (not last))
+	  (push "┬" tree-status))
+       ((and (eq 0 depth) (not first) last)
+	(push "╰" tree-status))
+       ((and (eq 0 depth) (not first) (not last))
+	(push "├" tree-status)))
+
+      (push (concat (if replies "┬" "─") "►") tree-status)
+      (notmuch-pick-insert-msg (plist-put msg :tree-status tree-status))
+      (pop tree-status)
+      (pop tree-status)
+
+      (if last
+	  (push " " tree-status)
+	(push "│" tree-status))
+
+    (notmuch-pick-insert-thread replies (1+ depth) tree-status)))
+
+(defun notmuch-pick-insert-thread (thread depth tree-status)
+  "Insert the thread THREAD at depth DEPTH >= 1 in the current forest."
+  (let ((n (length thread)))
+    (loop for tree in thread
+	  for count from 1 to n
+
+	  do (notmuch-pick-insert-tree tree depth tree-status (eq count 1) (eq count n)))))
+
+(defun notmuch-pick-insert-forest (forest)
+  (mapc '(lambda (thread)
+	   (let (tree-status)
+	     ;; Reset at the start of each main thread.
+	     (setq notmuch-pick-previous-subject nil)
+	     (notmuch-pick-insert-thread thread 0 tree-status)))
+	forest))
+
+(defun notmuch-pick-mode ()
+  "Major mode displaying messages (as opposed to threads) of of a notmuch search.
+
+This buffer contains the results of a \"notmuch pick\" of your
+email archives. Each line in the buffer represents a single
+message giving the relative date, the author, subject, and any
+tags.
+
+Pressing \\[notmuch-pick-show-message] on any line displays that message.
+
+Complete list of currently available key bindings:
+
+\\{notmuch-pick-mode-map}"
+
+  (interactive)
+  (kill-all-local-variables)
+  (use-local-map notmuch-pick-mode-map)
+  (setq major-mode 'notmuch-pick-mode
+	mode-name "notmuch-pick")
+  (hl-line-mode 1)
+  (setq buffer-read-only t
+	truncate-lines t))
+
+(defun notmuch-pick-process-sentinel (proc msg)
+  "Add a message to let user know when \"notmuch pick\" exits"
+  (let ((buffer (process-buffer proc))
+	(status (process-status proc))
+	(exit-status (process-exit-status proc))
+	(never-found-target-thread nil))
+    (when (memq status '(exit signal))
+        (kill-buffer (process-get proc 'parse-buf))
+	(if (buffer-live-p buffer)
+	    (with-current-buffer buffer
+	      (save-excursion
+		(let ((inhibit-read-only t)
+		      (atbob (bobp)))
+		  (goto-char (point-max))
+		  (if (eq status 'signal)
+		      (insert "Incomplete search results (pick process was killed).\n"))
+		  (when (eq status 'exit)
+		    (insert "End of search results.")
+		    (message "async parser finished %s"
+			     (format-time-string "%r"))
+		    (unless (= exit-status 0)
+		      (insert (format " (process returned %d)" exit-status)))
+		    (insert "\n")))))))))
+
+
+(defun notmuch-pick-show-error (string &rest objects)
+  (save-excursion
+    (goto-char (point-max))
+    (insert "Error: Unexpected output from notmuch search:\n")
+    (insert (apply #'format string objects))
+    (insert "\n")))
+
+
+(defvar notmuch-pick-json-parser nil
+  "Incremental JSON parser for the search process filter.")
+
+(defun notmuch-pick-process-filter (proc string)
+  "Process and filter the output of \"notmuch show\" (for pick)"
+  (let ((results-buf (process-buffer proc))
+        (parse-buf (process-get proc 'parse-buf))
+        (inhibit-read-only t)
+        done)
+    (if (not (buffer-live-p results-buf))
+        (delete-process proc)
+      (with-current-buffer parse-buf
+        ;; Insert new data
+        (save-excursion
+          (goto-char (point-max))
+          (insert string)))
+      (with-current-buffer results-buf
+	(save-excursion
+	  (goto-char (point-max))
+	  (while (not done)
+	    (condition-case nil
+		(case notmuch-pick-process-state
+		      ((begin)
+		       ;; Enter the results list
+		       (if (eq (notmuch-json-begin-compound
+				notmuch-pick-json-parser) 'retry)
+			   (setq done t)
+			 (setq notmuch-pick-process-state 'result)))
+		      ((result)
+		       ;; Parse a result
+		       (let ((result (notmuch-json-read notmuch-pick-json-parser)))
+			 (case result
+			       ((retry) (setq done t))
+			       ((end) (setq notmuch-pick-process-state 'end))
+			       (otherwise (notmuch-pick-insert-forest (list result))))))
+		      ((end)
+		       ;; Any trailing data is unexpected
+		       (with-current-buffer parse-buf
+			 (skip-chars-forward " \t\r\n")
+			 (if (eobp)
+			     (setq done t)
+			   (signal 'json-error nil)))))
+	      (json-error
+	       ;; Do our best to resynchronize and ensure forward
+	       ;; progress
+	       (notmuch-pick-show-error
+		"%s"
+		(with-current-buffer parse-buf
+		  (let ((bad (buffer-substring (line-beginning-position)
+					       (line-end-position))))
+		    (forward-line)
+		    bad))))))
+	  ;; Clear out what we've parsed
+	  (with-current-buffer parse-buf
+	    (delete-region (point-min) (point))))))))
+
+(defun notmuch-pick-worker (basic-query &optional query-context buffer)
+  (interactive)
+  (notmuch-pick-mode)
+  (setq notmuch-pick-basic-query basic-query)
+  (setq notmuch-pick-query-context query-context)
+  (setq notmuch-pick-buffer-name (buffer-name buffer))
+
+  (erase-buffer)
+  (goto-char (point-min))
+  (let* ((search-args (concat "\'" basic-query
+		       (if query-context (concat " and (" query-context ")"))
+		       "\'"))
+	 (message-arg "--entire-thread"))
+    (if (equal (car (process-lines notmuch-command "count" search-args)) "0")
+	(setq search-args basic-query))
+    (message "starting parser %s"
+	     (format-time-string "%r"))
+    (if notmuch-pick-asynchronous-parser
+	(let ((proc (start-process
+		     "notmuch-pick" buffer
+		     notmuch-command "show" "--body=false" "--format=json"
+		     message-arg search-args))
+	      ;; Use a scratch buffer to accumulate partial output.
+              ;; This buffer will be killed by the sentinel, which
+              ;; should be called no matter how the process dies.
+              (parse-buf (generate-new-buffer " *notmuch pick parse*")))
+          (set (make-local-variable 'notmuch-pick-process-state) 'begin)
+          (set (make-local-variable 'notmuch-pick-json-parser)
+               (notmuch-json-create-parser parse-buf))
+          (process-put proc 'parse-buf parse-buf)
+	  (set-process-sentinel proc 'notmuch-pick-process-sentinel)
+	  (set-process-filter proc 'notmuch-pick-process-filter)
+	  (set-process-query-on-exit-flag proc nil))
+      (progn
+	(notmuch-pick-insert-forest
+	 (notmuch-query-get-threads
+	  (list "--body=false" message-arg search-args)))
+	(save-excursion
+	  (goto-char (point-max))
+	  (insert "End of search results.\n"))
+	(message "sync parser finished %s"
+		 (format-time-string "%r"))))))
+
+(defvar notmuch-pick-initialized nil)
+
+(defun notmuch-pick-init()
+  (unless notmuch-pick-initialized
+    (define-key 'notmuch-search-mode-map "z" 'notmuch-pick)
+    (define-key 'notmuch-search-mode-map "Z" 'notmuch-pick-from-search-current-query)
+    (define-key 'notmuch-search-mode-map (kbd "M-RET") 'notmuch-pick-from-search-thread)
+    (define-key 'notmuch-hello-mode-map "z" 'notmuch-pick-from-hello)
+    (define-key 'notmuch-show-mode-map "z" 'notmuch-pick)
+    (define-key 'notmuch-show-mode-map "Z" 'notmuch-pick-from-show-current-query)
+    (notmuch-pick-setup-show-out)
+    (setq notmuch-pick-initialized t)
+    (message "Initialised notmuch-pick")))
+
+(defun notmuch-pick (&optional query query-context buffer-name show-first-match)
+  "Run notmuch pick with the given `query' and display the results"
+  (interactive "sNotmuch pick: ")
+  (if (null query)
+      (setq query (notmuch-read-query "Notmuch pick: ")))
+  (let ((buffer (get-buffer-create (generate-new-buffer-name
+				    (or buffer-name
+					(concat "*notmuch-pick-" query "*")))))
+	(inhibit-read-only t))
+
+    (switch-to-buffer buffer)
+    ;; Don't track undo information for this buffer
+    (set 'buffer-undo-list t)
+
+    (notmuch-pick-worker query query-context buffer)
+
+    (setq truncate-lines t)
+    (when show-first-match
+      (notmuch-pick-show-match-message-with-wait))))
+
+;;
+
+(provide 'notmuch-pick)
-- 
1.7.9.1

[PATCH 4/5] contrib: Added pick README.

[Mark Walters]

---
 contrib/notmuch-pick/README |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 contrib/notmuch-pick/README

diff --git a/contrib/notmuch-pick/README b/contrib/notmuch-pick/README
new file mode 100644
index 0000000..1b2399f
--- /dev/null
+++ b/contrib/notmuch-pick/README
@@ -0,0 +1,36 @@
+Notmuch Pick
+
+Notmuch pick is an experimental threaded message view for the emacs
+interface. Each message is one line in the results and the thread
+structure is shown using UFT-8 box drawing characters (similar to
+Mutt's threaded view). It comes between search and show in terms of
+amount of output and can be useful for viewing both single theads and
+multiple threads.
+
+Install
+
+To use notmuch pick copy or link the notmuch-pick.el file into the
+emacs directory of the main source and then compile and install
+notmuch as usual.
+
+Using Pick
+
+The main key entries to notmuch pick are
+
+'z' enter a query to view using notmuch pick (works in hello, search,
+    show and pick itself).
+'Z' view the current query in pick (works from search and show)
+'M-RET' view the selected thread in pick (works in search mode)
+
+Once in pick mode keybinding are mostly inline with the rest of
+notmuch and are all viewable with '?' as usual.
+
+Customising Pick
+
+Pick has several customisation variables. The most significant is the
+first notmuch-pick-show-out which determines the behaviour when
+selecting a message (with RET) in the pick view. By default pick uses
+a split window showing the single message in the bottom pane. However,
+if this option is set then it views the whole thread in the complete
+window jumping to the selected message in the thread. In either case
+M-RET selects the other option.
-- 
1.7.9.1

[PATCH 5/5] contrib: add pick TODO file

[Mark Walters]

---
 contrib/notmuch-pick/TODO |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)
 create mode 100644 contrib/notmuch-pick/TODO

diff --git a/contrib/notmuch-pick/TODO b/contrib/notmuch-pick/TODO
new file mode 100644
index 0000000..2d0796e
--- /dev/null
+++ b/contrib/notmuch-pick/TODO
@@ -0,0 +1,25 @@
+TODO for notmuch-pick
+
+(These are the things I can think of: to be added to as problems get
+reported or found!)
+
+Things that need fixing before acceptance to mainline
+
+- Review lisp to make idiomatic.
+- Unify functions with search or show where appropriate.
+- Work out a fall-back if the font does not contain box graphic characters.
+- Add extra functionality?
+
+- Decide whether the asynchronous parser is satisfactory.
+- Remove debugging/timing information.
+
+- Add tests (I have some but I am not sure how to add them if pick is
+  in contrib).
+
+Bugs:
+
+- The display flickers while pick is running. I have no idea why.
+
+Other todo items
+
+?
\ No newline at end of file
-- 
1.7.9.1

Re: [PATCH 0/5] Notmuch Pick (WIP or contrib)

[Aneesh Kumar K.V]

Mark Walters <markwalters1009@gmail.com> writes:

> Hello
>
> Notmuch pick is an emacs view which displays a threaded view of
> messages: each message has its own line and the thread structure is
> shown with UTF-8 graphics characters (so it looks vaguely similar to
> mutt's threaded view): see http://kanelephant.com/screen.png for a screenshot.
>
> Pick was originally written by David Edmondon and posted on irc and I
> have been developing it on and off since.  Now that almost all the
> backend changes it uses are in I would like to see about getting it
> into mainline.
>
> The code is not of the same standard as mainline code: in particular a
> lot of the code is written by me and is working but unidiomatic
> lisp. It has also not had widespread testing so I would expect it to
> have several bugs.
>
> We could try getting it into mainline using the normal review type
> approach, but the patch is necessarily large (it implements a new view
> similar in size to show or search) with the main pick.el file being
> nearly 900 lines.
>
> An alternative approach would be to accept it into contrib and then
> reviewers/users could submit fixes for the problems directly.
>
> This patch series implements the latter approach, but I am definitely
> happy to try for the former or some other approach.
>


I have been using this series for a long time and have not found any
issues. How about adding pick as a part of proper emacs file and have a
config value that disables pick by default. ? So only when
notmuch-enable-pick is set we will load notmuch-pick.el ?

> In its current form the user needs to copy (or link) the
> notmuch-pick.el from contrib into the emacs directory and then build
> notmuch as usual. 

With that users won't require to do the above.

>There are two very small patches to "mainline" code:
> one to compile and load the pick file if present and one small tweak
> to notmuch-show.el. Then in contrib/notmuch-pick there are three
> files: the notmuch-pick.el file itself, a README describing
> documenting install and use, and a TODO which contains the main things
> I think need doing (and I will try to update this in light of comments
> received).
>



-aneesh

Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[Tomi Ollila]

On Wed, Jul 25 2012, Mark Walters <markwalters1009@gmail.com> wrote:

> Compile and load notmuch-pick.el if present.
>
> All the actual setup of pick is done in the function notmuch-pick-init
> so we call that in the notmuch init function if it is bound. This
> function will setup all extra keybinding etc.

Great stuff! See a few thoughts below...

> ---
>  emacs/Makefile.local |    3 ++-
>  emacs/notmuch.el     |    5 +++++
>  2 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> index fb82247..9f4dba6 100644
> --- a/emacs/Makefile.local
> +++ b/emacs/Makefile.local
> @@ -15,7 +15,8 @@ emacs_sources := \
>  	$(dir)/notmuch-crypto.el \
>  	$(dir)/notmuch-tag.el \
>  	$(dir)/coolj.el \
> -	$(dir)/notmuch-print.el
> +	$(dir)/notmuch-print.el \
> +	$(wildcard $(dir)/notmuch-pick.el)

I wonder whether having this conditional is good idea. What if
someone copies (or (sym)links) notmuch-pick.el there and then
compiles and takes to use. Next time he takes clean tree and
forgets to do this copying and installs to the same destination.
Now there is old notmuch-pick.elc which might be out of sync.

I think it would be better to provide a shell script in 
notmuch-pick directory which byte-compiles and installs notmuch-pick
in case user wants to install/update notmuch-pick. Whenever notmuch-pick
is good enough to be shipped inside $(dir) above then the aboce conditional
is not needed (at all).


>  emacs_images := \
>  	$(srcdir)/$(dir)/notmuch-logo.png
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index fd1836f..4f3da4f 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -59,6 +59,9 @@
>  (require 'notmuch-maildir-fcc)
>  (require 'notmuch-message)
>  
> +;; Load notmuch-pick if available (but do not error if not present).
> +(load "notmuch-pick" t)
> +
>  (defcustom notmuch-search-result-format
>    `(("date" . "%12s ")
>      ("count" . "%-7s ")
> @@ -1088,6 +1091,8 @@ current search results AND that are tagged with the given tag."
>  (defun notmuch ()
>    "Run notmuch and display saved searches, known tags, etc."
>    (interactive)
> +  (when (fboundp 'notmuch-pick-init)
> +    (notmuch-pick-init))
>    (notmuch-hello))

Instead of this could notmuch-pick.el contain:

In the beginning:

(require 'notmuch-hello)
(require 'notmuch-show)
(require 'notmuch) ;; XXX ATM, as notmuch-search-mode-map is defined here

And, at the end, before (provide 'notmuch-pick), execute the lines
what currenty are contained in (notmuch-pick-init).

Then, those who want to start using notmuch-pick at this time
can (just) write the following in their .emacs:

(require 'notmuch)
(require 'notmuch-pick)

>  
>  (defun notmuch-interesting-buffer (b)
> -- 
> 1.7.9.1

Tomi

Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[Mark Walters]

On Wed, 25 Jul 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, Jul 25 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> Compile and load notmuch-pick.el if present.
>>
>> All the actual setup of pick is done in the function notmuch-pick-init
>> so we call that in the notmuch init function if it is bound. This
>> function will setup all extra keybinding etc.
>
> Great stuff! See a few thoughts below...

Thanks for these: I think they are all clear improvements.

>
>> ---
>>  emacs/Makefile.local |    3 ++-
>>  emacs/notmuch.el     |    5 +++++
>>  2 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>> index fb82247..9f4dba6 100644
>> --- a/emacs/Makefile.local
>> +++ b/emacs/Makefile.local
>> @@ -15,7 +15,8 @@ emacs_sources := \
>>  	$(dir)/notmuch-crypto.el \
>>  	$(dir)/notmuch-tag.el \
>>  	$(dir)/coolj.el \
>> -	$(dir)/notmuch-print.el
>> +	$(dir)/notmuch-print.el \
>> +	$(wildcard $(dir)/notmuch-pick.el)
>
> I wonder whether having this conditional is good idea. What if
> someone copies (or (sym)links) notmuch-pick.el there and then
> compiles and takes to use. Next time he takes clean tree and
> forgets to do this copying and installs to the same destination.
> Now there is old notmuch-pick.elc which might be out of sync.
>
> I think it would be better to provide a shell script in 
> notmuch-pick directory which byte-compiles and installs notmuch-pick
> in case user wants to install/update notmuch-pick. Whenever notmuch-pick
> is good enough to be shipped inside $(dir) above then the aboce conditional
> is not needed (at all).
>

I have now done this: I actually use a Makefile so that I can pick up
the config from notmuch: in particular the install directory
for the notmuch-pick.elc file. Does that seem reasonable?


>>  emacs_images := \
>>  	$(srcdir)/$(dir)/notmuch-logo.png
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index fd1836f..4f3da4f 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -59,6 +59,9 @@
>>  (require 'notmuch-maildir-fcc)
>>  (require 'notmuch-message)
>>  
>> +;; Load notmuch-pick if available (but do not error if not present).
>> +(load "notmuch-pick" t)
>> +
>>  (defcustom notmuch-search-result-format
>>    `(("date" . "%12s ")
>>      ("count" . "%-7s ")
>> @@ -1088,6 +1091,8 @@ current search results AND that are tagged with the given tag."
>>  (defun notmuch ()
>>    "Run notmuch and display saved searches, known tags, etc."
>>    (interactive)
>> +  (when (fboundp 'notmuch-pick-init)
>> +    (notmuch-pick-init))
>>    (notmuch-hello))
>
> Instead of this could notmuch-pick.el contain:
>
> In the beginning:
>
> (require 'notmuch-hello)
> (require 'notmuch-show)
> (require 'notmuch) ;; XXX ATM, as notmuch-search-mode-map is defined here
>
> And, at the end, before (provide 'notmuch-pick), execute the lines
> what currenty are contained in (notmuch-pick-init).
>
> Then, those who want to start using notmuch-pick at this time
> can (just) write the following in their .emacs:
>
> (require 'notmuch)
> (require 'notmuch-pick)

This works very nicely. And if we want a transitional period the
notmuch-pick could be in (dir) but not loaded automatically.

Many thanks for the excellent suggestions.

Mark

Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[Tomi Ollila]

On Thu, Jul 26 2012, Mark Walters <markwalters1009@gmail.com> wrote:

> On Wed, 25 Jul 2012, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Wed, Jul 25 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>>
>>> Compile and load notmuch-pick.el if present.
>>>
>>> All the actual setup of pick is done in the function notmuch-pick-init
>>> so we call that in the notmuch init function if it is bound. This
>>> function will setup all extra keybinding etc.
>>
>> Great stuff! See a few thoughts below...
>
> Thanks for these: I think they are all clear improvements.
>
>>
>>> ---
>>>  emacs/Makefile.local |    3 ++-
>>>  emacs/notmuch.el     |    5 +++++
>>>  2 files changed, 7 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>>> index fb82247..9f4dba6 100644
>>> --- a/emacs/Makefile.local
>>> +++ b/emacs/Makefile.local
>>> @@ -15,7 +15,8 @@ emacs_sources := \
>>>  	$(dir)/notmuch-crypto.el \
>>>  	$(dir)/notmuch-tag.el \
>>>  	$(dir)/coolj.el \
>>> -	$(dir)/notmuch-print.el
>>> +	$(dir)/notmuch-print.el \
>>> +	$(wildcard $(dir)/notmuch-pick.el)
>>
>> I wonder whether having this conditional is good idea. What if
>> someone copies (or (sym)links) notmuch-pick.el there and then
>> compiles and takes to use. Next time he takes clean tree and
>> forgets to do this copying and installs to the same destination.
>> Now there is old notmuch-pick.elc which might be out of sync.
>>
>> I think it would be better to provide a shell script in 
>> notmuch-pick directory which byte-compiles and installs notmuch-pick
>> in case user wants to install/update notmuch-pick. Whenever notmuch-pick
>> is good enough to be shipped inside $(dir) above then the aboce conditional
>> is not needed (at all).
>>
>
> I have now done this: I actually use a Makefile so that I can pick up
> the config from notmuch: in particular the install directory
> for the notmuch-pick.elc file. Does that seem reasonable?

I believe so.. I can say for sure when I see the code -- but anything
that makes the brave user's live in the bleeding edge easier is good.

>
>>>  emacs_images := \
>>>  	$(srcdir)/$(dir)/notmuch-logo.png
>>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>>> index fd1836f..4f3da4f 100644
>>> --- a/emacs/notmuch.el
>>> +++ b/emacs/notmuch.el
>>> @@ -59,6 +59,9 @@
>>>  (require 'notmuch-maildir-fcc)
>>>  (require 'notmuch-message)
>>>  
>>> +;; Load notmuch-pick if available (but do not error if not present).
>>> +(load "notmuch-pick" t)
>>> +
>>>  (defcustom notmuch-search-result-format
>>>    `(("date" . "%12s ")
>>>      ("count" . "%-7s ")
>>> @@ -1088,6 +1091,8 @@ current search results AND that are tagged with the given tag."
>>>  (defun notmuch ()
>>>    "Run notmuch and display saved searches, known tags, etc."
>>>    (interactive)
>>> +  (when (fboundp 'notmuch-pick-init)
>>> +    (notmuch-pick-init))
>>>    (notmuch-hello))
>>
>> Instead of this could notmuch-pick.el contain:
>>
>> In the beginning:
>>
>> (require 'notmuch-hello)
>> (require 'notmuch-show)
>> (require 'notmuch) ;; XXX ATM, as notmuch-search-mode-map is defined here
>>
>> And, at the end, before (provide 'notmuch-pick), execute the lines
>> what currenty are contained in (notmuch-pick-init).
>>
>> Then, those who want to start using notmuch-pick at this time
>> can (just) write the following in their .emacs:
>>
>> (require 'notmuch)
>> (require 'notmuch-pick)
>
> This works very nicely. And if we want a transitional period the
> notmuch-pick could be in (dir) but not loaded automatically.

Yes.

>
> Many thanks for the excellent suggestions.

Np. Thank you.

> Mark

Tomi

Re: [PATCH 2/5] emacs: make notmuch-show return its buffer

[Mark Walters]

Hi

I have a version 2 of the rest of this series. However, it obviously
conflicts with the two patches I submitted today splitting out the
async-json parser and the clean-author function. Hence I will see how
they are received before submitting version 2. 

I will mark Patches 3-5 obsolete (and have already marked 1 obsolete as
Tomi had a much better way of doing it). However Patch 2/5 will be needed in
any case so review/approval for this one-line patch would be great!

Many thanks

Mark


On Tue, 24 Jul 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> notmuch-pick uses the returned buffer to try and make sure it does not
> close the wrong buffer.
> ---
>  emacs/notmuch-show.el |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 6335d45..1cabd17 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1031,7 +1031,8 @@ function is used."
>  	  notmuch-show-parent-buffer parent-buffer
>  	  notmuch-show-query-context query-context)
>      (notmuch-show-build-buffer)
> -    (notmuch-show-goto-first-wanted-message)))
> +    (notmuch-show-goto-first-wanted-message)
> +    (current-buffer)))
>  
>  (defun notmuch-show-build-buffer ()
>    (let ((inhibit-read-only t))
> -- 
> 1.7.9.1

Re: [PATCH 2/5] emacs: make notmuch-show return its buffer

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> notmuch-pick uses the returned buffer to try and make sure it does not
> close the wrong buffer.

Pushed this one patch,

d

Re: [PATCH 1/5] emacs: compile and load notmuch-pick.el if present.

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:
> I think it would be better to provide a shell script in 
> notmuch-pick directory which byte-compiles and installs notmuch-pick
> in case user wants to install/update notmuch-pick. Whenever notmuch-pick
> is good enough to be shipped inside $(dir) above then the aboce conditional
> is not needed (at all).

I just tried notmuch-pick for the first time, and at least on this
relatively beefy machine, it runs fine uncompiled. 

I'm not sure if it is worth complicating the mainline install. Which
means I guess I am agreeing with Tomi.  It could also be in independant
Makefile rather than a script.

I don't think it's crucial to install notmuch-pick into the main lisp
context of (Debian) packaging I don't think that will work very well.
The way org-mode works is that it ships a seperate contrib lisp
directory; on Debian this is in /usr/share/org-mode/contrib. Then people
who want to use a contrib piece of lisp would just add the following to
their .emacs:

(add-to-list 'load-path "/usr/share/notmuch/contrib/emacs-lisp")
(require 'notmuch-pick)

at least given the changes that Tomi proposes below.
The path is obviously just an example.

> (require 'notmuch-hello)
> (require 'notmuch-show)
> (require 'notmuch) ;; XXX ATM, as notmuch-search-mode-map is defined here
>
> And, at the end, before (provide 'notmuch-pick), execute the lines
> what currenty are contained in (notmuch-pick-init).

Yes, this sounds about right to me. require should also take care of the
"only run this once" part.

FWIW, I'm currently running pick from the source tree as:

(add-to-list 'load-path
	     (expand-file-name "~/blah"))
(when (locate-library "notmuch-pick")
  (eval-after-load 'notmuch
    '(progn 
       (require 'notmuch-pick)
       (notmuch-pick-init))))

it's a bit overengineered, but this way it doesn't crap out on machines
where notmuch-pick is not installed.

Re: [PATCH 0/5] Notmuch Pick (WIP or contrib)

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

>
> The code is not of the same standard as mainline code: in particular a
> lot of the code is written by me and is working but unidiomatic
> lisp. It has also not had widespread testing so I would expect it to
> have several bugs.

I have gotten the following traceback when running in emacs 24.2.1 if
the pick buffer is the sole window.  It might be related somehow to
running emacsclient.

Debugger entered--Lisp error: (error "Attempt to delete minibuffer or sole ordinary window")
  signal(error ("Attempt to delete minibuffer or sole ordinary window"))
  error("Attempt to delete minibuffer or sole ordinary window")
  byte-code(" ")
  delete-window(#<window 327 on *notmuch-pick-thread:00000000000270ce*<2>>)
  notmuch-pick-close-message-window()
  notmuch-pick-quit()
  call-interactively(notmuch-pick-quit nil nil)

Re: [PATCH 0/5] Notmuch Pick (WIP or contrib)

[Mark Walters]

Hi

>> The code is not of the same standard as mainline code: in particular a
>> lot of the code is written by me and is working but unidiomatic
>> lisp. It has also not had widespread testing so I would expect it to
>> have several bugs.
>
> I have gotten the following traceback when running in emacs 24.2.1 if
> the pick buffer is the sole window.  It might be related somehow to
> running emacsclient.
>
> Debugger entered--Lisp error: (error "Attempt to delete minibuffer or sole ordinary window")
>   signal(error ("Attempt to delete minibuffer or sole ordinary window"))
>   error("Attempt to delete minibuffer or sole ordinary window")
>   byte-code(" ")
>   delete-window(#<window 327 on *notmuch-pick-thread:00000000000270ce*<2>>)
>   notmuch-pick-close-message-window()
>   notmuch-pick-quit()
>   call-interactively(notmuch-pick-quit nil nil)

Thanks for this (and sorry to be slow responding): I can reproduce this
error without emacsclient. I have a "fix" which I think is ok but I
want to try a bit more testing before posting anything.

Thanks

Mark