diff mbox

[v5,17/28] qapi: Allow true, false and null in schema json

Message ID 1427227433-5030-18-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake March 24, 2015, 8:03 p.m. UTC
From: Fam Zheng <famz@redhat.com>

In the near term, we will use it for a sensible-looking
'gen':false inside command declarations, instead of the
current ugly 'gen':'no'.

In the long term, it will allow conversion from shorthand
with defaults mentioned only in side-band documentation:
 'data':{'*flag':'bool', '*string':'str'}
into an explicit default value documentation, as in:
 'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
         'string':{'type':'str', 'optional':true, 'default':null}}

We still don't parse integer values (also necessary before
we can allow explicit defaults), but that can come in a later
series.

Update the testsuite to match an improved error message.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py                      | 21 ++++++++++++++++++---
 tests/qapi-schema/bad-type-bool.err  |  2 +-
 tests/qapi-schema/bad-type-bool.json |  1 -
 3 files changed, 19 insertions(+), 5 deletions(-)

Comments

Markus Armbruster March 26, 2015, 5:32 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> From: Fam Zheng <famz@redhat.com>
>
> In the near term, we will use it for a sensible-looking
> 'gen':false inside command declarations, instead of the
> current ugly 'gen':'no'.
>
> In the long term, it will allow conversion from shorthand
> with defaults mentioned only in side-band documentation:
>  'data':{'*flag':'bool', '*string':'str'}
> into an explicit default value documentation, as in:
>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>          'string':{'type':'str', 'optional':true, 'default':null}}
>
> We still don't parse integer values (also necessary before
> we can allow explicit defaults), but that can come in a later
> series.
>
> Update the testsuite to match an improved error message.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  scripts/qapi.py                      | 21 ++++++++++++++++++---
>  tests/qapi-schema/bad-type-bool.err  |  2 +-
>  tests/qapi-schema/bad-type-bool.json |  1 -
>  3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 5d0dc91..6ed6a34 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -158,6 +158,20 @@ class QAPISchema:
>                          return
>                      else:
>                          string += ch
> +            elif self.tok in "tfn":
> +                val = self.src[self.cursor - 1:]
> +                if val.startswith("true"):
> +                    self.val = True
> +                    self.cursor += 3
> +                    return
> +                elif val.startswith("false"):
> +                    self.val = False
> +                    self.cursor += 4
> +                    return
> +                elif val.startswith("null"):
> +                    self.val = None
> +                    self.cursor += 3
> +                    return
>              elif self.tok == '\n':
>                  if self.cursor == len(self.src):
>                      self.tok = None
> @@ -197,8 +211,9 @@ class QAPISchema:
>          if self.tok == ']':
>              self.accept()
>              return expr
> -        if not self.tok in [ '{', '[', "'" ]:
> -            raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
> +        if not self.tok in "{['tfn":
> +            raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
> +                                  'boolean or "null"')
>          while True:
>              expr.append(self.get_expr(True))
>              if self.tok == ']':
> @@ -217,7 +232,7 @@ class QAPISchema:
>          elif self.tok == '[':
>              self.accept()
>              expr = self.get_values()
> -        elif self.tok == "'":
> +        elif self.tok in "'tfn":
>              expr = self.val
>              self.accept()
>          else:

Exploiting that the three literal names start with different letters is
a a hack, but it'll do.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[...]
Kevin Wolf March 31, 2015, 3:23 p.m. UTC | #2
Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
> From: Fam Zheng <famz@redhat.com>
> 
> In the near term, we will use it for a sensible-looking
> 'gen':false inside command declarations, instead of the
> current ugly 'gen':'no'.
> 
> In the long term, it will allow conversion from shorthand
> with defaults mentioned only in side-band documentation:
>  'data':{'*flag':'bool', '*string':'str'}
> into an explicit default value documentation, as in:
>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>          'string':{'type':'str', 'optional':true, 'default':null}}

