[PATCH 21/57] docs/qapi-domain: add :deprecated: directive option

John Snow posted 57 patches 4 weeks ago
There is a newer version of this series
[PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by John Snow 4 weeks ago
Although "deprecated" is a feature (and *will* appear in the features
list), add a special :deprecated: option to generate an eye-catch that
makes this information very hard to miss.

(The intent is to modify qapidoc.py to add this option whenever it
detects that the features list attached to a definition contains the
"deprecated" entry.)

-

RFC: Technically, this object-level option is un-needed and could be
replaced with a standard content-level directive that e.g. qapidoc.py
could insert at the beginning of the content block. I've done it here as
an option to demonstrate how it would be possible to do.

It's a matter of taste for "where" we feel like implementing it.

One benefit of doing it this way is that we can create a single
containing box to set CSS style options controlling the flow of multiple
infoboxes. The other way to achieve that would be to create a directive
that allows us to set multiple options instead, e.g.:

.. qapi:infoboxes:: deprecated unstable

or possibly:

.. qapi:infoboxes::
   :deprecated:
   :unstable:

For now, I've left these as top-level QAPI object options. "Hey, it works."

P.S., I outsourced the CSS ;)

Signed-off-by: Harmonie Snow <harmonie@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx-static/theme_overrides.css | 25 +++++++++++++++++++++++++
 docs/sphinx/qapi_domain.py             | 26 ++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/docs/sphinx-static/theme_overrides.css b/docs/sphinx-static/theme_overrides.css
index 965ecac54fd..3765cab1b20 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -208,3 +208,28 @@ div[class^="highlight"] pre {
         color: inherit;
     }
 }
+
+/* QAPI domain theming */
+
+.qapi-infopips {
+    margin-bottom: 1em;
+}
+
+.qapi-infopip {
+    display: inline-block;
+    padding: 0em 0.5em 0em 0.5em;
+    margin: 0.25em;
+}
+
+.qapi-deprecated {
+    background-color: #fffef5;
+    border: solid #fff176 6px;
+    font-weight: bold;
+    padding: 8px;
+    border-radius: 15px;
+    margin: 5px;
+}
+
+.qapi-deprecated::before {
+    content: '⚠️ ';
+}
diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
index fff5cca24cc..1be59e36bdf 100644
--- a/docs/sphinx/qapi_domain.py
+++ b/docs/sphinx/qapi_domain.py
@@ -140,6 +140,7 @@ class QAPIObject(ObjectDescription[Signature]):
             "module": directives.unchanged,  # Override contextual module name
             # These are QAPI originals:
             "since": since_validator,
+            "deprecated": directives.flag,
         }
     )
 
@@ -253,6 +254,31 @@ def add_target_and_index(
                     ("single", indextext, node_id, "", None)
                 )
 
+    def _add_infopips(self, contentnode: addnodes.desc_content) -> None:
+        # Add various eye-catches and things that go below the signature
+        # bar, but precede the user-defined content.
+        infopips = nodes.container()
+        infopips.attributes["classes"].append("qapi-infopips")
+
+        def _add_pip(source: str, content: str, classname: str) -> None:
+            node = nodes.container(source)
+            node.append(nodes.Text(content))
+            node.attributes["classes"].extend(["qapi-infopip", classname])
+            infopips.append(node)
+
+        if "deprecated" in self.options:
+            _add_pip(
+                ":deprecated:",
+                f"This {self.objtype} is deprecated.",
+                "qapi-deprecated",
+            )
+
+        if infopips.children:
+            contentnode.insert(0, infopips)
+
+    def transform_content(self, content_node: addnodes.desc_content) -> None:
+        self._add_infopips(content_node)
+
     def _toc_entry_name(self, sig_node: desc_signature) -> str:
         # This controls the name in the TOC and on the sidebar.
 
-- 
2.48.1


