[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-27 Thread Mark Walters

Austin Clements  writes:

> Quoth Mark Walters on May 25 at  9:59 am:
>> 
>> Hi
>> 
>> On Wed, 22 May 2013, Austin Clements  wrote:
>> > On Tue, 21 May 2013, Mark Walters  wrote:
>> >> Hi
>> >>
>> >> This patch looks good to me. Some minor comments below.
>> >
>> > Some minor replies below.
>> >
>> > In building some other code on top of this, I found an interesting (but
>> > easy to fix) interface bug.  Currently, the interface is designed as if
>> > it doesn't matter what buffer these functions are called from, however,
>> > because they move point and expect this point motion to persist, it's
>> > actually not safe to call this interface unless the caller is in the
>> > right buffer anyway.  For example, if the buffer is selected in a
>> > window, the with-current-buffer in the parser functions will actually
>> > move a *temporary* point, meaning that the only way the caller can
>> > discover the new point is to first select the buffer for itself.  I can
>> > think of two solutions: 1) maintain our own mark for the parser's
>> > current position or 2) tweak the doc strings and code so that it reads
>> > from the current buffer.  1 keeps the interface the way it's currently
>> > documented, but complicates the parser implementation and interface and
>> > doesn't simplify the caller.  2 simplifies the parser and it turns out
>> > all callers already satisfy the requirement.
>> 
>> I am confused by this: the docs strings for json/sexp-parse-partial-list
>> both say something like "Parse a partial JSON list from current buffer"?
>> Or do you mean the with-current-buffer in notmuch-search-process-filter?
>
> I was referring to the lower level parser, which effectively has the
> same requirement but isn't documented to and has code that pointlessly
> tries to track the parsing buffer (I consider
> json/sexp-parse-partial-list to be a helper).  In fact, one reason the
> lower level parser didn't choke is because right now we only use it
> through json/sexp-parse-partial-list, which requires that it be called
> from the right buffer.

Right I think I see. I am definitely happy with insisting on being
called from the correct buffer in the low level code.

