| Message ID | 20181208111606.8505-23-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Hi, | expand |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Use a common function to generate the "If:..." line. > > While at it, get rid of the existing \n\n (no idea why it was > there). Use a line-break in member description, this seems to look > slightly better in the plaintext version. Where exactly in the patch is this done? > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > scripts/qapi/doc.py | 24 +++++++++++++++--------- > tests/qapi-schema/doc-good.json | 4 +++- > tests/qapi-schema/doc-good.out | 1 + > tests/qapi-schema/doc-good.texi | 2 +- > 4 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py > index 76cb186ff9..2133ded47e 100755 > --- a/scripts/qapi/doc.py > +++ b/scripts/qapi/doc.py > @@ -126,19 +126,26 @@ def texi_body(doc): > return texi_format(doc.body.text) > > > -def texi_enum_value(value): > +def texi_if(ifcond, prefix='\n', suffix='\n'): > + """Format the #if condition""" > + return '%s@b{If:} @code{%s}%s' % ( > + prefix, ', '.join(ifcond), suffix) if ifcond else '' I like the ternary operator, but I nevertheless think this function is easier to read as if not ifcond: return '' return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix) > + > + > +def texi_enum_value(value, desc, suffix=''): > """Format a table of members item for an enumeration value""" > - return '@item @code{%s}\n' % value.name > + return '@item @code{%s}\n%s%s' % ( > + value.name, desc, texi_if(value.ifcond, prefix='@*')) Do you ignore suffix intentionally? > > > -def texi_member(member, suffix=''): > +def texi_member(member, desc='', suffix=''): > """Format a table of members item for an object type member""" > typ = member.type.doc_type() > membertype = ': ' + typ if typ else '' > - return '@item @code{%s%s}%s%s\n' % ( > + return '@item @code{%s%s}%s%s\n%s' % ( > member.name, membertype, > ' (optional)' if member.optional else '', > - suffix) > + suffix, desc) > > > def texi_members(doc, what, base, variants, member_func): > @@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func): > desc = 'One of ' + members_text + '\n' > else: > desc = 'Not documented\n' > - items += member_func(section.member) + desc > + items += member_func(section.member, desc) Here, you pass a @desc argument, but no @suffix. @member_func is either texi_enum_value() or texi_member(). Works. > if base: > items += '@item The members of @code{%s}\n' % base.doc_type() > if variants: > @@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func): > if v.type.is_implicit(): > assert not v.type.base and not v.type.variants > for m in v.type.local_members: > - items += member_func(m, when) > + items += member_func(m, suffix=when) Here, you pass a @suffix argument, but no @desc. texi_enum_value() would choke on that, but it can't occur here, because enums have no variants. Still, I'd prefer texi_enum_value() and texi_member() to have the exact same signature. > else: > items += '@item The members of @code{%s}%s\n' % ( > v.type.doc_type(), when) > @@ -185,8 +192,7 @@ def texi_sections(doc, ifcond): > body += texi_example(section.text) > else: > body += texi_format(section.text) > - if ifcond: > - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) > + body += texi_if(ifcond, suffix='') > return body > > > diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json > index 984cd8ed06..c7fe08c530 100644 > --- a/tests/qapi-schema/doc-good.json > +++ b/tests/qapi-schema/doc-good.json > @@ -55,7 +55,9 @@ > # > # @two is undocumented > ## > -{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' } > +{ 'enum': 'Enum', 'data': > + [ { 'name': 'one', 'if': 'defined(IFENUM)' }, 'two' ], > + 'if': 'defined(IFCOND)' } IFCOND applies to the enum, and IFENUM applies to one of its values. Awkward. Let's rename IFENUM to IFONE. > > ## > # @Base: > diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out > index c2fc5c774a..a05535b69b 100644 > --- a/tests/qapi-schema/doc-good.out > +++ b/tests/qapi-schema/doc-good.out > @@ -11,6 +11,7 @@ enum QType > module doc-good.json > enum Enum > member one > + if ['defined(IFENUM)'] > member two > if ['defined(IFCOND)'] > object Base > diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi > index e42eace474..5c0231e0e6 100644 > --- a/tests/qapi-schema/doc-good.texi > +++ b/tests/qapi-schema/doc-good.texi > @@ -84,12 +84,12 @@ Examples: > @table @asis > @item @code{one} > The @emph{one} @{and only@} > +@*@b{If:} @code{defined(IFENUM)} @* forces a line break. Putting it at the end of the previous line would be tidier, but let's ignore that for now. > @item @code{two} > Not documented > @end table > @code{two} is undocumented > > - > @b{If:} @code{defined(IFCOND)} > @end deftp
Hi On Tue, Dec 11, 2018 at 8:00 PM Markus Armbruster <armbru@redhat.com> wrote: > > Marc-André Lureau <marcandre.lureau@redhat.com> writes: > > > Use a common function to generate the "If:..." line. > > > > While at it, get rid of the existing \n\n (no idea why it was > > there). Use a line-break in member description, this seems to look > > slightly better in the plaintext version. > > Where exactly in the patch is this done? - if ifcond: - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) + body += texi_if(ifcond, suffix='') See also doc-good.texi output > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > scripts/qapi/doc.py | 24 +++++++++++++++--------- > > tests/qapi-schema/doc-good.json | 4 +++- > > tests/qapi-schema/doc-good.out | 1 + > > tests/qapi-schema/doc-good.texi | 2 +- > > 4 files changed, 20 insertions(+), 11 deletions(-) > > > > diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py > > index 76cb186ff9..2133ded47e 100755 > > --- a/scripts/qapi/doc.py > > +++ b/scripts/qapi/doc.py > > @@ -126,19 +126,26 @@ def texi_body(doc): > > return texi_format(doc.body.text) > > > > > > -def texi_enum_value(value): > > +def texi_if(ifcond, prefix='\n', suffix='\n'): > > + """Format the #if condition""" > > + return '%s@b{If:} @code{%s}%s' % ( > > + prefix, ', '.join(ifcond), suffix) if ifcond else '' > > I like the ternary operator, but I nevertheless think this function is > easier to read as > > if not ifcond: > return '' > return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix) > No difference to me, sure > > + > > + > > +def texi_enum_value(value, desc, suffix=''): > > """Format a table of members item for an enumeration value""" > > - return '@item @code{%s}\n' % value.name > > + return '@item @code{%s}\n%s%s' % ( > > + value.name, desc, texi_if(value.ifcond, prefix='@*')) > > Do you ignore suffix intentionally? yes > > > > > > > -def texi_member(member, suffix=''): > > +def texi_member(member, desc='', suffix=''): > > """Format a table of members item for an object type member""" > > typ = member.type.doc_type() > > membertype = ': ' + typ if typ else '' > > - return '@item @code{%s%s}%s%s\n' % ( > > + return '@item @code{%s%s}%s%s\n%s' % ( > > member.name, membertype, > > ' (optional)' if member.optional else '', > > - suffix) > > + suffix, desc) > > > > > > def texi_members(doc, what, base, variants, member_func): > > @@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func): > > desc = 'One of ' + members_text + '\n' > > else: > > desc = 'Not documented\n' > > - items += member_func(section.member) + desc > > + items += member_func(section.member, desc) > > Here, you pass a @desc argument, but no @suffix. > > @member_func is either texi_enum_value() or texi_member(). Works. > > > if base: > > items += '@item The members of @code{%s}\n' % base.doc_type() > > if variants: > > @@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func): > > if v.type.is_implicit(): > > assert not v.type.base and not v.type.variants > > for m in v.type.local_members: > > - items += member_func(m, when) > > + items += member_func(m, suffix=when) > > Here, you pass a @suffix argument, but no @desc. > > texi_enum_value() would choke on that, but it can't occur here, because > enums have no variants. > > Still, I'd prefer texi_enum_value() and texi_member() to have the exact > same signature. OK (not sure what is the more pythonic way) > > > else: > > items += '@item The members of @code{%s}%s\n' % ( > > v.type.doc_type(), when) > > @@ -185,8 +192,7 @@ def texi_sections(doc, ifcond): > > body += texi_example(section.text) > > else: > > body += texi_format(section.text) > > - if ifcond: > > - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) > > + body += texi_if(ifcond, suffix='') > > return body > > > > > > diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json > > index 984cd8ed06..c7fe08c530 100644 > > --- a/tests/qapi-schema/doc-good.json > > +++ b/tests/qapi-schema/doc-good.json > > @@ -55,7 +55,9 @@ > > # > > # @two is undocumented > > ## > > -{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' } > > +{ 'enum': 'Enum', 'data': > > + [ { 'name': 'one', 'if': 'defined(IFENUM)' }, 'two' ], > > + 'if': 'defined(IFCOND)' } > > IFCOND applies to the enum, and IFENUM applies to one of its values. > Awkward. Let's rename IFENUM to IFONE. ok > > > > > ## > > # @Base: > > diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out > > index c2fc5c774a..a05535b69b 100644 > > --- a/tests/qapi-schema/doc-good.out > > +++ b/tests/qapi-schema/doc-good.out > > @@ -11,6 +11,7 @@ enum QType > > module doc-good.json > > enum Enum > > member one > > + if ['defined(IFENUM)'] > > member two > > if ['defined(IFCOND)'] > > object Base > > diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi > > index e42eace474..5c0231e0e6 100644 > > --- a/tests/qapi-schema/doc-good.texi > > +++ b/tests/qapi-schema/doc-good.texi > > @@ -84,12 +84,12 @@ Examples: > > @table @asis > > @item @code{one} > > The @emph{one} @{and only@} > > +@*@b{If:} @code{defined(IFENUM)} > > @* forces a line break. Putting it at the end of the previous line > would be tidier, but let's ignore that for now. For consistency and easier reading, I think it's better to keep it as a seperate line in the end for the corresponding section > > > @item @code{two} > > Not documented > > @end table > > @code{two} is undocumented > > > > - > > @b{If:} @code{defined(IFCOND)} > > @end deftp thanks
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Hi > > On Tue, Dec 11, 2018 at 8:00 PM Markus Armbruster <armbru@redhat.com> wrote: >> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes: >> >> > Use a common function to generate the "If:..." line. >> > >> > While at it, get rid of the existing \n\n (no idea why it was >> > there). Use a line-break in member description, this seems to look >> > slightly better in the plaintext version. >> >> Where exactly in the patch is this done? > > - if ifcond: > - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) > + body += texi_if(ifcond, suffix='') Thanks. I suspected that much, but wasn't sure. > See also doc-good.texi output > >> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> > --- >> > scripts/qapi/doc.py | 24 +++++++++++++++--------- >> > tests/qapi-schema/doc-good.json | 4 +++- >> > tests/qapi-schema/doc-good.out | 1 + >> > tests/qapi-schema/doc-good.texi | 2 +- >> > 4 files changed, 20 insertions(+), 11 deletions(-) >> > >> > diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py >> > index 76cb186ff9..2133ded47e 100755 >> > --- a/scripts/qapi/doc.py >> > +++ b/scripts/qapi/doc.py >> > @@ -126,19 +126,26 @@ def texi_body(doc): >> > return texi_format(doc.body.text) >> > >> > >> > -def texi_enum_value(value): >> > +def texi_if(ifcond, prefix='\n', suffix='\n'): >> > + """Format the #if condition""" >> > + return '%s@b{If:} @code{%s}%s' % ( >> > + prefix, ', '.join(ifcond), suffix) if ifcond else '' >> >> I like the ternary operator, but I nevertheless think this function is >> easier to read as >> >> if not ifcond: >> return '' >> return '%s@b{If:} @code{%s}%s' % (prefix, ', '.join(ifcond), suffix) >> > > No difference to me, sure > >> > + >> > + >> > +def texi_enum_value(value, desc, suffix=''): >> > """Format a table of members item for an enumeration value""" >> > - return '@item @code{%s}\n' % value.name >> > + return '@item @code{%s}\n%s%s' % ( >> > + value.name, desc, texi_if(value.ifcond, prefix='@*')) >> >> Do you ignore suffix intentionally? > > yes That's kind of surprising, until you pry open the abstraction and realize that we reach this one only from the first call of member_func() below, which ... >> >> > >> > >> > -def texi_member(member, suffix=''): >> > +def texi_member(member, desc='', suffix=''): >> > """Format a table of members item for an object type member""" >> > typ = member.type.doc_type() >> > membertype = ': ' + typ if typ else '' >> > - return '@item @code{%s%s}%s%s\n' % ( >> > + return '@item @code{%s%s}%s%s\n%s' % ( >> > member.name, membertype, >> > ' (optional)' if member.optional else '', >> > - suffix) >> > + suffix, desc) >> > >> > >> > def texi_members(doc, what, base, variants, member_func): >> > @@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func): >> > desc = 'One of ' + members_text + '\n' >> > else: >> > desc = 'Not documented\n' >> > - items += member_func(section.member) + desc >> > + items += member_func(section.member, desc) >> >> Here, you pass a @desc argument, but no @suffix. >> >> @member_func is either texi_enum_value() or texi_member(). Works. ... doesn't pass @suffix. So @suffix is empty when we ignore it. >> > if base: >> > items += '@item The members of @code{%s}\n' % base.doc_type() >> > if variants: >> > @@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func): >> > if v.type.is_implicit(): >> > assert not v.type.base and not v.type.variants >> > for m in v.type.local_members: >> > - items += member_func(m, when) >> > + items += member_func(m, suffix=when) >> >> Here, you pass a @suffix argument, but no @desc. >> >> texi_enum_value() would choke on that, but it can't occur here, because >> enums have no variants. >> >> Still, I'd prefer texi_enum_value() and texi_member() to have the exact >> same signature. > > OK (not sure what is the more pythonic way) I suspect there's a clean solution struggling to get out. But I can't grasp it yet. >> >> > else: >> > items += '@item The members of @code{%s}%s\n' % ( >> > v.type.doc_type(), when) >> > @@ -185,8 +192,7 @@ def texi_sections(doc, ifcond): >> > body += texi_example(section.text) >> > else: >> > body += texi_format(section.text) >> > - if ifcond: >> > - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) >> > + body += texi_if(ifcond, suffix='') >> > return body >> > >> > [...]
diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py index 76cb186ff9..2133ded47e 100755 --- a/scripts/qapi/doc.py +++ b/scripts/qapi/doc.py @@ -126,19 +126,26 @@ def texi_body(doc): return texi_format(doc.body.text) -def texi_enum_value(value): +def texi_if(ifcond, prefix='\n', suffix='\n'): + """Format the #if condition""" + return '%s@b{If:} @code{%s}%s' % ( + prefix, ', '.join(ifcond), suffix) if ifcond else '' + + +def texi_enum_value(value, desc, suffix=''): """Format a table of members item for an enumeration value""" - return '@item @code{%s}\n' % value.name + return '@item @code{%s}\n%s%s' % ( + value.name, desc, texi_if(value.ifcond, prefix='@*')) -def texi_member(member, suffix=''): +def texi_member(member, desc='', suffix=''): """Format a table of members item for an object type member""" typ = member.type.doc_type() membertype = ': ' + typ if typ else '' - return '@item @code{%s%s}%s%s\n' % ( + return '@item @code{%s%s}%s%s\n%s' % ( member.name, membertype, ' (optional)' if member.optional else '', - suffix) + suffix, desc) def texi_members(doc, what, base, variants, member_func): @@ -155,7 +162,7 @@ def texi_members(doc, what, base, variants, member_func): desc = 'One of ' + members_text + '\n' else: desc = 'Not documented\n' - items += member_func(section.member) + desc + items += member_func(section.member, desc) if base: items += '@item The members of @code{%s}\n' % base.doc_type() if variants: @@ -165,7 +172,7 @@ def texi_members(doc, what, base, variants, member_func): if v.type.is_implicit(): assert not v.type.base and not v.type.variants for m in v.type.local_members: - items += member_func(m, when) + items += member_func(m, suffix=when) else: items += '@item The members of @code{%s}%s\n' % ( v.type.doc_type(), when) @@ -185,8 +192,7 @@ def texi_sections(doc, ifcond): body += texi_example(section.text) else: body += texi_format(section.text) - if ifcond: - body += '\n\n@b{If:} @code{%s}' % ", ".join(ifcond) + body += texi_if(ifcond, suffix='') return body diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index 984cd8ed06..c7fe08c530 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -55,7 +55,9 @@ # # @two is undocumented ## -{ 'enum': 'Enum', 'data': [ 'one', 'two' ], 'if': 'defined(IFCOND)' } +{ 'enum': 'Enum', 'data': + [ { 'name': 'one', 'if': 'defined(IFENUM)' }, 'two' ], + 'if': 'defined(IFCOND)' } ## # @Base: diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index c2fc5c774a..a05535b69b 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -11,6 +11,7 @@ enum QType module doc-good.json enum Enum member one + if ['defined(IFENUM)'] member two if ['defined(IFCOND)'] object Base diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi index e42eace474..5c0231e0e6 100644 --- a/tests/qapi-schema/doc-good.texi +++ b/tests/qapi-schema/doc-good.texi @@ -84,12 +84,12 @@ Examples: @table @asis @item @code{one} The @emph{one} @{and only@} +@*@b{If:} @code{defined(IFENUM)} @item @code{two} Not documented @end table @code{two} is undocumented - @b{If:} @code{defined(IFCOND)} @end deftp
Use a common function to generate the "If:..." line. While at it, get rid of the existing \n\n (no idea why it was there). Use a line-break in member description, this seems to look slightly better in the plaintext version. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/qapi/doc.py | 24 +++++++++++++++--------- tests/qapi-schema/doc-good.json | 4 +++- tests/qapi-schema/doc-good.out | 1 + tests/qapi-schema/doc-good.texi | 2 +- 4 files changed, 20 insertions(+), 11 deletions(-)