On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes:
>
> > This method adds the options/preamble to each definition block. Notably,
> > :since: and :ifcond: are added, as are any "special features" such as
> > :deprecated: and :unstable:.
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > docs/sphinx/qapidoc.py | 33 ++++++++++++++++++++++++++++++++-
> > 1 file changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> > index 6f8f69077b1..85c7ce94564 100644
> > --- a/docs/sphinx/qapidoc.py
> > +++ b/docs/sphinx/qapidoc.py
> > @@ -38,7 +38,7 @@
> > from qapi.error import QAPIError, QAPISemError
> > from qapi.gen import QAPISchemaVisitor
> > from qapi.parser import QAPIDoc
> > -from qapi.schema import QAPISchema
> > +from qapi.schema import QAPISchema, QAPISchemaEntity
> > from qapi.source import QAPISourceInfo
> >
> > from sphinx import addnodes
> > @@ -125,6 +125,37 @@ def ensure_blank_line(self) -> None:
> > # +2: correct for zero/one index, then increment by one.
> > self.add_line_raw("", fname, line + 2)
> >
> > + # Transmogrification helpers
> > +
> > + def preamble(self, ent: QAPISchemaEntity) -> None:
> > + """
> > + Generate option lines for qapi entity directives.
> > + """
> > + if ent.doc and ent.doc.since:
> > + assert ent.doc.since.tag == QAPIDoc.Tag.SINCE
> > + # Generated from the entity's docblock; info location is
> exact.
> > + self.add_line(f":since: {ent.doc.since.text}",
> ent.doc.since.info)
> > +
> > + if ent.ifcond.is_present():
> > + doc = ent.ifcond.docgen()
> > + # Generated from entity definition; info location is
> approximate.
> > + self.add_line(f":ifcond: {doc}", ent.info)
> > +
> > + # Hoist special features such as :deprecated: and :unstable:
> > + # into the options block for the entity. If, in the future, new
> > + # special features are added, qapi-domain will chirp about
> > + # unrecognized options and fail.
> > + for feat in ent.features:
> > + if feat.is_special():
> > + # We don't expect special features to have an ifcond
> property.
> > + # (Hello, intrepid developer in the future who changed
> that!)
> > + # ((With luck, you are not me.))
> > + assert not feat.ifcond.is_present()
>
> Nope :)
>
> The attempt to add a conditional special feature now fails with
>
> Sphinx parallel build error:
> AssertionError
>
> If you want to outlaw conditional special features, reject them cleanly
> in schema.py, document the restriction in docs/devel/qapi-code-gen.rst,
> and explain why in the commit message. Recommend a separate commit, to
> make it stand out in git-log.
>
Do you advocate this? I wasn't sure what it *meant* for a special feature
to be conditional; I couldn't conceive of what it meant to have an ifcond
for "deprecated" or "unstable", for instance. It sounds like it isn't well
defined, but we happen to not expressly forbid it.
I guard against it here because, similarly, I have no idea how to handle
the case where it's true.
I didn't realize we technically allow it, though ... would you like me to
move to expressly forbid it in the parser? (Failing that, I have no idea
how to display this information otherwise, so I'd need you to sketch
something out for me; so my inclination is to forbid it as you suggest.
Future developers can always lift the restriction once they have some
use-case in mind and a plan for how to display that information.)
--js
>
> > + # Generated from entity def; info location is
> approximate.
> > + self.add_line(f":{feat.name}:", feat.info)
> > +
> > + self.ensure_blank_line()
> > +
> > # Transmogrification core methods
> >
> > def visit_module(self, path: str) -> None:
>
>