>> >> On Sat, 18 May 2013, Austin Clements  wrote:
>> >>> This provides the same interface as the streaming JSON parser, but
>> >>> reads S-expressions incrementally.  The only difference is that the
>> >>> `notmuch-sexp-parse-partial-list' helper does not handle interleaved
>> >>> error messages (since we now have the ability to separate these out at
>> >>> the invocation level), so it no longer takes an error function and
>> >>> does not need to do the horrible resynchronization that the JSON
>> >>> parser had to.
>> >>>
>> >>> Some implementation improvements have been made over the JSON parser.
>> >>> This uses a vector instead of a list for the parser data structure,
>> >>> since this allows faster access to elements (and modern versions of
>> >>> Emacs handle storage of small vectors efficiently).  Private functions
>> >>> follow the "prefix--name" convention.  And the implementation is much
>> >>> simpler overall because S-expressions are much easier to parse.
>> >>> ---
>> >>>  emacs/Makefile.local|1 +
>> >>>  emacs/notmuch-parser.el |  212 
>> >>> +++
>> >>>  2 files changed, 213 insertions(+)
>> >>>  create mode 100644 emacs/notmuch-parser.el
>> >>>
>> >>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>> >>> index 456700a..a910aff 100644
>> >>> --- a/emacs/Makefile.local
>> >>> +++ b/emacs/Makefile.local
>> >>> @@ -3,6 +3,7 @@
>> >>>  dir := emacs
>> >>>  emacs_sources := \
>> >>>  $(dir)/notmuch-lib.el \
>> >>> +$(dir)/notmuch-parser.el \
>> >>>  $(dir)/notmuch.el \
>> >>>  $(dir)/notmuch-query.el \
>> >>>  $(dir)/notmuch-show.el \
>> >>> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
>> >>> new file mode 100644
>> >>> index 000..1b7cf64
>> >>> --- /dev/null
>> >>> +++ b/emacs/notmuch-parser.el
>> >>> @@ -0,0 +1,212 @@
>> >>> +;; notmuch-parser.el --- streaming S-expression parser
>> >>> +;;
>> >>> +;; Copyright ? Austin Clements
>> >>> +;;
>> >>> +;; This file is part of Notmuch.
>> >>> +;;
>> >>> +;; Notmuch is free software: you can redistribute it and/or modify it
>> >>> +;; under the terms of the GNU General Public License as published by
>> >>> +;; the Free Software Foundation, either version 3 of the License, or
>> >>> +;; (at your option) any later version.
>> >>> +;;
>> >>> +;; Notmuch is distributed in the hope that it will be useful, but
>> >>> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
>> >>> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> >>> +;; General Public License for more details.
>> >>> +;;
>> >>> +;; You should have received a copy of the GNU General Public License
>> >>> +;; along with Notmuch.  If not, see .
>> >>> +;;
>> >>> +;; 

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-27 Thread Austin Clements
Quoth Mark Walters on May 25 at  9:59 am:
> 
> Hi
> 
> On Wed, 22 May 2013, Austin Clements  wrote:
> > On Tue, 21 May 2013, Mark Walters  wrote:
> >> Hi
> >>
> >> This patch looks good to me. Some minor comments below.
> >
> > Some minor replies below.
> >
> > In building some other code on top of this, I found an interesting (but
> > easy to fix) interface bug.  Currently, the interface is designed as if
> > it doesn't matter what buffer these functions are called from, however,
> > because they move point and expect this point motion to persist, it's
> > actually not safe to call this interface unless the caller is in the
> > right buffer anyway.  For example, if the buffer is selected in a
> > window, the with-current-buffer in the parser functions will actually
> > move a *temporary* point, meaning that the only way the caller can
> > discover the new point is to first select the buffer for itself.  I can
> > think of two solutions: 1) maintain our own mark for the parser's
> > current position or 2) tweak the doc strings and code so that it reads
> > from the current buffer.  1 keeps the interface the way it's currently
> > documented, but complicates the parser implementation and interface and
> > doesn't simplify the caller.  2 simplifies the parser and it turns out
> > all callers already satisfy the requirement.
> 
> I am confused by this: the docs strings for json/sexp-parse-partial-list
> both say something like "Parse a partial JSON list from current buffer"?
> Or do you mean the with-current-buffer in notmuch-search-process-filter?

I was referring to the lower level parser, which effectively has the
same requirement but isn't documented to and has code that pointlessly
tries to track the parsing buffer (I consider
json/sexp-parse-partial-list to be a helper).  In fact, one reason the
lower level parser didn't choke is because right now we only use it
through json/sexp-parse-partial-list, which requires that it be called
from the right buffer.

> >> On Sat, 18 May 2013, Austin Clements  wrote:
> >>> This provides the same interface as the streaming JSON parser, but
> >>> reads S-expressions incrementally.  The only difference is that the
> >>> `notmuch-sexp-parse-partial-list' helper does not handle interleaved
> >>> error messages (since we now have the ability to separate these out at
> >>> the invocation level), so it no longer takes an error function and
> >>> does not need to do the horrible resynchronization that the JSON
> >>> parser had to.
> >>>
> >>> Some implementation improvements have been made over the JSON parser.
> >>> This uses a vector instead of a list for the parser data structure,
> >>> since this allows faster access to elements (and modern versions of
> >>> Emacs handle storage of small vectors efficiently).  Private functions
> >>> follow the "prefix--name" convention.  And the implementation is much
> >>> simpler overall because S-expressions are much easier to parse.
> >>> ---
> >>>  emacs/Makefile.local|1 +
> >>>  emacs/notmuch-parser.el |  212 
> >>> +++
> >>>  2 files changed, 213 insertions(+)
> >>>  create mode 100644 emacs/notmuch-parser.el
> >>>
> >>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> >>> index 456700a..a910aff 100644
> >>> --- a/emacs/Makefile.local
> >>> +++ b/emacs/Makefile.local
> >>> @@ -3,6 +3,7 @@
> >>>  dir := emacs
> >>>  emacs_sources := \
> >>>   $(dir)/notmuch-lib.el \
> >>> + $(dir)/notmuch-parser.el \
> >>>   $(dir)/notmuch.el \
> >>>   $(dir)/notmuch-query.el \
> >>>   $(dir)/notmuch-show.el \
> >>> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
> >>> new file mode 100644
> >>> index 000..1b7cf64
> >>> --- /dev/null
> >>> +++ b/emacs/notmuch-parser.el
> >>> @@ -0,0 +1,212 @@
> >>> +;; notmuch-parser.el --- streaming S-expression parser
> >>> +;;
> >>> +;; Copyright ? Austin Clements
> >>> +;;
> >>> +;; This file is part of Notmuch.
> >>> +;;
> >>> +;; Notmuch is free software: you can redistribute it and/or modify it
> >>> +;; under the terms of the GNU General Public License as published by
> >>> +;; the Free Software Foundation, either version 3 of the License, or
> >>> +;; (at your option) any later version.
> >>> +;;
> >>> +;; Notmuch is distributed in the hope that it will be useful, but
> >>> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >>> +;; General Public License for more details.
> >>> +;;
> >>> +;; You should have received a copy of the GNU General Public License
> >>> +;; along with Notmuch.  If not, see .
> >>> +;;
> >>> +;; Authors: Austin Clements 
> >>> +
> >>> +(require 'cl)
> >>> +
> >>> +(defun notmuch-sexp-create-parser (buffer)
> >>> +  "Return a streaming S-expression parser that reads from BUFFER.
> >>> +
> >>> +This parser is designed to incrementally read an S-expression
> >>> +whose structure is 

Re: [PATCH 4/5] emacs: Streaming S-expression parser

2013-05-27 Thread Austin Clements
Quoth Mark Walters on May 25 at  9:59 am:
 
 Hi
 
 On Wed, 22 May 2013, Austin Clements amdra...@mit.edu wrote:
  On Tue, 21 May 2013, Mark Walters markwalters1...@gmail.com wrote:
  Hi
 
  This patch looks good to me. Some minor comments below.
 
  Some minor replies below.
 
  In building some other code on top of this, I found an interesting (but
  easy to fix) interface bug.  Currently, the interface is designed as if
  it doesn't matter what buffer these functions are called from, however,
  because they move point and expect this point motion to persist, it's
  actually not safe to call this interface unless the caller is in the
  right buffer anyway.  For example, if the buffer is selected in a
  window, the with-current-buffer in the parser functions will actually
  move a *temporary* point, meaning that the only way the caller can
  discover the new point is to first select the buffer for itself.  I can
  think of two solutions: 1) maintain our own mark for the parser's
  current position or 2) tweak the doc strings and code so that it reads
  from the current buffer.  1 keeps the interface the way it's currently
  documented, but complicates the parser implementation and interface and
  doesn't simplify the caller.  2 simplifies the parser and it turns out
  all callers already satisfy the requirement.
 
 I am confused by this: the docs strings for json/sexp-parse-partial-list
 both say something like Parse a partial JSON list from current buffer?
 Or do you mean the with-current-buffer in notmuch-search-process-filter?