Re: [PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by Markus Armbruster 3 weeks, 5 days ago
John Snow <jsnow@redhat.com> writes:

> Although "deprecated" is a feature (and *will* appear in the features
> list), add a special :deprecated: option to generate an eye-catch that
> makes this information very hard to miss.
>
> (The intent is to modify qapidoc.py to add this option whenever it
> detects that the features list attached to a definition contains the
> "deprecated" entry.)

Let me explain it in my own words to make sure I understand.

1. The transmogrifier emits a :feat: for the feature like for any other.
   Therefore, the feature is rendered like any other.,

2. The transmogrifier additionally emits the owning directive with a
   :deprecated: option.  This gets the eye-catch rendered.

Example:

    Command drive-backup (Since: 1.6)
 -->    This command is deprecated.

       Start a point-in-time copy of a block device to a new destination.
       The status of ongoing drive-backup operations can be checked with
       query-block-jobs where the BlockJobInfo.type field has the value
       'backup'.  The operation can be stopped before it has completed
       using the block-job-cancel command.

       Arguments:
          * The members of "DriveBackup".

       Features:
 -->      * **deprecated** -- This command is deprecated.  Use "blockdev-
            backup" instead.

The first arrow marks the eye-catch.  The second marks the normally
rendered feature.

The eye-catch is redundant with the feature rendering.  Readers may
nevertheless find an eye-catch useful.

For what it's worth, the Python documentation has deprecation
information at the end of a definition documentation, together with
"changed in" information.  Both are colored to catch the eye.  More
restrained than your eye-catch.  Also uses less space.

Hmm.

Not a blocker.

> -
>
> RFC: Technically, this object-level option is un-needed and could be
> replaced with a standard content-level directive that e.g. qapidoc.py
> could insert at the beginning of the content block. I've done it here as
> an option to demonstrate how it would be possible to do.
>
> It's a matter of taste for "where" we feel like implementing it.
>
> One benefit of doing it this way is that we can create a single
> containing box to set CSS style options controlling the flow of multiple
> infoboxes. The other way to achieve that would be to create a directive
> that allows us to set multiple options instead, e.g.:
>
> .. qapi:infoboxes:: deprecated unstable
>
> or possibly:
>
> .. qapi:infoboxes::
>    :deprecated:
>    :unstable:
>
> For now, I've left these as top-level QAPI object options. "Hey, it works."

Do you intend to drop this part in the final version?

Having the commit message explain paths not taken can be useful.  But
this is phrased as an RFC, which suggests to me you plan to drop it.

> P.S., I outsourced the CSS ;)

Hi, Harmonie!

> Signed-off-by: Harmonie Snow <harmonie@gmail.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
Re: [PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by John Snow 3 weeks, 3 days ago
On Fri, Mar 7, 2025 at 5:48 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Although "deprecated" is a feature (and *will* appear in the features
> > list), add a special :deprecated: option to generate an eye-catch that
> > makes this information very hard to miss.
> >
> > (The intent is to modify qapidoc.py to add this option whenever it
> > detects that the features list attached to a definition contains the
> > "deprecated" entry.)
>
> Let me explain it in my own words to make sure I understand.
>
> 1. The transmogrifier emits a :feat: for the feature like for any other.
>    Therefore, the feature is rendered like any other.,
>
> 2. The transmogrifier additionally emits the owning directive with a
>    :deprecated: option.  This gets the eye-catch rendered.
>
> Example:
>
>     Command drive-backup (Since: 1.6)
>  -->    This command is deprecated.
>
>        Start a point-in-time copy of a block device to a new destination.
>        The status of ongoing drive-backup operations can be checked with
>        query-block-jobs where the BlockJobInfo.type field has the value
>        'backup'.  The operation can be stopped before it has completed
>        using the block-job-cancel command.
>
>        Arguments:
>           * The members of "DriveBackup".
>
>        Features:
>  -->      * **deprecated** -- This command is deprecated.  Use "blockdev-
>             backup" instead.
>
> The first arrow marks the eye-catch.  The second marks the normally
> rendered feature.
>
> The eye-catch is redundant with the feature rendering.  Readers may
> nevertheless find an eye-catch useful.
>

Yep, you've got it.


>
> For what it's worth, the Python documentation has deprecation
> information at the end of a definition documentation, together with
> "changed in" information.  Both are colored to catch the eye.  More
> restrained than your eye-catch.  Also uses less space.
>
> Hmm.
>
> Not a blocker.
>

I am extremely happy to take patches to change the layout and formatting
after we merge. I'm providing the canvas, others can paint :)


>
> > -
> >
> > RFC: Technically, this object-level option is un-needed and could be
> > replaced with a standard content-level directive that e.g. qapidoc.py
> > could insert at the beginning of the content block. I've done it here as
> > an option to demonstrate how it would be possible to do.
> >
> > It's a matter of taste for "where" we feel like implementing it.
> >
> > One benefit of doing it this way is that we can create a single
> > containing box to set CSS style options controlling the flow of multiple
> > infoboxes. The other way to achieve that would be to create a directive
> > that allows us to set multiple options instead, e.g.:
> >
> > .. qapi:infoboxes:: deprecated unstable
> >
> > or possibly:
> >
> > .. qapi:infoboxes::
> >    :deprecated:
> >    :unstable:
> >
> > For now, I've left these as top-level QAPI object options. "Hey, it
> works."
>
> Do you intend to drop this part in the final version?
>
> Having the commit message explain paths not taken can be useful.  But
> this is phrased as an RFC, which suggests to me you plan to drop it.
>

Yep. You just hadn't reviewed these yet so I left the commentary in :) Now
that you've seen it, it can go.


>
> > P.S., I outsourced the CSS ;)
>
> Hi, Harmonie!
>
> > Signed-off-by: Harmonie Snow <harmonie@gmail.com>
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
>
Re: [PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by Markus Armbruster 4 weeks ago
John Snow <jsnow@redhat.com> writes:

> Although "deprecated" is a feature (and *will* appear in the features
> list), add a special :deprecated: option to generate an eye-catch that
> makes this information very hard to miss.
>
> (The intent is to modify qapidoc.py to add this option whenever it
> detects that the features list attached to a definition contains the
> "deprecated" entry.)
>
> -
>
> RFC: Technically, this object-level option is un-needed and could be
> replaced with a standard content-level directive that e.g. qapidoc.py
> could insert at the beginning of the content block. I've done it here as
> an option to demonstrate how it would be possible to do.
>
> It's a matter of taste for "where" we feel like implementing it.
>
> One benefit of doing it this way is that we can create a single
> containing box to set CSS style options controlling the flow of multiple
> infoboxes. The other way to achieve that would be to create a directive
> that allows us to set multiple options instead, e.g.:
>
> .. qapi:infoboxes:: deprecated unstable
>
> or possibly:
>
> .. qapi:infoboxes::
>    :deprecated:
>    :unstable:
>
> For now, I've left these as top-level QAPI object options. "Hey, it works."
>
> P.S., I outsourced the CSS ;)
>
> Signed-off-by: Harmonie Snow <harmonie@gmail.com>
> Signed-off-by: John Snow <jsnow@redhat.com>