FWIW, I don't think that's a very friendly syntax for humans, it's a bit
verbose. But that's no reason not to allow true/false/null, of course.

> We still don't parse integer values (also necessary before
> we can allow explicit defaults), but that can come in a later
> series.
> 
> Update the testsuite to match an improved error message.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Kevin
Markus Armbruster March 31, 2015, 8:09 p.m. UTC | #3
Kevin Wolf <kwolf@redhat.com> writes:

> Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
>> From: Fam Zheng <famz@redhat.com>
>> 
>> In the near term, we will use it for a sensible-looking
>> 'gen':false inside command declarations, instead of the
>> current ugly 'gen':'no'.
>> 
>> In the long term, it will allow conversion from shorthand
>> with defaults mentioned only in side-band documentation:
>>  'data':{'*flag':'bool', '*string':'str'}
>> into an explicit default value documentation, as in:
>>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>>          'string':{'type':'str', 'optional':true, 'default':null}}
>
> FWIW, I don't think that's a very friendly syntax for humans, it's a bit
> verbose. But that's no reason not to allow true/false/null, of course.

Here's my current thinking.

Longhand:

    # mandatory
    'name': { 'type': 'str' }
    # optional, with a default
    'flag': { 'type': 'bool', 'default': true }
    # optional, no default
    'string': { 'type': 'str', 'default': null }

Presence of 'default' implies optional.

Equivalent shorthand, if any:

    'name': 'str'
    '*string': 'str'

[...]
Kevin Wolf April 1, 2015, 8:31 a.m. UTC | #4
Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
> >> From: Fam Zheng <famz@redhat.com>
> >> 
> >> In the near term, we will use it for a sensible-looking
> >> 'gen':false inside command declarations, instead of the
> >> current ugly 'gen':'no'.
> >> 
> >> In the long term, it will allow conversion from shorthand
> >> with defaults mentioned only in side-band documentation:
> >>  'data':{'*flag':'bool', '*string':'str'}
> >> into an explicit default value documentation, as in:
> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
> >>          'string':{'type':'str', 'optional':true, 'default':null}}
> >
> > FWIW, I don't think that's a very friendly syntax for humans, it's a bit
> > verbose. But that's no reason not to allow true/false/null, of course.
> 
> Here's my current thinking.
> 
> Longhand:
> 
>     # mandatory
>     'name': { 'type': 'str' }
>     # optional, with a default
>     'flag': { 'type': 'bool', 'default': true }
>     # optional, no default
>     'string': { 'type': 'str', 'default': null }
> 
> Presence of 'default' implies optional.
> 
> Equivalent shorthand, if any:
> 
>     'name': 'str'
>     '*string': 'str'

A nice shorthand for defaults would be:

    '*name': 'str' = 'default'

Though that would be neither valid JSON nor Python any more. Do we
actually rely on this property anywhere or is it only parsed by the QAPI
generator anyway and we can extend the language in such ways?

Kevin
Markus Armbruster April 1, 2015, 9:33 a.m. UTC | #5
Kevin Wolf <kwolf@redhat.com> writes:

> Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
>> >> From: Fam Zheng <famz@redhat.com>
>> >> 
>> >> In the near term, we will use it for a sensible-looking
>> >> 'gen':false inside command declarations, instead of the
>> >> current ugly 'gen':'no'.
>> >> 
>> >> In the long term, it will allow conversion from shorthand
>> >> with defaults mentioned only in side-band documentation:
>> >>  'data':{'*flag':'bool', '*string':'str'}
>> >> into an explicit default value documentation, as in:
>> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>> >>          'string':{'type':'str', 'optional':true, 'default':null}}
>> >
>> > FWIW, I don't think that's a very friendly syntax for humans, it's a bit
>> > verbose. But that's no reason not to allow true/false/null, of course.
>> 
>> Here's my current thinking.
>> 
>> Longhand:
>> 
>>     # mandatory
>>     'name': { 'type': 'str' }
>>     # optional, with a default
>>     'flag': { 'type': 'bool', 'default': true }
>>     # optional, no default
>>     'string': { 'type': 'str', 'default': null }
>> 
>> Presence of 'default' implies optional.
>> 
>> Equivalent shorthand, if any:
>> 
>>     'name': 'str'
>>     '*string': 'str'
>
> A nice shorthand for defaults would be:
>
>     '*name': 'str' = 'default'
>
> Though that would be neither valid JSON nor Python any more. Do we
> actually rely on this property anywhere or is it only parsed by the QAPI
> generator anyway and we can extend the language in such ways?

