This version LGTM +1. I will let you and Tomi decide which shell magic to go with. One very trivial comment on a comment below Best wishes Mark Austin Clements <amdragon@MIT.EDU> writes: > This is v2 of id:1368851472-5382-1-git-send-email-amdragon@mit.edu. > The most substantial change from v1 is that the streaming S-expression > parser now requires the caller to invoke it from the appropriate > buffer and no longer attempts to track the buffer itself. For subtle > reasons arising from per-window points, the only *correct* way to use > the interface before required the caller to invoke it from the > appropriate buffer anyway (or risk losing track of what had been > parsed). The only place that currently invokes the streaming > S-expression parser already satisfied this requirement. > > I decided *not* to use --stderr to redirect stderr. As discussed on > IRC, --stderr causes serious problems for wrapper scripts, which > either have to handle --stderr themselves or risk mixing their own > stderr with stdout (e.g., errors from ssh) or, worse, redirecting > notmuch's stderr to a (world-readable) file on a *remote* machine. I > did fix the exec-path problem that Tomi pointed out, so > notmuch-command will continue to be searched for in exec-path, like it > currently is. > > The diff from v1 is below, with whitespace changes because of > re-indentation in the S-expression parser. > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index a543471..180f63d 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -493,10 +493,13 @@ status." > (let* ((err-file (make-temp-file "nmerr")) > ;; Use a pipe > (process-connection-type nil) > + ;; Find notmuch using Emacs' `exec-path' > + (command (or (executable-find notmuch-command) > + (error "command not found: %s" notmuch-command))) > (proc (apply #'start-process name buffer > "sh" "-c" > "ERR=\"$1\"; shift; exec \"$0\" \"$@\" 2>\"$ERR\"" > - notmuch-command err-file args))) > + command err-file args))) > (process-put proc 'err-file err-file) > (process-put proc 'sub-sentinel sentinel) > (process-put proc 'real-command (cons notmuch-command args)) > @@ -507,7 +510,7 @@ status." > (let ((err-file (process-get proc 'err-file)) > (sub-sentinel (process-get proc 'sub-sentinel)) > (real-command (process-get proc 'real-command))) > - (condition-case-unless-debug err > + (condition-case err > (progn > ;; Invoke the sub-sentinel, if any > (when sub-sentinel > @@ -530,7 +533,8 @@ status." > (when warnings > (notmuch-logged-error (car warnings) (cdr warnings))))) > (error > - ;; Don't signal an error from a sentinel > + ;; Emacs behaves strangely if error an error escapes from a > + ;; sentinel, so turns errors into messages. 2 typos: "error an error" and turns should be turn. > (message "%s" (error-message-string err)))) > (ignore-errors (delete-file err-file)))) > > diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el > index 1b7cf64..d59c0e1 100644 > --- a/emacs/notmuch-parser.el > +++ b/emacs/notmuch-parser.el > @@ -21,8 +21,8 @@ > > (require 'cl) > > -(defun notmuch-sexp-create-parser (buffer) > - "Return a streaming S-expression parser that reads from BUFFER. > +(defun notmuch-sexp-create-parser () > + "Return a new streaming S-expression parser. > > This parser is designed to incrementally read an S-expression > whose structure is known to the caller. Like a typical > @@ -33,12 +33,11 @@ input to be a list and descends into it, allowing its elements to > be read one at a time or further descended into. Both functions > can return 'retry to indicate that not enough input is available. > > -The parser always consumes input from BUFFER's point. Hence, the > -caller is allowed to delete any data before point and may > -resynchronize after an error by moving point." > +The parser always consumes input from point in the current > +buffer. Hence, the caller is allowed to delete any data before > +point and may resynchronize after an error by moving point." > > (vector 'notmuch-sexp-parser > - buffer > ;; List depth > 0 > ;; Partial parse position marker > @@ -46,13 +45,12 @@ resynchronize after an error by moving point." > ;; Partial parse state > nil)) > > -(defmacro notmuch-sexp--buffer (sp) `(aref ,sp 1)) > -(defmacro notmuch-sexp--depth (sp) `(aref ,sp 2)) > -(defmacro notmuch-sexp--partial-pos (sp) `(aref ,sp 3)) > -(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 4)) > +(defmacro notmuch-sexp--depth (sp) `(aref ,sp 1)) > +(defmacro notmuch-sexp--partial-pos (sp) `(aref ,sp 2)) > +(defmacro notmuch-sexp--partial-state (sp) `(aref ,sp 3)) > > (defun notmuch-sexp-read (sp) > - "Consume and return the value at point in SP's buffer. > + "Consume and return the value at point in the current buffer. > > Returns 'retry if there is insufficient input to parse a complete > value (though it may still move point over whitespace). If the > @@ -61,14 +59,13 @@ list, this moves point just past the terminator and returns 'end. > Otherwise, this moves point to just past the end of the value and > returns the value." > > - (with-current-buffer (notmuch-sexp--buffer sp) > (skip-chars-forward " \n\r\t") > (cond ((eobp) 'retry) > ((= (char-after) ?\)) > ;; We've reached the end of a list > (if (= (notmuch-sexp--depth sp) 0) > ;; .. but we weren't in a list. Let read signal the > - ;; error. > + ;; error to be consistent with all other code paths. > (read (current-buffer)) > ;; Go up a level and return an end token > (decf (notmuch-sexp--depth sp)) > @@ -124,7 +121,7 @@ returns the value." > 'retry)) > (end-of-file > (goto-char start) > - 'retry))))))) > + 'retry)))))) > > (defun notmuch-sexp-begin-list (sp) > "Parse the beginning of a list value and enter the list. > @@ -136,7 +133,6 @@ returns t. Later calls to `notmuch-sexp-read' will return the > elements inside the list. If the input in buffer is not the > beginning of a list, throw invalid-read-syntax." > > - (with-current-buffer (notmuch-sexp--buffer sp) > (skip-chars-forward " \n\r\t") > (cond ((eobp) 'retry) > ((= (char-after) ?\() > @@ -146,7 +142,7 @@ beginning of a list, throw invalid-read-syntax." > (t > ;; Skip over the bad character like `read' does > (forward-char) > - (signal 'invalid-read-syntax (list (string (char-before)))))))) > + (signal 'invalid-read-syntax (list (string (char-before))))))) > > (defun notmuch-sexp-eof (sp) > "Signal an error if there is more data in SP's buffer. > @@ -154,10 +150,9 @@ beginning of a list, throw invalid-read-syntax." > Moves point to the beginning of any trailing data or to the end > of the buffer if there is only trailing whitespace." > > - (with-current-buffer (notmuch-sexp--buffer sp) > (skip-chars-forward " \n\r\t") > (unless (eobp) > - (error "Trailing garbage following expression")))) > + (error "Trailing garbage following expression"))) > > (defvar notmuch-sexp--parser nil > "The buffer-local notmuch-sexp-parser instance. > @@ -170,7 +165,7 @@ Used by `notmuch-sexp-parse-partial-list'.") > (defun notmuch-sexp-parse-partial-list (result-function result-buffer) > "Incrementally parse an S-expression list from the current buffer. > > -This function consume an S-expression list from the current > +This function consumes an S-expression list from the current > buffer, applying RESULT-FUNCTION in RESULT-BUFFER to each > complete value in the list. It operates incrementally and should > be called whenever the input buffer has been extended with > @@ -180,7 +175,7 @@ move point in the input buffer." > ;; Set up the initial state > (unless (local-variable-p 'notmuch-sexp--parser) > (set (make-local-variable 'notmuch-sexp--parser) > - (notmuch-sexp-create-parser (current-buffer))) > + (notmuch-sexp-create-parser)) > (set (make-local-variable 'notmuch-sexp--state) 'begin)) > (let (done) > (while (not done)