[...]

> diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
> index fff5cca24cc..1be59e36bdf 100644
> --- a/docs/sphinx/qapi_domain.py
> +++ b/docs/sphinx/qapi_domain.py
> @@ -140,6 +140,7 @@ class QAPIObject(ObjectDescription[Signature]):
>              "module": directives.unchanged,  # Override contextual module name
>              # These are QAPI originals:
>              "since": since_validator,
> +            "deprecated": directives.flag,
>          }
>      )
>  
> @@ -253,6 +254,31 @@ def add_target_and_index(
>                      ("single", indextext, node_id, "", None)
>                  )
>  
> +    def _add_infopips(self, contentnode: addnodes.desc_content) -> None:
> +        # Add various eye-catches and things that go below the signature
> +        # bar, but precede the user-defined content.
> +        infopips = nodes.container()
> +        infopips.attributes["classes"].append("qapi-infopips")
> +
> +        def _add_pip(source: str, content: str, classname: str) -> None:
> +            node = nodes.container(source)
> +            node.append(nodes.Text(content))
> +            node.attributes["classes"].extend(["qapi-infopip", classname])
> +            infopips.append(node)
> +
> +        if "deprecated" in self.options:
> +            _add_pip(
> +                ":deprecated:",
> +                f"This {self.objtype} is deprecated.",
> +                "qapi-deprecated",
> +            )
> +
> +        if infopips.children:
> +            contentnode.insert(0, infopips)
> +
> +    def transform_content(self, content_node: addnodes.desc_content) -> None:

pylint warns:

    docs/sphinx/qapi_domain.py:279:4: W0237: Parameter 'contentnode' has been renamed to 'content_node' in overriding 'QAPIObject.transform_content' method (arguments-renamed)

For what it's worth, @content_node is easier on on my eyes than
@contentnode.