I guess JSON / Python was chosen as QAPI schema language to save us the
bother of defining a syntax and building the tools to work with it, like
an Emacs mode.  JSON's not exactly my favourite choice, but at least
it's not XML.

What we have now isn't JSON, but it's still a subset of Python, and the
Python tools work.  If we go beyond Python, they'll break.

If we decide to sacrifice these tools for readability, then we can just
as well replace the syntax entirely.  Preferably by something where I
don't have to put every identifier in quotes.

In short, you're welcome to hack up qapi.py some more for schema
readability, but either keep Emacs Python mode working, or provide a
replacement :)
Kevin Wolf April 1, 2015, 9:58 a.m. UTC | #6
Am 01.04.2015 um 11:33 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
> >> Kevin Wolf <kwolf@redhat.com> writes:
> >> 
> >> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
> >> >> From: Fam Zheng <famz@redhat.com>
> >> >> 
> >> >> In the near term, we will use it for a sensible-looking
> >> >> 'gen':false inside command declarations, instead of the
> >> >> current ugly 'gen':'no'.
> >> >> 
> >> >> In the long term, it will allow conversion from shorthand
> >> >> with defaults mentioned only in side-band documentation:
> >> >>  'data':{'*flag':'bool', '*string':'str'}
> >> >> into an explicit default value documentation, as in:
> >> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
> >> >>          'string':{'type':'str', 'optional':true, 'default':null}}
> >> >
> >> > FWIW, I don't think that's a very friendly syntax for humans, it's a bit
> >> > verbose. But that's no reason not to allow true/false/null, of course.
> >> 
> >> Here's my current thinking.
> >> 
> >> Longhand:
> >> 
> >>     # mandatory
> >>     'name': { 'type': 'str' }
> >>     # optional, with a default
> >>     'flag': { 'type': 'bool', 'default': true }
> >>     # optional, no default
> >>     'string': { 'type': 'str', 'default': null }
> >> 
> >> Presence of 'default' implies optional.
> >> 
> >> Equivalent shorthand, if any:
> >> 
> >>     'name': 'str'
> >>     '*string': 'str'
> >
> > A nice shorthand for defaults would be:
> >
> >     '*name': 'str' = 'default'
> >
> > Though that would be neither valid JSON nor Python any more. Do we
> > actually rely on this property anywhere or is it only parsed by the QAPI
> > generator anyway and we can extend the language in such ways?
> 
> I guess JSON / Python was chosen as QAPI schema language to save us the
> bother of defining a syntax and building the tools to work with it, like
> an Emacs mode.  JSON's not exactly my favourite choice, but at least
> it's not XML.
> 
> What we have now isn't JSON, but it's still a subset of Python, and the
> Python tools work.  If we go beyond Python, they'll break.
> 
> If we decide to sacrifice these tools for readability, then we can just
> as well replace the syntax entirely.  Preferably by something where I
> don't have to put every identifier in quotes.
> 
> In short, you're welcome to hack up qapi.py some more for schema
> readability, but either keep Emacs Python mode working, or provide a
> replacement :)

What features does this Python mode provide that you use?

