From: Austin Clements Date: Mon, 30 Jul 2012 01:35:06 +0000 (+2000) Subject: Re: [PATCH] emacs: move async json parser to its own function X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=77b6234652eda65df85958e5f028b142fa071181;p=notmuch-archives.git Re: [PATCH] emacs: move async json parser to its own function --- diff --git a/be/bfbcc42243f1dacc7f8cfb09abe7b243a123f8 b/be/bfbcc42243f1dacc7f8cfb09abe7b243a123f8 new file mode 100644 index 000000000..0d42b1f8b --- /dev/null +++ b/be/bfbcc42243f1dacc7f8cfb09abe7b243a123f8 @@ -0,0 +1,217 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 5A293431FAF + for ; Sun, 29 Jul 2012 18:35:16 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id SSDr-+mrrIuW for ; + Sun, 29 Jul 2012 18:35:12 -0700 (PDT) +Received: from dmz-mailsec-scanner-6.mit.edu (DMZ-MAILSEC-SCANNER-6.MIT.EDU + [18.7.68.35]) + by olra.theworths.org (Postfix) with ESMTP id 354A3431FAE + for ; Sun, 29 Jul 2012 18:35:12 -0700 (PDT) +X-AuditID: 12074423-b7f396d0000008f4-7a-5015e4cd8a59 +Received: from mailhub-auth-3.mit.edu ( [18.9.21.43]) + by dmz-mailsec-scanner-6.mit.edu (Symantec Messaging Gateway) with SMTP + id 07.13.02292.DC4E5105; Sun, 29 Jul 2012 21:35:09 -0400 (EDT) +Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) + by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q6U1Z8nq005853; + Sun, 29 Jul 2012 21:35:09 -0400 +Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) + (authenticated bits=0) + (User authenticated as amdragon@ATHENA.MIT.EDU) + by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q6U1Z7C6029430 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); + Sun, 29 Jul 2012 21:35:08 -0400 (EDT) +Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) + (envelope-from ) + id 1Svet0-0000Gh-Vu; Sun, 29 Jul 2012 21:35:07 -0400 +Date: Sun, 29 Jul 2012 21:35:06 -0400 +From: Austin Clements +To: Mark Walters +Subject: Re: [PATCH] emacs: move async json parser to its own function +Message-ID: <20120730013506.GF8502@mit.edu> +References: <87k3xo85tv.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline +In-Reply-To: <87k3xo85tv.fsf@qmul.ac.uk> +User-Agent: Mutt/1.5.21 (2010-09-15) +X-Brightmail-Tracker: + H4sIAAAAAAAAA+NgFmpkleLIzCtJLcpLzFFi42IR4hTV1j37RDTA4OEWYYvVc3ksrt+cyezA + 5LFz1l12j2erbjEHMEVx2aSk5mSWpRbp2yVwZWzum8tW8FGnYs+8DcwNjHOVuxg5OCQETCSm + T/HtYuQEMsUkLtxbz9bFyMUhJLCPUeJv81RGCGcDo0Tj4fcsEM5JJolzNw9DOUsYJZ7OecMM + 0s8ioCox90kHE4jNJqAhsW3/ckYQW0RAR+L2oQXsIDazgLTEt9/NTCCrhQXcJD4vDQYJ8wpo + S/x9dB6sREhAXeLTnFnsEHFBiZMzn7BAtGpJ3Pj3EqwVZMzyfxwgYU6gTfe+3WADsUUFVCSm + nNzGNoFRaBaS7llIumchdC9gZF7FKJuSW6Wbm5iZU5yarFucnJiXl1qka6aXm1mil5pSuokR + FNLsLso7GP8cVDrEKMDBqMTD23lVNECINbGsuDL3EKMkB5OSKG/LDaAQX1J+SmVGYnFGfFFp + TmrxIUYJDmYlEd5r54ByvCmJlVWpRfkwKWkOFiVx3mspN/2FBNITS1KzU1MLUotgsjIcHEoS + vAHA2BUSLEpNT61Iy8wpQUgzcXCCDOcBGm4CUsNbXJCYW5yZDpE/xagoJc775TFQQgAkkVGa + B9cLSzmvGMWBXhHm9QRp5wGmK7juV0CDmYAGW0QLgQwuSURISTUwbvp5dIty+TrXStuKb91v + zfZdiTv37OMhpQ3KX8uSI/x9Vj9QfV45VUu/90tC72bzlA0T3EJ/i91oEmFQmMIuH1eYaxn1 + fWeRrLywQIrv/Pnh3x9vKj3wJGJjUN0Ub95VDzfuFdrivvzpem2dnX6tJ7nqDyz/Jr/jf7KA + mFXHnNufpH7VTwpdqsRSnJFoqMVcVJwIAHYDyncUAwAA +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 30 Jul 2012 01:35:16 -0000 + +This seems like a good idea, but as a generic interface, this seems +suboptimal. In particular, it seems odd that a parsing function would +have to know about a process and require the caller to set up various +process properties and buffer-local variables. What about something +dedicated to incrementally parsing lists, like the async parser but +more specialized? Something along the lines of, + +(defun notmuch-json-parse-partial-list (result-function error-function + &optional buffer) + "Parse a partial JSON list from BUFFER (or the current buffer). + +This function consumes a JSON list from BUFFER, applying +RESULT-FUNCTION to each complete value in the list. It operates +incrementally and should be called whenever the buffer has been +extended with additional data. + +If there is a syntax error, this will attempt to resynchronize with +the input and will apply ERROR-FUNCTION to any input that was +skipped. + +This calls RESULT-FUNCTION and ERROR-FUNCTION with the same current +buffer as this function is called with (that is, this function does +not visibly switch to BUFFER)." + ...) + +This could use buffer-local variables for tracking the async parser +state as well its own state. It could even set these up automatically +when called for the first time in a buffer without them set, making it +very DWIM. It would clearly require a little more help from the +caller for process management than your patch does (and a little less +for parser setup), but I think the genericity would be worth it. + +Quoth Mark Walters on Jul 28 at 12:48 pm: +> +> We separate out the json parser into its own function. +> --- +> +> Hi +> +> Notmuch pick uses the new asynchronous json parser and the code to do so +> is almost identical to that for the search mode. Thus separate out the +> parsing in search mode into a more general function that can easily be +> used by both pick and search. +> +> This saves nearly 50 lines of duplicated code in notmuch-pick.el. +> +> The function notmuch-json-async-parse should probably be move in +> notmuch-lib but that can be a follow on patch. +> +> Best wishes +> +> Mark +> +> emacs/notmuch.el | 46 ++++++++++++++++++++++++++++++++++++---------- +> 1 files changed, 36 insertions(+), 10 deletions(-) +> +> diff --git a/emacs/notmuch.el b/emacs/notmuch.el +> index fd1836f..ee01028 100644 +> --- a/emacs/notmuch.el +> +++ b/emacs/notmuch.el +> @@ -816,7 +816,32 @@ non-authors is found, assume that all of the authors match." +> "Incremental JSON parser for the search process filter.") +> +> (defun notmuch-search-process-filter (proc string) +> - "Process and filter the output of \"notmuch search\"" +> + "Process and filter the output of \"notmuch search\" using the asynchronous parser." +> + (setq notmuch-search-process-state +> + (notmuch-json-async-parse proc +> + string +> + notmuch-search-process-state +> + notmuch-search-json-parser +> + 'notmuch-search-show-result +> + 'notmuch-search-show-error))) +> + +> +(defun notmuch-json-async-parse (proc string process-state parser result-function error-function) +> + "Process and filter the output using the asynchronous parser. +> + +> +This function steps into the first level of JSON nesting and then +> +applies RESULT-FUNCTION to each complete JSON object as it comes +> +in. +> + +> +PROC is the process: it should have a results buffer as +> +process-buffer and a 'parse-buf for the incoming json. +> +PROCESS-STATE the current state of filter process +> +STRING the incoming data +> +PARSER the parser +> +RESULT-FUNCTION a function to call on complete pieces of json +> +ERROR-FUNCTION the function to call on errors +> + +> +The function returns the new PROCESS-STATE" +> + +> (let ((results-buf (process-buffer proc)) +> (parse-buf (process-get proc 'parse-buf)) +> (inhibit-read-only t) +> @@ -831,28 +856,28 @@ non-authors is found, assume that all of the authors match." +> (with-current-buffer results-buf +> (while (not done) +> (condition-case nil +> - (case notmuch-search-process-state +> + (case process-state +> ((begin) +> ;; Enter the results list +> (if (eq (notmuch-json-begin-compound +> - notmuch-search-json-parser) 'retry) +> + parser) 'retry) +> (setq done t) +> - (setq notmuch-search-process-state 'result))) +> + (setq process-state 'result))) +> ((result) +> ;; Parse a result +> - (let ((result (notmuch-json-read notmuch-search-json-parser))) +> + (let ((result (notmuch-json-read parser))) +> (case result +> ((retry) (setq done t)) +> - ((end) (setq notmuch-search-process-state 'end)) +> - (otherwise (notmuch-search-show-result result))))) +> + ((end) (setq process-state 'end)) +> + (otherwise (funcall result-function result))))) +> ((end) +> ;; Any trailing data is unexpected +> - (notmuch-json-eof notmuch-search-json-parser) +> + (notmuch-json-eof parser) +> (setq done t))) +> (json-error +> ;; Do our best to resynchronize and ensure forward +> ;; progress +> - (notmuch-search-show-error +> + (funcall error-function +> "%s" +> (with-current-buffer parse-buf +> (let ((bad (buffer-substring (line-beginning-position) +> @@ -861,7 +886,8 @@ non-authors is found, assume that all of the authors match." +> bad)))))) +> ;; Clear out what we've parsed +> (with-current-buffer parse-buf +> - (delete-region (point-min) (point))))))) +> + (delete-region (point-min) (point)))) +> + process-state))) +> +> (defun notmuch-search-tag-all (&optional tag-changes) +> "Add/remove tags from all messages in current search buffer.