From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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.