For the JSON schema, the only thing I really use from the editor is
syntax highlighting, and that shouldn't be bothered by an addition like
this (in vim it doesn't hurt anyway). I also don't use any other Python
tools, though I'm not sure that none exist that would make sense with
the schema.

So if there are practically relevant advantages in being real Python
code, then we need to consider that, of course. That's why I was asking.
But if there aren't, it's just an arbitrary restriction that could be
removed.

Kevin
Markus Armbruster April 1, 2015, 11:03 a.m. UTC | #7
Kevin Wolf <kwolf@redhat.com> writes:

> Am 01.04.2015 um 11:33 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf <kwolf@redhat.com> writes:
>> >> 
>> >> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
>> >> >> From: Fam Zheng <famz@redhat.com>
>> >> >> 
>> >> >> In the near term, we will use it for a sensible-looking
>> >> >> 'gen':false inside command declarations, instead of the
>> >> >> current ugly 'gen':'no'.
>> >> >> 
>> >> >> In the long term, it will allow conversion from shorthand
>> >> >> with defaults mentioned only in side-band documentation:
>> >> >>  'data':{'*flag':'bool', '*string':'str'}
>> >> >> into an explicit default value documentation, as in:
>> >> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>> >> >>          'string':{'type':'str', 'optional':true, 'default':null}}
>> >> >
>> >> > FWIW, I don't think that's a very friendly syntax for humans, it's a bit
>> >> > verbose. But that's no reason not to allow true/false/null, of course.
>> >> 
>> >> Here's my current thinking.
>> >> 
>> >> Longhand:
>> >> 
>> >>     # mandatory
>> >>     'name': { 'type': 'str' }
>> >>     # optional, with a default
>> >>     'flag': { 'type': 'bool', 'default': true }
>> >>     # optional, no default
>> >>     'string': { 'type': 'str', 'default': null }
>> >> 
>> >> Presence of 'default' implies optional.
>> >> 
>> >> Equivalent shorthand, if any:
>> >> 
>> >>     'name': 'str'
>> >>     '*string': 'str'
>> >
>> > A nice shorthand for defaults would be:
>> >
>> >     '*name': 'str' = 'default'
>> >
>> > Though that would be neither valid JSON nor Python any more. Do we
>> > actually rely on this property anywhere or is it only parsed by the QAPI
>> > generator anyway and we can extend the language in such ways?
>> 
>> I guess JSON / Python was chosen as QAPI schema language to save us the
>> bother of defining a syntax and building the tools to work with it, like
>> an Emacs mode.  JSON's not exactly my favourite choice, but at least
>> it's not XML.
>> 
>> What we have now isn't JSON, but it's still a subset of Python, and the
>> Python tools work.  If we go beyond Python, they'll break.
>> 
>> If we decide to sacrifice these tools for readability, then we can just
>> as well replace the syntax entirely.  Preferably by something where I
>> don't have to put every identifier in quotes.
>> 
>> In short, you're welcome to hack up qapi.py some more for schema
>> readability, but either keep Emacs Python mode working, or provide a
>> replacement :)
>
> What features does this Python mode provide that you use?

Syntax highlighting, automatic indentation, possibly more I rely on
without noticing.  My fingers know, but they don't talk.

