| Message ID | 1489385927-6735-26-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi On Mon, Mar 13, 2017 at 10:31 AM Markus Armbruster <armbru@redhat.com> wrote: > The recent merge of docs/qmp-commands.txt and docs/qmp-events.txt into > the schema lost type information. Fix this documentation regression. > > Example change (qemu-qmp-ref.txt): > > -- Struct: InputKeyEvent > > Keyboard input event. > > Members: > - 'button' > + 'button: InputButton' > Which button this event is for. > - 'down' > + 'down: boolean' > True for key-down and false for key-up events. > > Since: 2.0 > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi.py | 14 ++++++++++++++ > scripts/qapi2texi.py | 8 ++++++-- > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 9430493..b82a2a6 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -1101,6 +1101,11 @@ class QAPISchemaType(QAPISchemaEntity): > } > return json2qtype.get(self.json_type()) > > + def doc_type(self): > + if self.is_implicit(): > + return None > + return self.name > + > > class QAPISchemaBuiltinType(QAPISchemaType): > def __init__(self, name, json_type, c_type): > @@ -1125,6 +1130,9 @@ class QAPISchemaBuiltinType(QAPISchemaType): > def json_type(self): > return self._json_type_name > > + def doc_type(self): > + return self.json_type() > + > def visit(self, visitor): > visitor.visit_builtin_type(self.name, self.info, > self.json_type()) > > @@ -1184,6 +1192,12 @@ class QAPISchemaArrayType(QAPISchemaType): > def json_type(self): > return 'array' > > + def doc_type(self): > + elt_doc_type = self.element_type.doc_type() > + if not elt_doc_type: > + return None > In which case is this expected to happen? place an assert here instead? Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > + return 'array of ' + elt_doc_type > + > def visit(self, visitor): > visitor.visit_array_type(self.name, self.info, self.element_type) > > diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py > index 3dd0146..993b652 100755 > --- a/scripts/qapi2texi.py > +++ b/scripts/qapi2texi.py > @@ -135,8 +135,12 @@ def texi_enum_value(value): > > def texi_member(member): > """Format a table of members item for an object type member""" > - return '@item @code{%s}%s\n' % ( > - member.name, ' (optional)' if member.optional else '') > + typ = member.type.doc_type() > + return '@item @code{%s%s%s}%s\n' % ( > + member.name, > + ': ' if typ else '', > + typ if typ else '', > + ' (optional)' if member.optional else '') > > > def texi_members(doc, what, member_func): > -- > 2.7.4 > > > -- Marc-André Lureau
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Mon, Mar 13, 2017 at 10:31 AM Markus Armbruster <armbru@redhat.com> > wrote: > >> The recent merge of docs/qmp-commands.txt and docs/qmp-events.txt into >> the schema lost type information. Fix this documentation regression. >> >> Example change (qemu-qmp-ref.txt): >> >> -- Struct: InputKeyEvent >> >> Keyboard input event. >> >> Members: >> - 'button' >> + 'button: InputButton' >> Which button this event is for. >> - 'down' >> + 'down: boolean' >> True for key-down and false for key-up events. >> >> Since: 2.0 >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi.py | 14 ++++++++++++++ >> scripts/qapi2texi.py | 8 ++++++-- >> 2 files changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index 9430493..b82a2a6 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -1101,6 +1101,11 @@ class QAPISchemaType(QAPISchemaEntity): >> } >> return json2qtype.get(self.json_type()) >> >> + def doc_type(self): >> + if self.is_implicit(): >> + return None >> + return self.name >> + >> >> class QAPISchemaBuiltinType(QAPISchemaType): >> def __init__(self, name, json_type, c_type): >> @@ -1125,6 +1130,9 @@ class QAPISchemaBuiltinType(QAPISchemaType): >> def json_type(self): >> return self._json_type_name >> >> + def doc_type(self): >> + return self.json_type() >> + >> def visit(self, visitor): >> visitor.visit_builtin_type(self.name, self.info, >> self.json_type()) >> >> @@ -1184,6 +1192,12 @@ class QAPISchemaArrayType(QAPISchemaType): >> def json_type(self): >> return 'array' >> >> + def doc_type(self): >> + elt_doc_type = self.element_type.doc_type() >> + if not elt_doc_type: >> + return None >> > > In which case is this expected to happen? place an assert here instead? I think assert should work now, the argument is a bit longwinded, and it won't let us simplify code. First the argument. T.doc_type() returns None for a non-array type T when T.is_implicit() and T isn't a built-in type, because: * QAPISchemaType.doc_type() returns None when self.is_implicit(), but * QAPISchemaBuiltinType().doc_type() overrides, and never returns None. T.is_implicit() is true for the following non-array, non-builtin T: * the implicitly defined enumeration type of a simple union's tag * the implicitly defined variant type of a simple union * an implicitly defined base type of a union * an implicitly defined argument type of a command or event We can't actually make arrays of these. Now let's see whether we can use it to simplify code. There are four calls of .doc_type() besides the one shown above: * texi_member() calls it for types of - the (common) members of object types (including command and event arguments) and members of alternate types. None happens for the implicitly defined tag member of simple unions. The easiest way to cope with it is to cope with None for any member. - the members of variant members of simple unions. It doesn't bother to handle None, because a member type can only be a built-in or explictly defined type, or an array thereof. * texi_members() calls it and checks for None to help identify undocumented members where we can do better than say "Not documented". * texi_members() calls it for the base type, except when it's implicitly defined. It doesn't bother to handle None, because it can only be an explictly defined struct type. * texi_members() calls it for the types of variant members of flat unions. It doesn't bother to handle None, because a member type can only be a built-in or explictly defined type. Adding the assertion makes no case of None go away. > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Thanks!
On 03/13/2017 01:18 AM, Markus Armbruster wrote: > The recent merge of docs/qmp-commands.txt and docs/qmp-events.txt into > the schema lost type information. Fix this documentation regression. > > Example change (qemu-qmp-ref.txt): > > -- Struct: InputKeyEvent > > Keyboard input event. > > Members: > - 'button' > + 'button: InputButton' > Which button this event is for. > - 'down' > + 'down: boolean' > True for key-down and false for key-up events. > > Since: 2.0 Definitely worth having. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi.py | 14 ++++++++++++++ > scripts/qapi2texi.py | 8 ++++++-- > 2 files changed, 20 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/scripts/qapi.py b/scripts/qapi.py index 9430493..b82a2a6 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1101,6 +1101,11 @@ class QAPISchemaType(QAPISchemaEntity): } return json2qtype.get(self.json_type()) + def doc_type(self): + if self.is_implicit(): + return None + return self.name + class QAPISchemaBuiltinType(QAPISchemaType): def __init__(self, name, json_type, c_type): @@ -1125,6 +1130,9 @@ class QAPISchemaBuiltinType(QAPISchemaType): def json_type(self): return self._json_type_name + def doc_type(self): + return self.json_type() + def visit(self, visitor): visitor.visit_builtin_type(self.name, self.info, self.json_type()) @@ -1184,6 +1192,12 @@ class QAPISchemaArrayType(QAPISchemaType): def json_type(self): return 'array' + def doc_type(self): + elt_doc_type = self.element_type.doc_type() + if not elt_doc_type: + return None + return 'array of ' + elt_doc_type + def visit(self, visitor): visitor.visit_array_type(self.name, self.info, self.element_type) diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py index 3dd0146..993b652 100755 --- a/scripts/qapi2texi.py +++ b/scripts/qapi2texi.py @@ -135,8 +135,12 @@ def texi_enum_value(value): def texi_member(member): """Format a table of members item for an object type member""" - return '@item @code{%s}%s\n' % ( - member.name, ' (optional)' if member.optional else '') + typ = member.type.doc_type() + return '@item @code{%s%s%s}%s\n' % ( + member.name, + ': ' if typ else '', + typ if typ else '', + ' (optional)' if member.optional else '') def texi_members(doc, what, member_func):
The recent merge of docs/qmp-commands.txt and docs/qmp-events.txt into the schema lost type information. Fix this documentation regression. Example change (qemu-qmp-ref.txt): -- Struct: InputKeyEvent Keyboard input event. Members: - 'button' + 'button: InputButton' Which button this event is for. - 'down' + 'down: boolean' True for key-down and false for key-up events. Since: 2.0 Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi.py | 14 ++++++++++++++ scripts/qapi2texi.py | 8 ++++++-- 2 files changed, 20 insertions(+), 2 deletions(-)