[PATCH 0/2] contrib: pick: remove sync parser

[PATCH 0/2] contrib: pick: remove sync parser

[Mark Walters]

Pick has two possible parsing methods: a synchronous one (like show)
and an async one (like search). The async parser has been the default
since at least the time pick first entered contrib. I don't believe
there have been any reported problems with it during this time so we
might as well remove the sync parser and the associated defcustom. We
also remove the sync tests.

Best wishes

Mark




Mark Walters (2):
  contrib: pick: remove sync parser
  contrib: pick: remove sync tests

 contrib/notmuch-pick/notmuch-pick.el      |   36 +++++-----------
 contrib/notmuch-pick/run-tests.sh         |    2 +-
 contrib/notmuch-pick/test/emacs-pick-sync |   64 -----------------------------
 3 files changed, 12 insertions(+), 90 deletions(-)
 delete mode 100755 contrib/notmuch-pick/test/emacs-pick-sync

-- 
1.7.9.1

[PATCH 1/2] contrib: pick: remove sync parser

[Mark Walters]

Previously pick had the option of using an async parser like search or
a sync parser like show. The async parser has always been the default
and it seems fine so we can remove the sync one and the corresponding
defcustom.
---
 contrib/notmuch-pick/notmuch-pick.el |   36 ++++++++++-----------------------
 1 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 37dc161..8c499b0 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -77,11 +77,6 @@
   :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))
@@ -817,26 +812,17 @@ Complete list of currently available key bindings:
 	 (message-arg "--entire-thread"))
     (if (equal (car (process-lines notmuch-command "count" search-args)) "0")
 	(setq search-args basic-query))
-    (if notmuch-pick-asynchronous-parser
-	(let ((proc (notmuch-start-notmuch
-		     "notmuch-pick" buffer #'notmuch-pick-process-sentinel
-		     "show" "--body=false" "--format=sexp"
-		     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*")))
-          (process-put proc 'parse-buf parse-buf)
-	  (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"))))))
-
+    (let ((proc (notmuch-start-notmuch
+		 "notmuch-pick" buffer #'notmuch-pick-process-sentinel
+		 "show" "--body=false" "--format=sexp"
+		 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*")))
+      (process-put proc 'parse-buf parse-buf)
+      (set-process-filter proc 'notmuch-pick-process-filter)
+      (set-process-query-on-exit-flag proc nil))))
 
 (defun notmuch-pick (&optional query query-context target buffer-name open-target)
   "Run notmuch pick with the given `query' and display the results.
-- 
1.7.9.1

[PATCH 2/2] contrib: pick: remove sync tests

[Mark Walters]

Since the previous commit removed the sync parser we may as well
remove its tests too.
---
 contrib/notmuch-pick/run-tests.sh         |    2 +-
 contrib/notmuch-pick/test/emacs-pick-sync |   64 -----------------------------
 2 files changed, 1 insertions(+), 65 deletions(-)
 delete mode 100755 contrib/notmuch-pick/test/emacs-pick-sync

diff --git a/contrib/notmuch-pick/run-tests.sh b/contrib/notmuch-pick/run-tests.sh
index 7ddc9cc..b642ae8 100755
--- a/contrib/notmuch-pick/run-tests.sh
+++ b/contrib/notmuch-pick/run-tests.sh
@@ -7,7 +7,7 @@ fail() {
     exit 1
 }
 
-TESTS="emacs-pick emacs-pick-sync"
+TESTS="emacs-pick"
 TESTFILES="$TESTS pick.expected-output"
 
 export PICK_DIR="`cd \`dirname "$0"\` && pwd`"
diff --git a/contrib/notmuch-pick/test/emacs-pick-sync b/contrib/notmuch-pick/test/emacs-pick-sync
deleted file mode 100755
index a7da0ff..0000000
--- a/contrib/notmuch-pick/test/emacs-pick-sync
+++ /dev/null
@@ -1,64 +0,0 @@
-#!/usr/bin/env bash
-
-test_description="emacs pick interface (sync parser)"
-. test-lib.sh
-
-EXPECTED=$TEST_DIRECTORY/pick.expected-output
-
-add_email_corpus
-test_begin_subtest "Do we have emacs"
-test_emacs '(insert "hello\n")
-	    (test-output)'
-cat <<EOF >EXPECTED
-hello
-EOF
-test_expect_equal_file OUTPUT EXPECTED
-
-test_begin_subtest "Basic notmuch-pick view in emacs"
-test_emacs '(add-to-list (quote load-path) "'$PICK_DIR'")
-	    (require (quote notmuch-pick))
-	    (setq notmuch-pick-asynchronous-parser nil)
-	    (notmuch-pick "tag:inbox")
-	    (notmuch-test-wait)
-	    (test-output)
-	    (delete-other-windows)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox
-
-test_begin_subtest "Navigation of notmuch-hello to search results"
-test_emacs '(notmuch-hello)
-	    (goto-char (point-min))
-	    (re-search-forward "inbox")
-	    (widget-button-press (1- (point)))
-	    (notmuch-test-wait)
-	    (notmuch-pick-from-search-current-query)
-	    (notmuch-test-wait)
-	    (test-output)
-	    (delete-other-windows)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-tag-inbox
-
-test_begin_subtest "Pick of a single thread (from search)"
-test_emacs '(notmuch-hello)
-	    (goto-char (point-min))
-	    (re-search-forward "inbox")
-	    (widget-button-press (1- (point)))
-	    (notmuch-test-wait)
-	    (notmuch-pick-from-search-thread)
-	    (notmuch-test-wait)
-	    (test-output)
-	    (delete-other-windows)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-single-thread
-
-test_begin_subtest "Pick of a single thread (from show)"
-test_emacs '(notmuch-hello)
-	    (goto-char (point-min))
-	    (re-search-forward "inbox")
-	    (widget-button-press (1- (point)))
-	    (notmuch-test-wait)
-	    (notmuch-search-show-thread)
-	    (notmuch-pick-from-show-current-query)
-	    (notmuch-test-wait)
-	    (test-output)
-	    (delete-other-windows)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-pick-single-thread
-
-test_done
-- 
1.7.9.1

Re: [PATCH 0/2] contrib: pick: remove sync parser

[Tomi Ollila]

On Mon, Aug 26 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Pick has two possible parsing methods: a synchronous one (like show)
> and an async one (like search). The async parser has been the default
> since at least the time pick first entered contrib. I don't believe
> there have been any reported problems with it during this time so we
> might as well remove the sync parser and the associated defcustom. We
> also remove the sync tests.

LGTM (and +1 for the code loss :)

>
> Best wishes
>
> Mark

Tomi
>
>
>
>
> Mark Walters (2):
>   contrib: pick: remove sync parser
>   contrib: pick: remove sync tests
>
>  contrib/notmuch-pick/notmuch-pick.el      |   36 +++++-----------
>  contrib/notmuch-pick/run-tests.sh         |    2 +-
>  contrib/notmuch-pick/test/emacs-pick-sync |   64 -----------------------------
>  3 files changed, 12 insertions(+), 90 deletions(-)
>  delete mode 100755 contrib/notmuch-pick/test/emacs-pick-sync
>
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] contrib: pick: remove sync parser

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Previously pick had the option of using an async parser like search or
> a sync parser like show. The async parser has always been the default
> and it seems fine so we can remove the sync one and the corresponding
> defcustom.

pushed both.

d