> For the JSON schema, the only thing I really use from the editor is
> syntax highlighting, and that shouldn't be bothered by an addition like
> this (in vim it doesn't hurt anyway). I also don't use any other Python
> tools, though I'm not sure that none exist that would make sense with
> the schema.
>
> So if there are practically relevant advantages in being real Python
> code, then we need to consider that, of course. That's why I was asking.
> But if there aren't, it's just an arbitrary restriction that could be
> removed.

If we decide to revise the decision to borrow existing syntax and roll
our own instead, let's 'make' 'our' 'own' 'syntax' 'not' 'suck'.

Anyway, we got bigger fish to fry right now.
Kevin Wolf April 1, 2015, 11:17 a.m. UTC | #8
Am 01.04.2015 um 13:03 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 01.04.2015 um 11:33 hat Markus Armbruster geschrieben:
> >> Kevin Wolf <kwolf@redhat.com> writes:
> >> 
> >> > Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
> >> >> Kevin Wolf <kwolf@redhat.com> writes:
> >> >> 
> >> >> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
> >> >> >> From: Fam Zheng <famz@redhat.com>
> >> >> >> 
> >> >> >> In the near term, we will use it for a sensible-looking
> >> >> >> 'gen':false inside command declarations, instead of the
> >> >> >> current ugly 'gen':'no'.
> >> >> >> 
> >> >> >> In the long term, it will allow conversion from shorthand
> >> >> >> with defaults mentioned only in side-band documentation:
> >> >> >>  'data':{'*flag':'bool', '*string':'str'}
> >> >> >> into an explicit default value documentation, as in:
> >> >> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
> >> >> >>          'string':{'type':'str', 'optional':true, 'default':null}}
> >> >> >
> >> >> > FWIW, I don't think that's a very friendly syntax for humans, it's a bit
> >> >> > verbose. But that's no reason not to allow true/false/null, of course.
> >> >> 
> >> >> Here's my current thinking.
> >> >> 
> >> >> Longhand:
> >> >> 
> >> >>     # mandatory
> >> >>     'name': { 'type': 'str' }
> >> >>     # optional, with a default
> >> >>     'flag': { 'type': 'bool', 'default': true }
> >> >>     # optional, no default
> >> >>     'string': { 'type': 'str', 'default': null }
> >> >> 
> >> >> Presence of 'default' implies optional.
> >> >> 
> >> >> Equivalent shorthand, if any:
> >> >> 
> >> >>     'name': 'str'
> >> >>     '*string': 'str'
> >> >
> >> > A nice shorthand for defaults would be:
> >> >
> >> >     '*name': 'str' = 'default'
> >> >
> >> > Though that would be neither valid JSON nor Python any more. Do we
> >> > actually rely on this property anywhere or is it only parsed by the QAPI
> >> > generator anyway and we can extend the language in such ways?
> >> 
> >> I guess JSON / Python was chosen as QAPI schema language to save us the
> >> bother of defining a syntax and building the tools to work with it, like
> >> an Emacs mode.  JSON's not exactly my favourite choice, but at least
> >> it's not XML.
> >> 
> >> What we have now isn't JSON, but it's still a subset of Python, and the
> >> Python tools work.  If we go beyond Python, they'll break.
> >> 
> >> If we decide to sacrifice these tools for readability, then we can just
> >> as well replace the syntax entirely.  Preferably by something where I
> >> don't have to put every identifier in quotes.
> >> 
> >> In short, you're welcome to hack up qapi.py some more for schema
> >> readability, but either keep Emacs Python mode working, or provide a
> >> replacement :)
> >
> > What features does this Python mode provide that you use?
> 
> Syntax highlighting, automatic indentation, possibly more I rely on
> without noticing.  My fingers know, but they don't talk.
> 
> > For the JSON schema, the only thing I really use from the editor is
> > syntax highlighting, and that shouldn't be bothered by an addition like
> > this (in vim it doesn't hurt anyway). I also don't use any other Python
> > tools, though I'm not sure that none exist that would make sense with
> > the schema.
> >
> > So if there are practically relevant advantages in being real Python
> > code, then we need to consider that, of course. That's why I was asking.
> > But if there aren't, it's just an arbitrary restriction that could be
> > removed.
> 
> If we decide to revise the decision to borrow existing syntax and roll
> our own instead, let's 'make' 'our' 'own' 'syntax' 'not' 'suck'.
> 
> Anyway, we got bigger fish to fry right now.

Okay, I got it. Asking me to create a completely new syntax and the
generator for it is the longhand version for "no".

Kevin
Eric Blake April 1, 2015, 12:17 p.m. UTC | #9
On 04/01/2015 03:33 AM, Markus Armbruster wrote:

>>> Longhand:
>>>
>>>     # mandatory
>>>     'name': { 'type': 'str' }
>>>     # optional, with a default
>>>     'flag': { 'type': 'bool', 'default': true }
>>>     # optional, no default
>>>     'string': { 'type': 'str', 'default': null }
>>>
>>> Presence of 'default' implies optional.
>>>
>>> Equivalent shorthand, if any:
>>>
>>>     'name': 'str'
>>>     '*string': 'str'
>>
>> A nice shorthand for defaults would be:
>>
>>     '*name': 'str' = 'default'
>>
>> Though that would be neither valid JSON nor Python any more. Do we
>> actually rely on this property anywhere or is it only parsed by the QAPI
>> generator anyway and we can extend the language in such ways?
> 
> I guess JSON / Python was chosen as QAPI schema language to save us the
> bother of defining a syntax and building the tools to work with it, like
> an Emacs mode.  JSON's not exactly my favourite choice, but at least
> it's not XML.
> 
> What we have now isn't JSON, but it's still a subset of Python, and the
> Python tools work.  If we go beyond Python, they'll break.

Well, we were a subset of Python, until this patch added true, false,
and null (the Python way would have been True, False, and None).  We are
also similar to JSON5, http://json5.org/

Among other things, JSON5 allows trailing commas, allows unquoted keys
in a dictionary, allows single-quoted strings, and allows C-style comments.

> 
> If we decide to sacrifice these tools for readability, then we can just
> as well replace the syntax entirely.  Preferably by something where I
> don't have to put every identifier in quotes.
> 
> In short, you're welcome to hack up qapi.py some more for schema
> readability, but either keep Emacs Python mode working, or provide a
> replacement :)

