[PATCH 16/57] docs/qapi-domain: add "Returns:" field lists

John Snow posted 57 patches 4 weeks ago
There is a newer version of this series
[PATCH 16/57] docs/qapi-domain: add "Returns:" field lists
Posted by John Snow 4 weeks ago
Add "Returns:" field list syntax to QAPI Commands.

Like "Arguments:" and "Errors:", the type name isn't currently processed
for cross-referencing, but this will be addressed in a forthcoming
commit.

This patch adds "errors" as a GroupedField, which means that multiple
return values can be annotated - this is only done because Sphinx does
not seemingly (Maybe I missed it?) support mandatory type arguments to
Ungrouped fields. Because we want to cross-reference this type
information later, we want to make the type argument mandatory. As a
result, you can technically add multiple :return: fields, though I'm not
aware of any circumstance in which you'd need or want
to. Recommendation: "Don't do that, then."

Since this field describes an action/event instead of describing a list
of nouns (arguments, features, errors), I added both the imperative and
indicative forms (:return: and :returns:) to allow doc writers to use
whichever mood "feels right" in the source document. The rendered output
will always use the "Returns:" label, however.

I'm sure you'll let me know how you feel about that. O:-)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapi_domain.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
index d60cccb8e95..7531bdfbba7 100644
--- a/docs/sphinx/qapi_domain.py
+++ b/docs/sphinx/qapi_domain.py
@@ -292,6 +292,13 @@ class QAPICommand(QAPIObject):
                 names=("error", "errors"),
                 has_arg=False,
             ),
+            # :returns TypeName: descr
+            GroupedField(
+                "returnvalue",
+                label=_("Returns"),
+                names=("return", "returns"),
+                can_collapse=True,
+            ),
         ]
     )
 
-- 
2.48.1
Re: [PATCH 16/57] docs/qapi-domain: add "Returns:" field lists
Posted by Markus Armbruster 3 weeks, 5 days ago
John Snow <jsnow@redhat.com> writes:

> Add "Returns:" field list syntax to QAPI Commands.
>
> Like "Arguments:" and "Errors:", the type name isn't currently processed
> for cross-referencing, but this will be addressed in a forthcoming
> commit.
>
> This patch adds "errors" as a GroupedField, which means that multiple

"errors"?

> return values can be annotated - this is only done because Sphinx does
> not seemingly (Maybe I missed it?) support mandatory type arguments to
> Ungrouped fields. Because we want to cross-reference this type
> information later, we want to make the type argument mandatory. As a
> result, you can technically add multiple :return: fields, though I'm not
> aware of any circumstance in which you'd need or want
> to. Recommendation: "Don't do that, then."

scripts/qapi/parser.py rejects duplicate 'Returns:' tags.  So, to do the
thing you shouldn't do, you'd have to use the QAPI domain directly.
I doubt such shenanigans would survive review :)

> Since this field describes an action/event instead of describing a list
> of nouns (arguments, features, errors), I added both the imperative and
> indicative forms (:return: and :returns:) to allow doc writers to use
> whichever mood "feels right" in the source document. The rendered output
> will always use the "Returns:" label, however.
>
> I'm sure you'll let me know how you feel about that. O:-)

My personal taste is imperative mood, always.

Sadly, the QAPI schema language uses 'Returns:'.

The Sphinx Python Domain appears to use :return:.

I recommend to go for consistency with the Python Domain, and ditch
:returns:.

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/sphinx/qapi_domain.py | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
> index d60cccb8e95..7531bdfbba7 100644
> --- a/docs/sphinx/qapi_domain.py
> +++ b/docs/sphinx/qapi_domain.py
> @@ -292,6 +292,13 @@ class QAPICommand(QAPIObject):
>                  names=("error", "errors"),
>                  has_arg=False,
>              ),
> +            # :returns TypeName: descr
> +            GroupedField(
> +                "returnvalue",
> +                label=_("Returns"),
> +                names=("return", "returns"),
> +                can_collapse=True,
> +            ),
>          ]
>      )
Re: [PATCH 16/57] docs/qapi-domain: add "Returns:" field lists
Posted by John Snow 3 weeks, 4 days ago
On Fri, Mar 7, 2025 at 2:58 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Add "Returns:" field list syntax to QAPI Commands.
> >
> > Like "Arguments:" and "Errors:", the type name isn't currently processed
> > for cross-referencing, but this will be addressed in a forthcoming
> > commit.
> >
> > This patch adds "errors" as a GroupedField, which means that multiple
>
> "errors"?
>

Copy-pasto :)