I was referring to the lower level parser, which effectively has the
same requirement but isn't documented to and has code that pointlessly
tries to track the parsing buffer (I consider
json/sexp-parse-partial-list to be a helper).  In fact, one reason the
lower level parser didn't choke is because right now we only use it
through json/sexp-parse-partial-list, which requires that it be called
from the right buffer.

  On Sat, 18 May 2013, Austin Clements amdra...@mit.edu wrote:
  This provides the same interface as the streaming JSON parser, but
  reads S-expressions incrementally.  The only difference is that the
  `notmuch-sexp-parse-partial-list' helper does not handle interleaved
  error messages (since we now have the ability to separate these out at
  the invocation level), so it no longer takes an error function and
  does not need to do the horrible resynchronization that the JSON
  parser had to.
 
  Some implementation improvements have been made over the JSON parser.
  This uses a vector instead of a list for the parser data structure,
  since this allows faster access to elements (and modern versions of
  Emacs handle storage of small vectors efficiently).  Private functions
  follow the prefix--name convention.  And the implementation is much
  simpler overall because S-expressions are much easier to parse.
  ---
   emacs/Makefile.local|1 +
   emacs/notmuch-parser.el |  212 
  +++
   2 files changed, 213 insertions(+)
   create mode 100644 emacs/notmuch-parser.el
 
  diff --git a/emacs/Makefile.local b/emacs/Makefile.local
  index 456700a..a910aff 100644
  --- a/emacs/Makefile.local
  +++ b/emacs/Makefile.local
  @@ -3,6 +3,7 @@
   dir := emacs
   emacs_sources := \
$(dir)/notmuch-lib.el \
  + $(dir)/notmuch-parser.el \
$(dir)/notmuch.el \
$(dir)/notmuch-query.el \
$(dir)/notmuch-show.el \
  diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
  new file mode 100644
  index 000..1b7cf64
  --- /dev/null
  +++ b/emacs/notmuch-parser.el
  @@ -0,0 +1,212 @@
  +;; notmuch-parser.el --- streaming S-expression parser
  +;;
  +;; Copyright © Austin Clements
  +;;
  +;; This file is part of Notmuch.
  +;;
  +;; Notmuch is free software: you can redistribute it and/or modify it
  +;; under the terms of the GNU General Public License as published by
  +;; the Free Software Foundation, either version 3 of the License, or
  +;; (at your option) any later version.
  +;;
  +;; Notmuch is distributed in the hope that it will be useful, but
  +;; WITHOUT ANY WARRANTY; without even the implied warranty of
  +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  +;; General Public License for more details.
  +;;
  +;; You should have received a copy of the GNU General Public License
  +;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
  +;;
  +;; Authors: Austin Clements acleme...@csail.mit.edu
  +
  +(require 'cl)
  +
  +(defun notmuch-sexp-create-parser (buffer)
  +  Return a streaming S-expression parser that reads from BUFFER.
  +
  +This parser is designed to incrementally read an S-expression
  +whose structure is known to the caller.  Like a typical
  +S-expression parsing interface, it provides a function to read a
  +complete S-expression from the input.  However, it extends this
  +with an additional function that requires the next value in the
  +input to be a 

Re: [PATCH 4/5] emacs: Streaming S-expression parser

2013-05-27 Thread Mark Walters

Austin Clements amdra...@mit.edu writes:

 Quoth Mark Walters on May 25 at  9:59 am:
 
 Hi
 
 On Wed, 22 May 2013, Austin Clements amdra...@mit.edu wrote:
  On Tue, 21 May 2013, Mark Walters markwalters1...@gmail.com wrote:
  Hi
 
  This patch looks good to me. Some minor comments below.
 
  Some minor replies below.
 
  In building some other code on top of this, I found an interesting (but
  easy to fix) interface bug.  Currently, the interface is designed as if
  it doesn't matter what buffer these functions are called from, however,
  because they move point and expect this point motion to persist, it's
  actually not safe to call this interface unless the caller is in the
  right buffer anyway.  For example, if the buffer is selected in a
  window, the with-current-buffer in the parser functions will actually
  move a *temporary* point, meaning that the only way the caller can
  discover the new point is to first select the buffer for itself.  I can
  think of two solutions: 1) maintain our own mark for the parser's
  current position or 2) tweak the doc strings and code so that it reads
  from the current buffer.  1 keeps the interface the way it's currently
  documented, but complicates the parser implementation and interface and
  doesn't simplify the caller.  2 simplifies the parser and it turns out
  all callers already satisfy the requirement.
 
 I am confused by this: the docs strings for json/sexp-parse-partial-list
 both say something like Parse a partial JSON list from current buffer?
 Or do you mean the with-current-buffer in notmuch-search-process-filter?

 I was referring to the lower level parser, which effectively has the
 same requirement but isn't documented to and has code that pointlessly
 tries to track the parsing buffer (I consider
 json/sexp-parse-partial-list to be a helper).  In fact, one reason the
 lower level parser didn't choke is because right now we only use it
 through json/sexp-parse-partial-list, which requires that it be called
 from the right buffer.

Right I think I see. I am definitely happy with insisting on being
called from the correct buffer in the low level code.

  On Sat, 18 May 2013, Austin Clements amdra...@mit.edu wrote:
  This provides the same interface as the streaming JSON parser, but
  reads S-expressions incrementally.  The only difference is that the
  `notmuch-sexp-parse-partial-list' helper does not handle interleaved
  error messages (since we now have the ability to separate these out at
  the invocation level), so it no longer takes an error function and
  does not need to do the horrible resynchronization that the JSON
  parser had to.
 
  Some implementation improvements have been made over the JSON parser.
  This uses a vector instead of a list for the parser data structure,
  since this allows faster access to elements (and modern versions of
  Emacs handle storage of small vectors efficiently).  Private functions
  follow the prefix--name convention.  And the implementation is much
  simpler overall because S-expressions are much easier to parse.
  ---
   emacs/Makefile.local|1 +
   emacs/notmuch-parser.el |  212 
  +++
   2 files changed, 213 insertions(+)
   create mode 100644 emacs/notmuch-parser.el
 
  diff --git a/emacs/Makefile.local b/emacs/Makefile.local
  index 456700a..a910aff 100644
  --- a/emacs/Makefile.local
  +++ b/emacs/Makefile.local
  @@ -3,6 +3,7 @@
   dir := emacs
   emacs_sources := \
   $(dir)/notmuch-lib.el \
  +$(dir)/notmuch-parser.el \
   $(dir)/notmuch.el \
   $(dir)/notmuch-query.el \
   $(dir)/notmuch-show.el \
  diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
  new file mode 100644
  index 000..1b7cf64
  --- /dev/null
  +++ b/emacs/notmuch-parser.el
  @@ -0,0 +1,212 @@
  +;; notmuch-parser.el --- streaming S-expression parser
  +;;
  +;; Copyright © Austin Clements
  +;;
  +;; This file is part of Notmuch.
  +;;
  +;; Notmuch is free software: you can redistribute it and/or modify it
  +;; under the terms of the GNU General Public License as published by
  +;; the Free Software Foundation, either version 3 of the License, or
  +;; (at your option) any later version.
  +;;
  +;; Notmuch is distributed in the hope that it will be useful, but
  +;; WITHOUT ANY WARRANTY; without even the implied warranty of
  +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  +;; General Public License for more details.
  +;;
  +;; You should have received a copy of the GNU General Public License
  +;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
  +;;
  +;; Authors: Austin Clements acleme...@csail.mit.edu
  +
  +(require 'cl)
  +
  +(defun notmuch-sexp-create-parser (buffer)
  +  Return a streaming S-expression parser that reads from BUFFER.
  +
  +This parser is designed to incrementally read an S-expression
  +whose structure is known to the caller.  Like a typical
  

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-25 Thread Mark Walters

Hi

On Wed, 22 May 2013, Austin Clements  wrote:
> On Tue, 21 May 2013, Mark Walters  wrote:
>> Hi
>>
>> This patch looks good to me. Some minor comments below.
>
> Some minor replies below.
>
> In building some other code on top of this, I found an interesting (but
> easy to fix) interface bug.  Currently, the interface is designed as if
> it doesn't matter what buffer these functions are called from, however,
> because they move point and expect this point motion to persist, it's
> actually not safe to call this interface unless the caller is in the
> right buffer anyway.  For example, if the buffer is selected in a
> window, the with-current-buffer in the parser functions will actually
> move a *temporary* point, meaning that the only way the caller can
> discover the new point is to first select the buffer for itself.  I can
> think of two solutions: 1) maintain our own mark for the parser's
> current position or 2) tweak the doc strings and code so that it reads
> from the current buffer.  1 keeps the interface the way it's currently
> documented, but complicates the parser implementation and interface and
> doesn't simplify the caller.  2 simplifies the parser and it turns out
> all callers already satisfy the requirement.

