qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 11/23] docs/qapidoc: add preamble() method


From: John Snow
Subject: Re: [PATCH 11/23] docs/qapidoc: add preamble() method
Date: Wed, 8 Jan 2025 17:58:03 -0500



On Fri, Dec 20, 2024 at 9:15 AM Markus Armbruster <armbru@redhat.com> wrote:
John Snow <jsnow@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 <jsnow@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:


reply via email to

[Prev in Thread] Current Thread [Next in Thread]