Since we're not quite python or JSON, we've already rolled our own
parser; so rewriting QAPI to use a syntax of our own choosing is not
that much of a leap.  But not for this series.
Markus Armbruster April 1, 2015, 2:51 p.m. UTC | #10
Kevin Wolf <kwolf@redhat.com> writes:

> Am 01.04.2015 um 13:03 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 01.04.2015 um 11:33 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf <kwolf@redhat.com> writes:
>> >> 
>> >> > Am 31.03.2015 um 22:09 hat Markus Armbruster geschrieben:
>> >> >> Kevin Wolf <kwolf@redhat.com> writes:
>> >> >> 
>> >> >> > Am 24.03.2015 um 21:03 hat Eric Blake geschrieben:
>> >> >> >> From: Fam Zheng <famz@redhat.com>
>> >> >> >> 
>> >> >> >> In the near term, we will use it for a sensible-looking
>> >> >> >> 'gen':false inside command declarations, instead of the
>> >> >> >> current ugly 'gen':'no'.
>> >> >> >> 
>> >> >> >> In the long term, it will allow conversion from shorthand
>> >> >> >> with defaults mentioned only in side-band documentation:
>> >> >> >>  'data':{'*flag':'bool', '*string':'str'}
>> >> >> >> into an explicit default value documentation, as in:
>> >> >> >>  'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
>> >> >> >>          'string':{'type':'str', 'optional':true, 'default':null}}
>> >> >> >
>> >> >> > FWIW, I don't think that's a very friendly syntax for
>> >> >> > humans, it's a bit
>> >> >> > verbose. But that's no reason not to allow true/false/null, of course.
>> >> >> 
>> >> >> Here's my current thinking.
>> >> >> 
>> >> >> Longhand:
>> >> >> 
>> >> >>     # mandatory
>> >> >>     'name': { 'type': 'str' }
>> >> >>     # optional, with a default
>> >> >>     'flag': { 'type': 'bool', 'default': true }
>> >> >>     # optional, no default
>> >> >>     'string': { 'type': 'str', 'default': null }
>> >> >> 
>> >> >> Presence of 'default' implies optional.
>> >> >> 
>> >> >> Equivalent shorthand, if any:
>> >> >> 
>> >> >>     'name': 'str'
>> >> >>     '*string': 'str'
>> >> >
>> >> > A nice shorthand for defaults would be:
>> >> >
>> >> >     '*name': 'str' = 'default'
>> >> >
>> >> > Though that would be neither valid JSON nor Python any more. Do we
>> >> > actually rely on this property anywhere or is it only parsed by the QAPI
>> >> > generator anyway and we can extend the language in such ways?
>> >> 
>> >> I guess JSON / Python was chosen as QAPI schema language to save us the
>> >> bother of defining a syntax and building the tools to work with it, like
>> >> an Emacs mode.  JSON's not exactly my favourite choice, but at least
>> >> it's not XML.
>> >> 
>> >> What we have now isn't JSON, but it's still a subset of Python, and the
>> >> Python tools work.  If we go beyond Python, they'll break.
>> >> 
>> >> If we decide to sacrifice these tools for readability, then we can just
>> >> as well replace the syntax entirely.  Preferably by something where I
>> >> don't have to put every identifier in quotes.
>> >> 
>> >> In short, you're welcome to hack up qapi.py some more for schema
>> >> readability, but either keep Emacs Python mode working, or provide a
>> >> replacement :)
>> >
>> > What features does this Python mode provide that you use?
>> 
>> Syntax highlighting, automatic indentation, possibly more I rely on
>> without noticing.  My fingers know, but they don't talk.
>> 
>> > For the JSON schema, the only thing I really use from the editor is
>> > syntax highlighting, and that shouldn't be bothered by an addition like
>> > this (in vim it doesn't hurt anyway). I also don't use any other Python
>> > tools, though I'm not sure that none exist that would make sense with
>> > the schema.
>> >
>> > So if there are practically relevant advantages in being real Python
>> > code, then we need to consider that, of course. That's why I was asking.
>> > But if there aren't, it's just an arbitrary restriction that could be
>> > removed.
>> 
>> If we decide to revise the decision to borrow existing syntax and roll
>> our own instead, let's 'make' 'our' 'own' 'syntax' 'not' 'suck'.
>> 
>> Anyway, we got bigger fish to fry right now.
>
> Okay, I got it. Asking me to create a completely new syntax and the
> generator for it is the longhand version for "no".

It's advice, not a proactive NAK.

Anyway, let's get Eric's series in, and crack the introspection problem
before we start even more QAPI generator projects.
Markus Armbruster April 1, 2015, 2:55 p.m. UTC | #11
Eric Blake <eblake@redhat.com> writes:

> On 04/01/2015 03:33 AM, Markus Armbruster wrote:
>
>>>> Longhand:
>>>>
>>>>     # mandatory
>>>>     'name': { 'type': 'str' }
>>>>     # optional, with a default
>>>>     'flag': { 'type': 'bool', 'default': true }
>>>>     # optional, no default
>>>>     'string': { 'type': 'str', 'default': null }
>>>>
>>>> Presence of 'default' implies optional.
>>>>
>>>> Equivalent shorthand, if any:
>>>>
>>>>     'name': 'str'
>>>>     '*string': 'str'
>>>
>>> A nice shorthand for defaults would be:
>>>
>>>     '*name': 'str' = 'default'
>>>
>>> Though that would be neither valid JSON nor Python any more. Do we
>>> actually rely on this property anywhere or is it only parsed by the QAPI
>>> generator anyway and we can extend the language in such ways?
>> 
>> I guess JSON / Python was chosen as QAPI schema language to save us the
>> bother of defining a syntax and building the tools to work with it, like
>> an Emacs mode.  JSON's not exactly my favourite choice, but at least
>> it's not XML.
>> 
>> What we have now isn't JSON, but it's still a subset of Python, and the
>> Python tools work.  If we go beyond Python, they'll break.
>
> Well, we were a subset of Python, until this patch added true, false,
> and null (the Python way would have been True, False, and None).  We are
> also similar to JSON5, http://json5.org/
>
> Among other things, JSON5 allows trailing commas, allows unquoted keys
> in a dictionary, allows single-quoted strings, and allows C-style comments.

I'm open to consider adopting it once the dust settles.

>> If we decide to sacrifice these tools for readability, then we can just
>> as well replace the syntax entirely.  Preferably by something where I
>> don't have to put every identifier in quotes.
>> 
>> In short, you're welcome to hack up qapi.py some more for schema
>> readability, but either keep Emacs Python mode working, or provide a
>> replacement :)
>
> Since we're not quite python or JSON, we've already rolled our own
> parser; so rewriting QAPI to use a syntax of our own choosing is not
> that much of a leap.

