[PATCH v2 0/5] Make Emacs search use sexp format
Mark Walters
markwalters1009 at gmail.com
Fri May 31 15:38:37 PDT 2013
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 at MIT.EDU> writes:
> This is v2 of id:1368851472-5382-1-git-send-email-amdragon at 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)
More information about the notmuch
mailing list