Eric Blake <ebl...@redhat.com> writes:
> On 07/26/2013 08:16 AM, Markus Armbruster wrote:
>> Eric Blake <ebl...@redhat.com> writes:
>>
>>> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>>>> The parser handles erroneous input badly. To be improved shortly.
>>>>
>>>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>>>> ---
>>>
>>> Lots of proof on how bad it is! I'd also like to see a couple tests on
>>> trailing commas:
>>>
>>> { 'enum': 'Foo', [ 'bar' ], }
>>> { 'enum': 'Gur', [ 'ble', ] }
>>
>> I figure you mean
>>
>> { 'enum': 'Foo', 'data': [ 'bar' ], }
>> { 'enum': 'Gur', 'data': [ 'ble', ] }
>
> Yep, you got my intent, even if I didn't type it right.
>
>>
>> My parser rejects both:
>>
>> <stdin>:1:37: Expected string
>> <stdin>:2:35: Expected "{", "[" or string
>
> Good!
>
>>
>> I commented out the first to get the second error. Making the parser
>> continue after errors didn't seem to be worthwhile.
>
> Agree about not continuing after errors; once the file is clean, anyone
> adding a new command will have errors in at most the one command they
> are trying to add, and even if it is an iterative approach to get them
> to find all the problems, it's a lot easier to have that one developer
> fix their work than trying to bake error recovery into the parser.
>
> If you do add these two test cases (which I recommend), it should indeed
> be two separate tests.
Could be done on top. If I need to respin anyway, I'll add them in the
first patch, of course.
> Another thought - is it worth testing other valid JSON but invalid
> schema constructs, such as:
>
> { 'enum': 1, 'data': false }
Maybe, but I limited myself just to the parser in this series.