Point taken.

>                       But not for this series.

Certainly.
Eric Blake April 1, 2015, 3:43 p.m. UTC | #12
On 04/01/2015 06:17 AM, Eric Blake wrote:

>> I guess JSON / Python was chosen as QAPI schema language to save us the
>> bother of defining a syntax and building the tools to work with it, like
>> an Emacs mode.  JSON's not exactly my favourite choice, but at least
>> it's not XML.
>>
>> What we have now isn't JSON, but it's still a subset of Python, and the
>> Python tools work.  If we go beyond Python, they'll break.
> 
> Well, we were a subset of Python, until this patch added true, false,
> and null (the Python way would have been True, False, and None).  We are
> also similar to JSON5, http://json5.org/
> 
> Among other things, JSON5 allows trailing commas, allows unquoted keys
> in a dictionary, allows single-quoted strings, and allows C-style comments.

Another thing I just noticed: JSON allows '\u0061' as a synonym for the
one-byte string 'a', but our parser does not (instead, our parser treats
it like the five bytes 'u0061').  I guess we haven't noticed, since qapi
has never needed non-ascii names...

> Since we're not quite python or JSON, we've already rolled our own
> parser; so rewriting QAPI to use a syntax of our own choosing is not
> that much of a leap.  But not for this series.

So of course this still holds, and I won't stall my v6 posting because
of it.
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 5d0dc91..6ed6a34 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -158,6 +158,20 @@  class QAPISchema:
                         return
                     else:
                         string += ch