> +        self._add_infopips(content_node)
> +
>      def _toc_entry_name(self, sig_node: desc_signature) -> str:
>          # This controls the name in the TOC and on the sidebar.
Re: [PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by John Snow 3 weeks, 6 days ago
On Wed, Mar 5, 2025, 4:13 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Although "deprecated" is a feature (and *will* appear in the features
> > list), add a special :deprecated: option to generate an eye-catch that
> > makes this information very hard to miss.
> >
> > (The intent is to modify qapidoc.py to add this option whenever it
> > detects that the features list attached to a definition contains the
> > "deprecated" entry.)
> >
> > -
> >
> > RFC: Technically, this object-level option is un-needed and could be
> > replaced with a standard content-level directive that e.g. qapidoc.py
> > could insert at the beginning of the content block. I've done it here as
> > an option to demonstrate how it would be possible to do.
> >
> > It's a matter of taste for "where" we feel like implementing it.
> >
> > One benefit of doing it this way is that we can create a single
> > containing box to set CSS style options controlling the flow of multiple
> > infoboxes. The other way to achieve that would be to create a directive
> > that allows us to set multiple options instead, e.g.:
> >
> > .. qapi:infoboxes:: deprecated unstable
> >
> > or possibly:
> >
> > .. qapi:infoboxes::
> >    :deprecated:
> >    :unstable:
> >
> > For now, I've left these as top-level QAPI object options. "Hey, it
> works."
> >
> > P.S., I outsourced the CSS ;)
> >
> > Signed-off-by: Harmonie Snow <harmonie@gmail.com>
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> [...]
>
> > diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
> > index fff5cca24cc..1be59e36bdf 100644
> > --- a/docs/sphinx/qapi_domain.py
> > +++ b/docs/sphinx/qapi_domain.py
> > @@ -140,6 +140,7 @@ class QAPIObject(ObjectDescription[Signature]):
> >              "module": directives.unchanged,  # Override contextual
> module name
> >              # These are QAPI originals:
> >              "since": since_validator,
> > +            "deprecated": directives.flag,
> >          }
> >      )
> >
> > @@ -253,6 +254,31 @@ def add_target_and_index(
> >                      ("single", indextext, node_id, "", None)
> >                  )
> >
> > +    def _add_infopips(self, contentnode: addnodes.desc_content) -> None:
> > +        # Add various eye-catches and things that go below the signature
> > +        # bar, but precede the user-defined content.
> > +        infopips = nodes.container()
> > +        infopips.attributes["classes"].append("qapi-infopips")
> > +
> > +        def _add_pip(source: str, content: str, classname: str) -> None:
> > +            node = nodes.container(source)
> > +            node.append(nodes.Text(content))
> > +            node.attributes["classes"].extend(["qapi-infopip",
> classname])
> > +            infopips.append(node)
> > +
> > +        if "deprecated" in self.options:
> > +            _add_pip(
> > +                ":deprecated:",
> > +                f"This {self.objtype} is deprecated.",
> > +                "qapi-deprecated",
> > +            )
> > +
> > +        if infopips.children:
> > +            contentnode.insert(0, infopips)
> > +
> > +    def transform_content(self, content_node: addnodes.desc_content) ->
> None:
>
> pylint warns:
>
>     docs/sphinx/qapi_domain.py:279:4: W0237: Parameter 'contentnode' has
> been renamed to 'content_node' in overriding 'QAPIObject.transform_content'
> method (arguments-renamed)
>
> For what it's worth, @content_node is easier on on my eyes than
> @contentnode.
>

Almost certifiably a Sphinx version difference that I simply won't be able
to accommodate. It comes back clean against 8.x, and does not impact the
runtime functionality at all.


> > +        self._add_infopips(content_node)
> > +
> >      def _toc_entry_name(self, sig_node: desc_signature) -> str:
> >          # This controls the name in the TOC and on the sidebar.
>
>
Re: [PATCH 21/57] docs/qapi-domain: add :deprecated: directive option
Posted by Markus Armbruster 3 weeks, 6 days ago
John Snow <jsnow@redhat.com> writes:

> On Wed, Mar 5, 2025, 4:13 AM Markus Armbruster <armbru@redhat.com> wrote:

[...]

>> pylint warns:
>>
>>     docs/sphinx/qapi_domain.py:279:4: W0237: Parameter 'contentnode' has
>> been renamed to 'content_node' in overriding 'QAPIObject.transform_content'
>> method (arguments-renamed)
>>
>> For what it's worth, @content_node is easier on on my eyes than
>> @contentnode.
>>
>
> Almost certifiably a Sphinx version difference that I simply won't be able
> to accommodate. It comes back clean against 8.x, and does not impact the
> runtime functionality at all.

If that's the case, don't worry about it.

[...]