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 DFCF5431FAF for ; Mon, 30 Jul 2012 13:35:41 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -1.098 X-Spam-Level: X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] 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 eu4rvh0TM589 for ; Mon, 30 Jul 2012 13:35:37 -0700 (PDT) Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 83CC2431FAE for ; Mon, 30 Jul 2012 13:35:37 -0700 (PDT) Received: from smtp.qmul.ac.uk ([138.37.6.40]) by mail2.qmul.ac.uk with esmtp (Exim 4.71) (envelope-from ) id 1Svwge-0001si-9F; Mon, 30 Jul 2012 21:35:32 +0100 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223] helo=localhost) by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69) (envelope-from ) id 1Svwgd-00022y-R5; Mon, 30 Jul 2012 21:35:32 +0100 From: Mark Walters To: Austin Clements Subject: Re: [PATCH] emacs: move async json parser to its own function In-Reply-To: <20120730013506.GF8502@mit.edu> References: <87k3xo85tv.fsf@qmul.ac.uk> <20120730013506.GF8502@mit.edu> User-Agent: Notmuch/0.13.2+96~g634443c (http://notmuchmail.org) Emacs/23.4.1 (x86_64-pc-linux-gnu) Date: Mon, 30 Jul 2012 21:35:30 +0100 Message-ID: <87obmxyokt.fsf@qmul.ac.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Sender-Host-Address: 94.192.233.223 X-QM-SPAM-Info: Sender has good ham record. :) X-QM-Body-MD5: 2a8679cc1c65562ecb10fffbf0b0b313 (of first 20000 bytes) X-SpamAssassin-Score: -1.8 X-SpamAssassin-SpamBar: - X-SpamAssassin-Report: The QM spam filters have analysed this message to determine if it is spam. We require at least 5.0 points to mark a message as spam. This message scored -1.8 points. Summary of the scoring: * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, * medium trust * [138.37.6.40 listed in list.dnswl.org] * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider * (markwalters1009[at]gmail.com) * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay * domain * 0.5 AWL AWL: From: address is in the auto white-list X-QM-Scan-Virus: ClamAV says the message is clean 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 20:35:42 -0000 On Mon, 30 Jul 2012, Austin Clements wrote: > 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. This all seems good: I will send a draft patch in a reply to this email. The one change I have made is to make the function called with current-buffer the parse-buffer and give the results-buffer as an argument. This seems more natural to me as the caller has probably just added data to the parse-buffer, and it slightly simplifies the function. The other change from the current state is that the parser and state variable are buffer local to the parse-buffer rather than the results-buffer. Best wishes Mark > > 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.