+            elif self.tok in "tfn":
+                val = self.src[self.cursor - 1:]
+                if val.startswith("true"):
+                    self.val = True
+                    self.cursor += 3
+                    return
+                elif val.startswith("false"):
+                    self.val = False
+                    self.cursor += 4
+                    return
+                elif val.startswith("null"):
+                    self.val = None
+                    self.cursor += 3
+                    return
             elif self.tok == '\n':
                 if self.cursor == len(self.src):
                     self.tok = None
@@ -197,8 +211,9 @@  class QAPISchema:
         if self.tok == ']':
             self.accept()
             return expr
-        if not self.tok in [ '{', '[', "'" ]:
-            raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
+        if not self.tok in "{['tfn":
+            raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
+                                  'boolean or "null"')
         while True:
             expr.append(self.get_expr(True))
             if self.tok == ']':
@@ -217,7 +232,7 @@  class QAPISchema:
         elif self.tok == '[':
             self.accept()
             expr = self.get_values()
-        elif self.tok == "'":
+        elif self.tok in "'tfn":
             expr = self.val
             self.accept()
         else:
diff --git a/tests/qapi-schema/bad-type-bool.err b/tests/qapi-schema/bad-type-bool.err
index badb7c2..de6168c 100644
--- a/tests/qapi-schema/bad-type-bool.err
+++ b/tests/qapi-schema/bad-type-bool.err
@@ -1 +1 @@ 
-tests/qapi-schema/bad-type-bool.json:3:11: Stray "t"
+tests/qapi-schema/bad-type-bool.json:2: 'type' key must have a string value
diff --git a/tests/qapi-schema/bad-type-bool.json b/tests/qapi-schema/bad-type-bool.json
index 22d6369..e1e9fb0 100644
--- a/tests/qapi-schema/bad-type-bool.json
+++ b/tests/qapi-schema/bad-type-bool.json
@@ -1,3 +1,2 @@ 
 # we reject an expression with a metatype that is not a string
-# FIXME: once the parser understands bool inputs, improve the error message
 { 'type': true, 'data': { } }