I am confused by this: the docs strings for json/sexp-parse-partial-list
both say something like "Parse a partial JSON list from current buffer"?
Or do you mean the with-current-buffer in notmuch-search-process-filter?

>> On Sat, 18 May 2013, Austin Clements  wrote:
>>> This provides the same interface as the streaming JSON parser, but
>>> reads S-expressions incrementally.  The only difference is that the
>>> `notmuch-sexp-parse-partial-list' helper does not handle interleaved
>>> error messages (since we now have the ability to separate these out at
>>> the invocation level), so it no longer takes an error function and
>>> does not need to do the horrible resynchronization that the JSON
>>> parser had to.
>>>
>>> Some implementation improvements have been made over the JSON parser.
>>> This uses a vector instead of a list for the parser data structure,
>>> since this allows faster access to elements (and modern versions of
>>> Emacs handle storage of small vectors efficiently).  Private functions
>>> follow the "prefix--name" convention.  And the implementation is much
>>> simpler overall because S-expressions are much easier to parse.
>>> ---
>>>  emacs/Makefile.local|1 +
>>>  emacs/notmuch-parser.el |  212 
>>> +++
>>>  2 files changed, 213 insertions(+)
>>>  create mode 100644 emacs/notmuch-parser.el
>>>
>>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>>> index 456700a..a910aff 100644
>>> --- a/emacs/Makefile.local
>>> +++ b/emacs/Makefile.local
>>> @@ -3,6 +3,7 @@
>>>  dir := emacs
>>>  emacs_sources := \
>>> $(dir)/notmuch-lib.el \
>>> +   $(dir)/notmuch-parser.el \
>>> $(dir)/notmuch.el \
>>> $(dir)/notmuch-query.el \
>>> $(dir)/notmuch-show.el \
>>> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
>>> new file mode 100644
>>> index 000..1b7cf64
>>> --- /dev/null
>>> +++ b/emacs/notmuch-parser.el
>>> @@ -0,0 +1,212 @@
>>> +;; notmuch-parser.el --- streaming S-expression parser
>>> +;;
>>> +;; Copyright ? Austin Clements
>>> +;;
>>> +;; This file is part of Notmuch.
>>> +;;
>>> +;; Notmuch is free software: you can redistribute it and/or modify it
>>> +;; under the terms of the GNU General Public License as published by
>>> +;; the Free Software Foundation, either version 3 of the License, or
>>> +;; (at your option) any later version.
>>> +;;
>>> +;; Notmuch is distributed in the hope that it will be useful, but
>>> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +;; General Public License for more details.
>>> +;;
>>> +;; You should have received a copy of the GNU General Public License
>>> +;; along with Notmuch.  If not, see .
>>> +;;
>>> +;; Authors: Austin Clements 
>>> +
>>> +(require 'cl)
>>> +
>>> +(defun notmuch-sexp-create-parser (buffer)
>>> +  "Return a streaming S-expression parser that reads from BUFFER.
>>> +
>>> +This parser is designed to incrementally read an S-expression
>>> +whose structure is known to the caller.  Like a typical
>>> +S-expression parsing interface, it provides a function to read a
>>> +complete S-expression from the input.  However, it extends this
>>> +with an additional function that requires the next value in the
>>> +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."

Re: [PATCH 4/5] emacs: Streaming S-expression parser

2013-05-25 Thread Mark Walters

Hi

On Wed, 22 May 2013, Austin Clements amdra...@mit.edu wrote:
 On Tue, 21 May 2013, Mark Walters markwalters1...@gmail.com wrote:
 Hi

 This patch looks good to me. Some minor comments below.

 Some minor replies below.

 In building some other code on top of this, I found an interesting (but
 easy to fix) interface bug.  Currently, the interface is designed as if
 it doesn't matter what buffer these functions are called from, however,
 because they move point and expect this point motion to persist, it's
 actually not safe to call this interface unless the caller is in the
 right buffer anyway.  For example, if the buffer is selected in a
 window, the with-current-buffer in the parser functions will actually
 move a *temporary* point, meaning that the only way the caller can
 discover the new point is to first select the buffer for itself.  I can
 think of two solutions: 1) maintain our own mark for the parser's
 current position or 2) tweak the doc strings and code so that it reads
 from the current buffer.  1 keeps the interface the way it's currently
 documented, but complicates the parser implementation and interface and
 doesn't simplify the caller.  2 simplifies the parser and it turns out
 all callers already satisfy the requirement.

I am confused by this: the docs strings for json/sexp-parse-partial-list
both say something like Parse a partial JSON list from current buffer?
Or do you mean the with-current-buffer in notmuch-search-process-filter?

 On Sat, 18 May 2013, Austin Clements amdra...@mit.edu wrote:
 This provides the same interface as the streaming JSON parser, but
 reads S-expressions incrementally.  The only difference is that the
 `notmuch-sexp-parse-partial-list' helper does not handle interleaved
 error messages (since we now have the ability to separate these out at
 the invocation level), so it no longer takes an error function and
 does not need to do the horrible resynchronization that the JSON
 parser had to.

 Some implementation improvements have been made over the JSON parser.
 This uses a vector instead of a list for the parser data structure,
 since this allows faster access to elements (and modern versions of
 Emacs handle storage of small vectors efficiently).  Private functions
 follow the prefix--name convention.  And the implementation is much
 simpler overall because S-expressions are much easier to parse.
 ---
  emacs/Makefile.local|1 +
  emacs/notmuch-parser.el |  212 
 +++
  2 files changed, 213 insertions(+)
  create mode 100644 emacs/notmuch-parser.el

 diff --git a/emacs/Makefile.local b/emacs/Makefile.local
 index 456700a..a910aff 100644
 --- a/emacs/Makefile.local
 +++ b/emacs/Makefile.local
 @@ -3,6 +3,7 @@
  dir := emacs
  emacs_sources := \
 $(dir)/notmuch-lib.el \
 +   $(dir)/notmuch-parser.el \
 $(dir)/notmuch.el \
 $(dir)/notmuch-query.el \
 $(dir)/notmuch-show.el \
 diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
 new file mode 100644
 index 000..1b7cf64
 --- /dev/null
 +++ b/emacs/notmuch-parser.el
 @@ -0,0 +1,212 @@
 +;; notmuch-parser.el --- streaming S-expression parser
 +;;
 +;; Copyright © Austin Clements
 +;;
 +;; This file is part of Notmuch.
 +;;
 +;; Notmuch is free software: you can redistribute it and/or modify it
 +;; under the terms of the GNU General Public License as published by
 +;; the Free Software Foundation, either version 3 of the License, or
 +;; (at your option) any later version.
 +;;
 +;; Notmuch is distributed in the hope that it will be useful, but
 +;; WITHOUT ANY WARRANTY; without even the implied warranty of
 +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 +;; General Public License for more details.
 +;;
 +;; You should have received a copy of the GNU General Public License
 +;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
 +;;
 +;; Authors: Austin Clements acleme...@csail.mit.edu
 +
 +(require 'cl)
 +
 +(defun notmuch-sexp-create-parser (buffer)
 +  Return a streaming S-expression parser that reads from BUFFER.
 +
 +This parser is designed to incrementally read an S-expression
 +whose structure is known to the caller.  Like a typical
 +S-expression parsing interface, it provides a function to read a
 +complete S-expression from the input.  However, it extends this
 +with an additional function that requires the next value in the
 +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.
 +
 +  (vector 'notmuch-sexp-parser
 + buffer
 + ;; List depth
 + 0
 + ;; Partial parse position marker
 + nil
 + ;; Partial parse state
 + nil))
 +
 +(defmacro 

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-21 Thread Austin Clements
On Tue, 21 May 2013, Mark Walters  wrote:
> Hi
>
> This patch looks good to me. Some minor comments below.

Some minor replies below.

In building some other code on top of this, I found an interesting (but
easy to fix) interface bug.  Currently, the interface is designed as if
it doesn't matter what buffer these functions are called from, however,
because they move point and expect this point motion to persist, it's
actually not safe to call this interface unless the caller is in the
right buffer anyway.  For example, if the buffer is selected in a
window, the with-current-buffer in the parser functions will actually
move a *temporary* point, meaning that the only way the caller can
discover the new point is to first select the buffer for itself.  I can
think of two solutions: 1) maintain our own mark for the parser's
current position or 2) tweak the doc strings and code so that it reads
from the current buffer.  1 keeps the interface the way it's currently
documented, but complicates the parser implementation and interface and
doesn't simplify the caller.  2 simplifies the parser and it turns out
all callers already satisfy the requirement.