>
> > return values can be annotated - this is only done because Sphinx does
> > not seemingly (Maybe I missed it?) support mandatory type arguments to
> > Ungrouped fields. Because we want to cross-reference this type
> > information later, we want to make the type argument mandatory. As a
> > result, you can technically add multiple :return: fields, though I'm not
> > aware of any circumstance in which you'd need or want
> > to. Recommendation: "Don't do that, then."
>
> scripts/qapi/parser.py rejects duplicate 'Returns:' tags.  So, to do the
> thing you shouldn't do, you'd have to use the QAPI domain directly.
> I doubt such shenanigans would survive review :)
>

Sure, but it's a little weird to be in the headspace of writing a domain
extension that was based on one which *can* be used directly. I know we
won't, but I suppose I am still documenting it and treating it as if you
could.

More the case, it serves as reference if anyone wants to adjust the
behavior of the transmogrifier.

So, consider this documentation for me in the future, or whoever touches
qapidoc if I am felled by an errant spacerock.


>
> > Since this field describes an action/event instead of describing a list
> > of nouns (arguments, features, errors), I added both the imperative and
> > indicative forms (:return: and :returns:) to allow doc writers to use
> > whichever mood "feels right" in the source document. The rendered output
> > will always use the "Returns:" label, however.
> >
> > I'm sure you'll let me know how you feel about that. O:-)
>
> My personal taste is imperative mood, always.
>
> Sadly, the QAPI schema language uses 'Returns:'.
>
> The Sphinx Python Domain appears to use :return:.
>
> I recommend to go for consistency with the Python Domain, and ditch
> :returns:.
>

Done.


>
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  docs/sphinx/qapi_domain.py | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
> > index d60cccb8e95..7531bdfbba7 100644
> > --- a/docs/sphinx/qapi_domain.py
> > +++ b/docs/sphinx/qapi_domain.py
> > @@ -292,6 +292,13 @@ class QAPICommand(QAPIObject):
> >                  names=("error", "errors"),
> >                  has_arg=False,
> >              ),
> > +            # :returns TypeName: descr
> > +            GroupedField(
> > +                "returnvalue",
> > +                label=_("Returns"),
> > +                names=("return", "returns"),
> > +                can_collapse=True,
> > +            ),
> >          ]
> >      )
>
>
Re: [PATCH 16/57] docs/qapi-domain: add "Returns:" field lists
Posted by Markus Armbruster 3 weeks, 4 days ago
John Snow <jsnow@redhat.com> writes:

> On Fri, Mar 7, 2025 at 2:58 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > Add "Returns:" field list syntax to QAPI Commands.
>> >
>> > Like "Arguments:" and "Errors:", the type name isn't currently processed
>> > for cross-referencing, but this will be addressed in a forthcoming
>> > commit.
>> >
>> > This patch adds "errors" as a GroupedField, which means that multiple
>>
>> "errors"?
>>
>
> Copy-pasto :)
>
>
>>
>> > return values can be annotated - this is only done because Sphinx does
>> > not seemingly (Maybe I missed it?) support mandatory type arguments to
>> > Ungrouped fields. Because we want to cross-reference this type
>> > information later, we want to make the type argument mandatory. As a
>> > result, you can technically add multiple :return: fields, though I'm not
>> > aware of any circumstance in which you'd need or want
>> > to. Recommendation: "Don't do that, then."
>>
>> scripts/qapi/parser.py rejects duplicate 'Returns:' tags.  So, to do the
>> thing you shouldn't do, you'd have to use the QAPI domain directly.
>> I doubt such shenanigans would survive review :)
>>
>
> Sure, but it's a little weird to be in the headspace of writing a domain
> extension that was based on one which *can* be used directly. I know we
> won't, but I suppose I am still documenting it and treating it as if you
> could.

Valid argument.

Our test suite only covers use via transmogrifier, not direct use.
Fixable.  I'm not asking you to fix it now.

Commit message could mention the emerging new QAPI doc tool chain obeys
"Don't do that, then".  But I figure you have bigger fish to fry.

> More the case, it serves as reference if anyone wants to adjust the
> behavior of the transmogrifier.
>
> So, consider this documentation for me in the future, or whoever touches
> qapidoc if I am felled by an errant spacerock.
>
>
>>
>> > Since this field describes an action/event instead of describing a list
>> > of nouns (arguments, features, errors), I added both the imperative and
>> > indicative forms (:return: and :returns:) to allow doc writers to use
>> > whichever mood "feels right" in the source document. The rendered output
>> > will always use the "Returns:" label, however.
>> >
>> > I'm sure you'll let me know how you feel about that. O:-)
>>
>> My personal taste is imperative mood, always.
>>
>> Sadly, the QAPI schema language uses 'Returns:'.
>>
>> The Sphinx Python Domain appears to use :return:.
>>
>> I recommend to go for consistency with the Python Domain, and ditch
>> :returns:.
>>
>
> Done.

Thanks!  A few instrances of "returns" in later commit messages need
adjustment.

[...]