From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: notmuch@notmuchmail.org
Subject: [PATCH 1/4] emacs: unify search mechanisms
Date: Sun, 25 Dec 2011 08:14:52 +0400
Message-Id: <1324786495-14221-1-git-send-email-dmitry.kurochkin@gmail.com>
Precedence: list

Before the change, there were two ways to do search in Emacs UI:
search widget in notmuch-hello buffer and `notmuch-search'
function bound to "s".  Internally, notmuch-hello search widget
uses `notmuch-search' function.  But it uses widget field input
instead of minibuffer.  Such duplication is a major issue for
notmuch-hello usability: search interface is inconsistent and
lacks features that are available in minibuffer (e.g. history and
auto completion).  Some of these features may be relatively easy
to implement for notmuch-hello search, others would be much more
tricky.  So to avoid duplication, make UI more consistent, bring
notmuch-hello search to feature parity with the minibuffer
search, the patch replaces notmuch-hello search widget and with a
button that works the same way as "s" key binding.
---
 emacs/notmuch-hello.el |   84 +++++++++++++++---------------------------------
 emacs/notmuch-lib.el   |    9 +++++
 emacs/notmuch.el       |   20 ++++++-----
 3 files changed, 46 insertions(+), 67 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 333d4c1..eb08a09 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -29,11 +29,8 @@
 (declare-function notmuch-search "notmuch" (query &optional oldest-first target-thread target-line continuation))
 (declare-function notmuch-poll "notmuch" ())
 
-(defvar notmuch-hello-search-bar-marker nil
-  "The position of the search bar within the notmuch-hello buffer.")
-
-(defcustom notmuch-recent-searches-max 10
-  "The number of recent searches to store and display."
+(defcustom notmuch-hello-recent-searches-max 10
+  "The number of recent searches to display."
   :type 'integer
   :group 'notmuch)
 
@@ -154,16 +151,6 @@ International Bureau of Weights and Measures."
 (defvar notmuch-hello-url "http://notmuchmail.org"
   "The `notmuch' web site.")
 
-(defvar notmuch-hello-recent-searches nil)
-
-(defun notmuch-hello-remember-search (search)
-  (setq notmuch-hello-recent-searches
-	(delete search notmuch-hello-recent-searches))
-  (push search notmuch-hello-recent-searches)
-  (if (> (length notmuch-hello-recent-searches)
-	 notmuch-recent-searches-max)
-      (setq notmuch-hello-recent-searches (butlast notmuch-hello-recent-searches))))
-
 (defun notmuch-hello-nice-number (n)
   (let (result)
     (while (> n 0)
@@ -176,16 +163,10 @@ International Bureau of Weights and Measures."
 	      (format "%s%03d" notmuch-hello-thousands-separator elem))
 	     (cdr result)))))
 