> Best wishes
>
> Mark
>
>
> On Sat, 18 May 2013, Austin Clements  wrote:
>> This provides the same interface as the streaming JSON parser, but
>> reads S-expressions incrementally.  The only difference is that the
>> `notmuch-sexp-parse-partial-list' helper does not handle interleaved
>> error messages (since we now have the ability to separate these out at
>> the invocation level), so it no longer takes an error function and
>> does not need to do the horrible resynchronization that the JSON
>> parser had to.
>>
>> Some implementation improvements have been made over the JSON parser.
>> This uses a vector instead of a list for the parser data structure,
>> since this allows faster access to elements (and modern versions of
>> Emacs handle storage of small vectors efficiently).  Private functions
>> follow the "prefix--name" convention.  And the implementation is much
>> simpler overall because S-expressions are much easier to parse.
>> ---
>>  emacs/Makefile.local|1 +
>>  emacs/notmuch-parser.el |  212 
>> +++
>>  2 files changed, 213 insertions(+)
>>  create mode 100644 emacs/notmuch-parser.el
>>
>> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
>> index 456700a..a910aff 100644
>> --- a/emacs/Makefile.local
>> +++ b/emacs/Makefile.local
>> @@ -3,6 +3,7 @@
>>  dir := emacs
>>  emacs_sources := \
>>  $(dir)/notmuch-lib.el \
>> +$(dir)/notmuch-parser.el \
>>  $(dir)/notmuch.el \
>>  $(dir)/notmuch-query.el \
>>  $(dir)/notmuch-show.el \
>> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
>> new file mode 100644
>> index 000..1b7cf64
>> --- /dev/null
>> +++ b/emacs/notmuch-parser.el
>> @@ -0,0 +1,212 @@
>> +;; notmuch-parser.el --- streaming S-expression parser
>> +;;
>> +;; Copyright ? Austin Clements
>> +;;
>> +;; This file is part of Notmuch.
>> +;;
>> +;; Notmuch is free software: you can redistribute it and/or modify it
>> +;; under the terms of the GNU General Public License as published by
>> +;; the Free Software Foundation, either version 3 of the License, or
>> +;; (at your option) any later version.
>> +;;
>> +;; Notmuch is distributed in the hope that it will be useful, but
>> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
>> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +;; General Public License for more details.
>> +;;
>> +;; You should have received a copy of the GNU General Public License
>> +;; along with Notmuch.  If not, see .
>> +;;
>> +;; Authors: Austin Clements 
>> +
>> +(require 'cl)
>> +
>> +(defun notmuch-sexp-create-parser (buffer)
>> +  "Return a streaming S-expression parser that reads from BUFFER.
>> +
>> +This parser is designed to incrementally read an S-expression
>> +whose structure is known to the caller.  Like a typical
>> +S-expression parsing interface, it provides a function to read a
>> +complete S-expression from the input.  However, it extends this
>> +with an additional function that requires the next value in the
>> +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."
>> +
>> +  (vector 'notmuch-sexp-parser
>> +  buffer
>> +  ;; List depth
>> +  0
>> +  ;; Partial parse position marker
>> +  nil
>> +  ;; Partial parse state
>> +  nil))
>> +
>> +(defmacro notmuch-sexp--buffer (sp)`(aref ,sp 1))
>> +(defmacro notmuch-sexp--depth (sp) `(aref ,sp 2))
>> +(defmacro 

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-21 Thread Mark Walters

Hi

This patch looks good to me. Some minor comments below.

Best wishes

Mark


On Sat, 18 May 2013, Austin Clements  wrote:
> This provides the same interface as the streaming JSON parser, but
> reads S-expressions incrementally.  The only difference is that the
> `notmuch-sexp-parse-partial-list' helper does not handle interleaved
> error messages (since we now have the ability to separate these out at
> the invocation level), so it no longer takes an error function and
> does not need to do the horrible resynchronization that the JSON
> parser had to.
>
> Some implementation improvements have been made over the JSON parser.
> This uses a vector instead of a list for the parser data structure,
> since this allows faster access to elements (and modern versions of
> Emacs handle storage of small vectors efficiently).  Private functions
> follow the "prefix--name" convention.  And the implementation is much
> simpler overall because S-expressions are much easier to parse.
> ---
>  emacs/Makefile.local|1 +
>  emacs/notmuch-parser.el |  212 
> +++
>  2 files changed, 213 insertions(+)
>  create mode 100644 emacs/notmuch-parser.el
>
> diff --git a/emacs/Makefile.local b/emacs/Makefile.local
> index 456700a..a910aff 100644
> --- a/emacs/Makefile.local
> +++ b/emacs/Makefile.local
> @@ -3,6 +3,7 @@
>  dir := emacs
>  emacs_sources := \
>   $(dir)/notmuch-lib.el \
> + $(dir)/notmuch-parser.el \
>   $(dir)/notmuch.el \
>   $(dir)/notmuch-query.el \
>   $(dir)/notmuch-show.el \
> diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
> new file mode 100644
> index 000..1b7cf64
> --- /dev/null
> +++ b/emacs/notmuch-parser.el
> @@ -0,0 +1,212 @@
> +;; notmuch-parser.el --- streaming S-expression parser
> +;;
> +;; Copyright ? Austin Clements
> +;;
> +;; This file is part of Notmuch.
> +;;
> +;; Notmuch is free software: you can redistribute it and/or modify it
> +;; under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.
> +;;
> +;; Notmuch is distributed in the hope that it will be useful, but
> +;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +;; General Public License for more details.
> +;;
> +;; You should have received a copy of the GNU General Public License
> +;; along with Notmuch.  If not, see .
> +;;
> +;; Authors: Austin Clements 
> +
> +(require 'cl)
> +
> +(defun notmuch-sexp-create-parser (buffer)
> +  "Return a streaming S-expression parser that reads from BUFFER.
> +
> +This parser is designed to incrementally read an S-expression
> +whose structure is known to the caller.  Like a typical
> +S-expression parsing interface, it provides a function to read a
> +complete S-expression from the input.  However, it extends this
> +with an additional function that requires the next value in the
> +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."
> +
> +  (vector 'notmuch-sexp-parser
> +   buffer
> +   ;; List depth
> +   0
> +   ;; Partial parse position marker
> +   nil
> +   ;; 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))

Why the double hyphen --? Is it a name-space or some convention?


> +(defun notmuch-sexp-read (sp)
> +  "Consume and return the value at point in SP's buffer.
> +
> +Returns 'retry if there is insufficient input to parse a complete
> +value (though it may still move point over whitespace).  If the
> +parser is currently inside a list and the next token ends the
> +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.
> +(read (current-buffer))

Why is good for read to signal the error rather than us doing it?

> +  ;; Go up a level and return an end token
> +  (decf (notmuch-sexp--depth sp))
> +  

Re: [PATCH 4/5] emacs: Streaming S-expression parser

2013-05-21 Thread Mark Walters

Hi

This patch looks good to me. Some minor comments below.

Best wishes

Mark


On Sat, 18 May 2013, Austin Clements amdra...@mit.edu wrote:
 This provides the same interface as the streaming JSON parser, but
 reads S-expressions incrementally.  The only difference is that the
 `notmuch-sexp-parse-partial-list' helper does not handle interleaved
 error messages (since we now have the ability to separate these out at
 the invocation level), so it no longer takes an error function and
 does not need to do the horrible resynchronization that the JSON
 parser had to.

 Some implementation improvements have been made over the JSON parser.
 This uses a vector instead of a list for the parser data structure,
 since this allows faster access to elements (and modern versions of
 Emacs handle storage of small vectors efficiently).  Private functions
 follow the prefix--name convention.  And the implementation is much
 simpler overall because S-expressions are much easier to parse.
 ---
  emacs/Makefile.local|1 +
  emacs/notmuch-parser.el |  212 
 +++
  2 files changed, 213 insertions(+)
  create mode 100644 emacs/notmuch-parser.el

 diff --git a/emacs/Makefile.local b/emacs/Makefile.local
 index 456700a..a910aff 100644
 --- a/emacs/Makefile.local
 +++ b/emacs/Makefile.local
 @@ -3,6 +3,7 @@
  dir := emacs
  emacs_sources := \
   $(dir)/notmuch-lib.el \
 + $(dir)/notmuch-parser.el \
   $(dir)/notmuch.el \
   $(dir)/notmuch-query.el \
   $(dir)/notmuch-show.el \
 diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
 new file mode 100644
 index 000..1b7cf64
 --- /dev/null
 +++ b/emacs/notmuch-parser.el
 @@ -0,0 +1,212 @@
 +;; notmuch-parser.el --- streaming S-expression parser
 +;;
 +;; Copyright © Austin Clements
 +;;
 +;; This file is part of Notmuch.
 +;;
 +;; Notmuch is free software: you can redistribute it and/or modify it
 +;; under the terms of the GNU General Public License as published by
 +;; the Free Software Foundation, either version 3 of the License, or
 +;; (at your option) any later version.
 +;;
 +;; Notmuch is distributed in the hope that it will be useful, but
 +;; WITHOUT ANY WARRANTY; without even the implied warranty of
 +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 +;; General Public License for more details.
 +;;
 +;; You should have received a copy of the GNU General Public License
 +;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
 +;;
 +;; Authors: Austin Clements acleme...@csail.mit.edu
 +
 +(require 'cl)
 +
 +(defun notmuch-sexp-create-parser (buffer)
 +  Return a streaming S-expression parser that reads from BUFFER.
 +
 +This parser is designed to incrementally read an S-expression
 +whose structure is known to the caller.  Like a typical
 +S-expression parsing interface, it provides a function to read a
 +complete S-expression from the input.  However, it extends this
 +with an additional function that requires the next value in the
 +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.
 +
 +  (vector 'notmuch-sexp-parser
 +   buffer
 +   ;; List depth
 +   0
 +   ;; Partial parse position marker
 +   nil
 +   ;; 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))

Why the double hyphen --? Is it a name-space or some convention?


 +(defun notmuch-sexp-read (sp)
 +  Consume and return the value at point in SP's buffer.
 +
 +Returns 'retry if there is insufficient input to parse a complete
 +value (though it may still move point over whitespace).  If the
 +parser is currently inside a list and the next token ends the
 +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.
 +(read (current-buffer))

Why is good for read to signal the error rather than us doing it?

 +  ;; Go up a level and return an end token
 +  (decf (notmuch-sexp--depth sp))
 +  (forward-char)
 +  'end))
 +   ((= (char-after) ?\()
 +;; We're at the 

Re: [PATCH 4/5] emacs: Streaming S-expression parser

2013-05-21 Thread Austin Clements
On Tue, 21 May 2013, Mark Walters markwalters1...@gmail.com wrote:
 Hi

 This patch looks good to me. Some minor comments below.

Some minor replies below.

In building some other code on top of this, I found an interesting (but
easy to fix) interface bug.  Currently, the interface is designed as if
it doesn't matter what buffer these functions are called from, however,
because they move point and expect this point motion to persist, it's
actually not safe to call this interface unless the caller is in the
right buffer anyway.  For example, if the buffer is selected in a
window, the with-current-buffer in the parser functions will actually
move a *temporary* point, meaning that the only way the caller can
discover the new point is to first select the buffer for itself.  I can
think of two solutions: 1) maintain our own mark for the parser's
current position or 2) tweak the doc strings and code so that it reads
from the current buffer.  1 keeps the interface the way it's currently
documented, but complicates the parser implementation and interface and
doesn't simplify the caller.  2 simplifies the parser and it turns out
all callers already satisfy the requirement.

 Best wishes

 Mark


 On Sat, 18 May 2013, Austin Clements amdra...@mit.edu wrote:
 This provides the same interface as the streaming JSON parser, but
 reads S-expressions incrementally.  The only difference is that the
 `notmuch-sexp-parse-partial-list' helper does not handle interleaved
 error messages (since we now have the ability to separate these out at
 the invocation level), so it no longer takes an error function and
 does not need to do the horrible resynchronization that the JSON
 parser had to.

 Some implementation improvements have been made over the JSON parser.
 This uses a vector instead of a list for the parser data structure,
 since this allows faster access to elements (and modern versions of
 Emacs handle storage of small vectors efficiently).  Private functions
 follow the prefix--name convention.  And the implementation is much
 simpler overall because S-expressions are much easier to parse.
 ---
  emacs/Makefile.local|1 +
  emacs/notmuch-parser.el |  212 
 +++
  2 files changed, 213 insertions(+)
  create mode 100644 emacs/notmuch-parser.el

 diff --git a/emacs/Makefile.local b/emacs/Makefile.local
 index 456700a..a910aff 100644
 --- a/emacs/Makefile.local
 +++ b/emacs/Makefile.local
 @@ -3,6 +3,7 @@
  dir := emacs
  emacs_sources := \
  $(dir)/notmuch-lib.el \
 +$(dir)/notmuch-parser.el \
  $(dir)/notmuch.el \
  $(dir)/notmuch-query.el \
  $(dir)/notmuch-show.el \
 diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
 new file mode 100644
 index 000..1b7cf64
 --- /dev/null
 +++ b/emacs/notmuch-parser.el
 @@ -0,0 +1,212 @@
 +;; notmuch-parser.el --- streaming S-expression parser
 +;;
 +;; Copyright © Austin Clements
 +;;
 +;; This file is part of Notmuch.
 +;;
 +;; Notmuch is free software: you can redistribute it and/or modify it
 +;; under the terms of the GNU General Public License as published by
 +;; the Free Software Foundation, either version 3 of the License, or
 +;; (at your option) any later version.
 +;;
 +;; Notmuch is distributed in the hope that it will be useful, but
 +;; WITHOUT ANY WARRANTY; without even the implied warranty of
 +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 +;; General Public License for more details.
 +;;
 +;; You should have received a copy of the GNU General Public License
 +;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
 +;;
 +;; Authors: Austin Clements acleme...@csail.mit.edu
 +
 +(require 'cl)
 +
 +(defun notmuch-sexp-create-parser (buffer)
 +  Return a streaming S-expression parser that reads from BUFFER.
 +
 +This parser is designed to incrementally read an S-expression
 +whose structure is known to the caller.  Like a typical
 +S-expression parsing interface, it provides a function to read a
 +complete S-expression from the input.  However, it extends this
 +with an additional function that requires the next value in the
 +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.
 +
 +  (vector 'notmuch-sexp-parser
 +  buffer
 +  ;; List depth
 +  0
 +  ;; Partial parse position marker
 +  nil
 +  ;; 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))

 Why the double hyphen --? Is it a 

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-18 Thread Austin Clements
This provides the same interface as the streaming JSON parser, but
reads S-expressions incrementally.  The only difference is that the
`notmuch-sexp-parse-partial-list' helper does not handle interleaved
error messages (since we now have the ability to separate these out at
the invocation level), so it no longer takes an error function and
does not need to do the horrible resynchronization that the JSON
parser had to.

Some implementation improvements have been made over the JSON parser.
This uses a vector instead of a list for the parser data structure,
since this allows faster access to elements (and modern versions of
Emacs handle storage of small vectors efficiently).  Private functions
follow the "prefix--name" convention.  And the implementation is much
simpler overall because S-expressions are much easier to parse.
---
 emacs/Makefile.local|1 +
 emacs/notmuch-parser.el |  212 +++
 2 files changed, 213 insertions(+)
 create mode 100644 emacs/notmuch-parser.el

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 456700a..a910aff 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -3,6 +3,7 @@
 dir := emacs
 emacs_sources := \
$(dir)/notmuch-lib.el \
+   $(dir)/notmuch-parser.el \
$(dir)/notmuch.el \
$(dir)/notmuch-query.el \
$(dir)/notmuch-show.el \
diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
new file mode 100644
index 000..1b7cf64
--- /dev/null
+++ b/emacs/notmuch-parser.el
@@ -0,0 +1,212 @@
+;; notmuch-parser.el --- streaming S-expression parser
+;;
+;; Copyright ? Austin Clements
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see .
+;;
+;; Authors: Austin Clements 
+
+(require 'cl)
+
+(defun notmuch-sexp-create-parser (buffer)
+  "Return a streaming S-expression parser that reads from BUFFER.
+
+This parser is designed to incrementally read an S-expression
+whose structure is known to the caller.  Like a typical
+S-expression parsing interface, it provides a function to read a
+complete S-expression from the input.  However, it extends this
+with an additional function that requires the next value in the
+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."
+
+  (vector 'notmuch-sexp-parser
+ buffer
+ ;; List depth
+ 0
+ ;; Partial parse position marker
+ nil
+ ;; 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))
+
+(defun notmuch-sexp-read (sp)
+  "Consume and return the value at point in SP's buffer.
+
+Returns 'retry if there is insufficient input to parse a complete
+value (though it may still move point over whitespace).  If the
+parser is currently inside a list and the next token ends the
+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.
+  (read (current-buffer))
+;; Go up a level and return an end token
+(decf (notmuch-sexp--depth sp))
+(forward-char)
+'end))
+ ((= (char-after) ?\()
+  ;; We're at the beginning of a list.  If we haven't started
+  ;; a partial parse yet, attempt to read the list in its
+  ;; entirety.  If this fails, or we've started a partial
+  ;; parse, extend the partial parse to figure out when we
+  ;; have a complete list.
+  (catch 'return
+(when (null 

[PATCH 4/5] emacs: Streaming S-expression parser

2013-05-17 Thread Austin Clements
This provides the same interface as the streaming JSON parser, but
reads S-expressions incrementally.  The only difference is that the
`notmuch-sexp-parse-partial-list' helper does not handle interleaved
error messages (since we now have the ability to separate these out at
the invocation level), so it no longer takes an error function and
does not need to do the horrible resynchronization that the JSON
parser had to.

Some implementation improvements have been made over the JSON parser.
This uses a vector instead of a list for the parser data structure,
since this allows faster access to elements (and modern versions of
Emacs handle storage of small vectors efficiently).  Private functions
follow the prefix--name convention.  And the implementation is much
simpler overall because S-expressions are much easier to parse.
---
 emacs/Makefile.local|1 +
 emacs/notmuch-parser.el |  212 +++
 2 files changed, 213 insertions(+)
 create mode 100644 emacs/notmuch-parser.el

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 456700a..a910aff 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -3,6 +3,7 @@
 dir := emacs
 emacs_sources := \
$(dir)/notmuch-lib.el \
+   $(dir)/notmuch-parser.el \
$(dir)/notmuch.el \
$(dir)/notmuch-query.el \
$(dir)/notmuch-show.el \
diff --git a/emacs/notmuch-parser.el b/emacs/notmuch-parser.el
new file mode 100644
index 000..1b7cf64
--- /dev/null
+++ b/emacs/notmuch-parser.el
@@ -0,0 +1,212 @@
+;; notmuch-parser.el --- streaming S-expression parser
+;;
+;; Copyright © Austin Clements
+;;
+;; This file is part of Notmuch.
+;;
+;; Notmuch is free software: you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+;;
+;; Notmuch is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with Notmuch.  If not, see http://www.gnu.org/licenses/.
+;;
+;; Authors: Austin Clements acleme...@csail.mit.edu
+
+(require 'cl)
+
+(defun notmuch-sexp-create-parser (buffer)
+  Return a streaming S-expression parser that reads from BUFFER.
+
+This parser is designed to incrementally read an S-expression
+whose structure is known to the caller.  Like a typical
+S-expression parsing interface, it provides a function to read a
+complete S-expression from the input.  However, it extends this
+with an additional function that requires the next value in the
+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.
+
+  (vector 'notmuch-sexp-parser
+ buffer
+ ;; List depth
+ 0
+ ;; Partial parse position marker
+ nil
+ ;; 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))
+
+(defun notmuch-sexp-read (sp)
+  Consume and return the value at point in SP's buffer.
+
+Returns 'retry if there is insufficient input to parse a complete
+value (though it may still move point over whitespace).  If the
+parser is currently inside a list and the next token ends the
+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.
+  (read (current-buffer))
+;; Go up a level and return an end token
+(decf (notmuch-sexp--depth sp))
+(forward-char)
+'end))
+ ((= (char-after) ?\()
+  ;; We're at the beginning of a list.  If we haven't started
+  ;; a partial parse yet, attempt to read the list in its
+  ;; entirety.  If this fails, or we've started a partial
+  ;; parse, extend the partial parse to figure out when we
+  ;; have a complete list.
+  (catch 'return
+(when (null