-(defun notmuch-hello-trim (search)
-  "Trim whitespace."
-  (if (string-match "^[[:space:]]*\\(.*[^[:space:]]\\)[[:space:]]*$" search)
-      (match-string 1 search)
-    search))
-
-(defun notmuch-hello-search (search)
-  (let ((search (notmuch-hello-trim search)))
-    (notmuch-hello-remember-search search)
-    (notmuch-search search notmuch-search-oldest-first nil nil #'notmuch-hello-search-continuation)))
+(defun notmuch-hello-search (&optional search)
+  (interactive)
+  (notmuch-search search notmuch-search-oldest-first nil nil
+		  #'notmuch-hello-search-continuation))
 
 (defun notmuch-hello-add-saved-search (widget)
   (interactive)
@@ -319,11 +300,6 @@ should be. Returns a cons cell `(tags-per-line width)'."
 	(widget-insert "\n"))
     found-target-pos))
 
-(defun notmuch-hello-goto-search ()
-  "Put point inside the `search' widget."
-  (interactive)
-  (goto-char notmuch-hello-search-bar-marker))
-
 (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
 
 (defun notmuch-hello-search-continuation()
@@ -353,7 +329,7 @@ should be. Returns a cons cell `(tags-per-line width)'."
     (define-key map "G" 'notmuch-hello-poll-and-update)
     (define-key map (kbd "<C-tab>") 'widget-backward)
     (define-key map "m" 'notmuch-mua-new-mail)
-    (define-key map "s" 'notmuch-hello-goto-search)
+    (define-key map "s" 'notmuch-hello-search)
     map)
   "Keymap for \"notmuch hello\" buffers.")
 (fset 'notmuch-hello-mode-map notmuch-hello-mode-map)
@@ -466,7 +442,8 @@ Complete list of currently available key bindings:
       (widget-insert " messages.\n"))
 
     (let ((found-target-pos nil)
-	  (final-target-pos nil))
+	  (final-target-pos nil)
+	  (default-pos))
       (let* ((saved-alist
 	      ;; Filter out empty saved searches if required.
 	      (if notmuch-show-empty-saved-searches
@@ -497,33 +474,26 @@ Complete list of currently available key bindings:
 		(setq final-target-pos found-target-pos))
 	    (indent-rigidly start (point) notmuch-hello-indent)))
 
-	(widget-insert "\nSearch: ")
-	(setq notmuch-hello-search-bar-marker (point-marker))
-	(widget-create 'editable-field
-		       ;; Leave some space at the start and end of the
-		       ;; search boxes.
-		       :size (max 8 (- (window-width) notmuch-hello-indent
-				       (length "Search: ")))
-		       :action (lambda (widget &rest ignore)
-				 (notmuch-hello-search (widget-value widget))))
-	;; add an invisible space to make `widget-end-of-line' ignore
-	;; trailine spaces in the search widget field
-	(widget-insert " ")
-	(put-text-property (1- (point)) (point) 'invisible t)
+	(widget-insert "\n")
+	(setq default-pos (point-marker))
+	(widget-create 'push-button
+		       :notify (lambda (&rest ignore)
+				 (notmuch-hello-search))
+		       " Search! ")
 	(widget-insert "\n")
 
-	(when notmuch-hello-recent-searches
+	(when notmuch-search-history
 	  (widget-insert "\nRecent searches: ")
 	  (widget-create 'push-button
 			 :notify (lambda (&rest ignore)
-				   (setq notmuch-hello-recent-searches nil)
+				   (setq notmuch-search-history nil)
 				   (notmuch-hello-update))
 			 "clear")
 	  (widget-insert "\n\n")
-	  (let ((start (point))
-		(nth 0))
-	    (mapc (lambda (search)
-		    (let ((widget-symbol (intern (format "notmuch-hello-search-%d" nth))))
+	  (let ((start (point)))
+	    (loop for i from 1 to notmuch-hello-recent-searches-max
+		  for search in notmuch-search-history do
+		    (let ((widget-symbol (intern (format "notmuch-hello-search-%d" i))))
 		      (set widget-symbol
 			   (widget-create 'editable-field
 					  ;; Don't let the search boxes be
@@ -550,9 +520,7 @@ Complete list of currently available key bindings:
 					       (notmuch-hello-add-saved-search widget))
 				     :notmuch-saved-search-widget widget-symbol
 				     "save"))
-		    (widget-insert "\n")
-		    (setq nth (1+ nth)))
-		  notmuch-hello-recent-searches)
+		    (widget-insert "\n"))
 	    (indent-rigidly start (point) notmuch-hello-indent)))
 
 	(when alltags-alist
@@ -580,14 +548,14 @@ Complete list of currently available key bindings:
 
       (let ((start (point)))
 	(widget-insert "\n\n")
-	(widget-insert "Type a search query and hit RET to view matching threads.\n")
-	(when notmuch-hello-recent-searches
+	(widget-insert "Use the `search' button or hit `s' to enter search query.\n")
+	(when notmuch-search-history
 	  (widget-insert "Hit RET to re-submit a previous search. Edit it first if you like.\n")
 	  (widget-insert "Save recent searches with the `save' button.\n"))
 	(when notmuch-saved-searches
 	  (widget-insert "Edit saved searches with the `edit' button.\n"))
 	(widget-insert "Hit RET or click on a saved search or tag name to view matching threads.\n")
-	(widget-insert "`=' refreshes this screen. `s' jumps to the search box. `q' to quit.\n")
+	(widget-insert "`=' refreshes this screen. `s' to search messages. `q' to quit.\n")
 	(let ((fill-column (- (window-width) notmuch-hello-indent)))
 	  (center-region start (point))))
 
@@ -599,7 +567,7 @@ Complete list of currently available key bindings:
 	  (widget-forward 1)))
 
       (unless (widget-at)
-	(notmuch-hello-goto-search))))
+	(goto-char default-pos))))
 
   (run-hooks 'notmuch-hello-refresh-hook))
 
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 0f856bf..a2cb6f2 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -35,6 +35,9 @@
 
 ;;
 
+(defvar notmuch-search-history nil
+  "Variable to store minibuffer history for notmuch searches.")
+
 (defcustom notmuch-saved-searches nil
   "A list of saved searches to display."
   :type '(alist :key-type string :value-type string)
@@ -114,6 +117,12 @@ the user hasn't set this variable with the old or new value."
       (setq list (cdr list)))
     (nreverse out)))
 
+(defun notmuch-trim (search)
+  "Trim whitespaces."
+  (if (string-match "^[[:space:]]*\\(.*[^[:space:]]\\)[[:space:]]*$" search)
+      (match-string 1 search)
+    search))
+
 ; This lets us avoid compiling these replacement functions when emacs
 ; is sufficiently new enough to supply them alone. We do the macro
 ; treatment rather than just wrapping our defun calls in a when form
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fde2377..938a149 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -72,9 +72,6 @@ For example:
   :type '(alist :key-type (string) :value-type (string))
   :group 'notmuch)
 
-(defvar notmuch-query-history nil
-  "Variable to store minibuffer history for notmuch queries")
-
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
   (let ((tag-list
 	 (with-output-to-string
@@ -902,21 +899,26 @@ PROMPT is the string to prompt with."
 	       (t (list string)))))))
       ;; this was simpler than convincing completing-read to accept spaces:
       (define-key keymap (kbd "<tab>") 'minibuffer-complete)
-      (read-from-minibuffer prompt nil keymap nil
-			    'notmuch-query-history nil nil))))
+      (let ((history-delete-duplicates t))
+	(read-from-minibuffer prompt nil keymap nil
+			      'notmuch-search-history nil nil)))))
 
 ;;;###autoload
-(defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
-  "Run \"notmuch search\" with the given query string and display results.
+(defun notmuch-search (&optional query oldest-first target-thread target-line continuation)
+  "Run \"notmuch search\" with the given `query' and display results.
 
-The optional parameters are used as follows:
+If `query' is nil, it is read interactively from the minibuffer.
+Other optional parameters are used as follows:
 
   oldest-first: A Boolean controlling the sort order of returned threads
   target-thread: A thread ID (with the thread: prefix) that will be made
                  current if it appears in the search results.
   target-line: The line number to move to if the target thread does not
                appear in the search results."
-  (interactive (list (notmuch-read-query "Notmuch search: ")))
+  (interactive)
+  (if (null query)
+      (setq query (notmuch-read-query "Notmuch search: ")))
+  (setq query (notmuch-trim query))
   (let ((buffer (get-buffer-create (notmuch-search-buffer-title query))))
     (switch-to-buffer buffer)
     (notmuch-search-mode)
-- 
1.7.7.3