This patch changes the qapidoc transmogrifier to generate Return value
documentation for any command that has a return value but hasn't
explicitly documented that return value.
Signed-off-by: John Snow <jsnow@redhat.com>
---
docs/sphinx/qapidoc.py | 14 ++++++++------
scripts/qapi/parser.py | 15 +++++++++++++++
scripts/qapi/schema.py | 3 +++
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 0930307bc73..aaf9921c06c 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -255,16 +255,18 @@ def visit_feature(self, section: QAPIDoc.ArgSection) -> None:
def visit_returns(self, section: QAPIDoc.Section) -> None:
assert isinstance(self.entity, QAPISchemaCommand)
rtype = self.entity.ret_type
- # q_empty can produce None, but we won't be documenting anything
- # without an explicit return statement in the doc block, and we
- # should not have any such explicit statements when there is no
- # return value.
+ # return statements will not be present (and won't be
+ # autogenerated) for any command that doesn't return
+ # *something*, so ret_type will always be defined here.
assert rtype
typ = self.format_type(rtype)
assert typ
- assert section.text
- self.add_field("return", typ, section.text, section.info)
+
+ if section.text:
+ self.add_field("return", typ, section.text, section.info)
+ else:
+ self.add_lines(f":return-nodesc: {typ}", section.info)
def visit_errors(self, section: QAPIDoc.Section) -> None:
# FIXME: the formatting for errors may be inconsistent and may
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 949d9e8bff7..6db08f82409 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -815,6 +815,21 @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
% feature.name)
self.features[feature.name].connect(feature)
+ def ensure_returns(self, info: QAPISourceInfo) -> None:
+ if not any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections):
+
+ stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS)
+
+ # Stub "Returns" section for undocumented returns value.
+ # Insert stub after the last non-PLAIN section.
+ for sect in reversed(self.all_sections):
+ if sect.kind != QAPIDoc.Kind.PLAIN:
+ idx = self.all_sections.index(sect) + 1
+ self.all_sections.insert(idx, stub)
+ break
+ else:
+ self.all_sections.append(stub)
+
def check_expr(self, expr: QAPIExpression) -> None:
if 'command' in expr:
if self.returns and 'returns' not in expr:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cbe3b5aa91e..3abddea3525 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -1062,6 +1062,9 @@ def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
if self.arg_type and self.arg_type.is_implicit():
self.arg_type.connect_doc(doc)
+ if self.ret_type and self.info:
+ doc.ensure_returns(self.info)
+
def visit(self, visitor: QAPISchemaVisitor) -> None:
super().visit(visitor)
visitor.visit_command(
--
2.48.1John Snow <jsnow@redhat.com> writes: > This patch changes the qapidoc transmogrifier to generate Return value > documentation for any command that has a return value but hasn't > explicitly documented that return value. > > Signed-off-by: John Snow <jsnow@redhat.com> Might want to briefly explain placement of the auto-generated return value documentation. But before we discuss that any further, let's review the actual changes the the generated docs. This patch adds auto-generated return value documentation where we have none. The next patch replaces handwritten by auto-generated return value documentation where these are at least as good. Moves the return value docs in some cases. First the additions: * x-debug-query-block-graph Title, intro, features, return * query-tpm Title, intro, return, example * query-dirty-rate Title, intro, arguments, return, examples * query-vcpu-dirty-limit Title, intro, return, example * query-vm-generation-id Title, return * query-memory-size-summary Title, intro, example, return * query-memory-devices Title, intro, return, example * query-acpi-ospm-status Title, intro, return, example * query-stats-schemas Title, intro, arguments, note, return Undesirable: * query-memory-size-summary has returns after the example instead of before. I figure it needs the TODO hack to separate intro and example (see announce-self). * query-stats-schemas has a note between arguments and return. I think this demonstrates that the placement algorithm is too simplistic. Debatable: * x-debug-query-block-graph has returns after features. I'd prefer returns before features. No need to debate this now. Next the movements: * x-debug-block-dirty-bitmap-sha256 From right before errors to right after * blockdev-snapshot-delete-internal-sync From right before errors to right after * query-xen-replication-status From between intro and example to the end * query-colo-status From between intro and example to the end * query-balloon From right before errors to right after * query-hv-balloon-status-report From right before errors to right after * query-yank From between intro and example to the end * add-fd From between arguments and errors to between last note and example I don't like any of these :) Undesirable: * query-xen-replication-status, query-yank, and query-colo-status now have return after the example instead of before. I figure they now need the TODO hack to separate intro and example. * add-fd now has a note between arguments and return. Same placement weakness as for query-stats above. Debatable: * x-debug-block-dirty-bitmap-sha256, blockdev-snapshot-delete-internal-sync, query-colo-status, and query-hv-balloon-status-report now have return after errors instead of before. I'd prefer before. What's the stupidest acceptable placement algorithm? Maybe this one: 1. If we have arguments, return goes right after them. 2. Else if we have errors, return goes right before them. 3. Else if we have features, return goes right before them. 4. Else return goes right after the intro (to make this work, we need a few more TODO hacks). Would you be willing to give this a try?
On Thu, Mar 27, 2025 at 5:11 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > This patch changes the qapidoc transmogrifier to generate Return value > > documentation for any command that has a return value but hasn't > > explicitly documented that return value. > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > Might want to briefly explain placement of the auto-generated return > value documentation. But before we discuss that any further, let's > review the actual changes the the generated docs. > > This patch adds auto-generated return value documentation where we have > none. > > The next patch replaces handwritten by auto-generated return value > documentation where these are at least as good. Moves the return value > docs in some cases. > > First the additions: > > * x-debug-query-block-graph > > Title, intro, features, return > > * query-tpm > > Title, intro, return, example > > * query-dirty-rate > > Title, intro, arguments, return, examples > > * query-vcpu-dirty-limit > > Title, intro, return, example > > * query-vm-generation-id > > Title, return > > * query-memory-size-summary > > Title, intro, example, return > > * query-memory-devices > > Title, intro, return, example > > * query-acpi-ospm-status > > Title, intro, return, example > > * query-stats-schemas > > Title, intro, arguments, note, return > > Undesirable: > > * query-memory-size-summary has returns after the example instead of > before. I figure it needs the TODO hack to separate intro and example > (see announce-self). > Yes > > * query-stats-schemas has a note between arguments and return. I think > this demonstrates that the placement algorithm is too simplistic. > Yeah ... It's just that *glances at length of email* it's been a sensitive topic without a lot of certainty in desire, so I've tried to keep things on the stupid/simple side ... > > Debatable: > > * x-debug-query-block-graph has returns after features. I'd prefer > returns before features. No need to debate this now. > Willing to do this for you if you'd like to enforce it globally. I'd be happy with any mandated order of sections, really. > > Next the movements: > > * x-debug-block-dirty-bitmap-sha256 > > From right before errors to right after > > * blockdev-snapshot-delete-internal-sync > > From right before errors to right after > > * query-xen-replication-status > > From between intro and example to the end > > * query-colo-status > > From between intro and example to the end > > * query-balloon > > From right before errors to right after > > * query-hv-balloon-status-report > > From right before errors to right after > > * query-yank > > From between intro and example to the end > > * add-fd > > From between arguments and errors to between last note and example > > I don't like any of these :) > So it goes ... > > Undesirable: > > * query-xen-replication-status, query-yank, and query-colo-status now > have return after the example instead of before. I figure they now > need the TODO hack to separate intro and example. > Yes > > * add-fd now has a note between arguments and return. Same placement > weakness as for query-stats above. > > Debatable: > > * x-debug-block-dirty-bitmap-sha256, > blockdev-snapshot-delete-internal-sync, query-colo-status, and > query-hv-balloon-status-report now have return after errors instead of > before. I'd prefer before. > > What's the stupidest acceptable placement algorithm? Maybe this one: > > 1. If we have arguments, return goes right after them. > > 2. Else if we have errors, return goes right before them. > > 3. Else if we have features, return goes right before them. > > 4. Else return goes right after the intro (to make this work, we need > a few more TODO hacks). > > Would you be willing to give this a try? > Probably ... So this algorithm seems to imply a preference for this ordering: 1. Intro 2. Arguments 3. Return 4. Errors 5. Features 6. Details Do I have that correct?
John Snow <jsnow@redhat.com> writes: > On Thu, Mar 27, 2025 at 5:11 AM Markus Armbruster <armbru@redhat.com> wrote: > >> John Snow <jsnow@redhat.com> writes: >> >> > This patch changes the qapidoc transmogrifier to generate Return value >> > documentation for any command that has a return value but hasn't >> > explicitly documented that return value. >> > >> > Signed-off-by: John Snow <jsnow@redhat.com> >> >> Might want to briefly explain placement of the auto-generated return >> value documentation. But before we discuss that any further, let's >> review the actual changes the the generated docs. >> >> This patch adds auto-generated return value documentation where we have >> none. >> >> The next patch replaces handwritten by auto-generated return value >> documentation where these are at least as good. Moves the return value >> docs in some cases. >> >> First the additions: >> >> * x-debug-query-block-graph >> >> Title, intro, features, return >> >> * query-tpm >> >> Title, intro, return, example >> >> * query-dirty-rate >> >> Title, intro, arguments, return, examples >> >> * query-vcpu-dirty-limit >> >> Title, intro, return, example >> >> * query-vm-generation-id >> >> Title, return >> >> * query-memory-size-summary >> >> Title, intro, example, return >> >> * query-memory-devices >> >> Title, intro, return, example >> >> * query-acpi-ospm-status >> >> Title, intro, return, example >> >> * query-stats-schemas >> >> Title, intro, arguments, note, return >> >> Undesirable: >> >> * query-memory-size-summary has returns after the example instead of >> before. I figure it needs the TODO hack to separate intro and example >> (see announce-self). >> > > Yes > > >> >> * query-stats-schemas has a note between arguments and return. I think >> this demonstrates that the placement algorithm is too simplistic. >> > > Yeah ... It's just that *glances at length of email* it's been a sensitive > topic without a lot of certainty in desire, so I've tried to keep things on > the stupid/simple side ... When the best solution is unclear, starting discussion with a simplistic solution is smart. Beats starting with a complicated solution that gets shot down. >> Debatable: >> >> * x-debug-query-block-graph has returns after features. I'd prefer >> returns before features. No need to debate this now. >> > > Willing to do this for you if you'd like to enforce it globally. I'd be > happy with any mandated order of sections, really. Could a more rigid order help the inliner, too? >> Next the movements: >> >> * x-debug-block-dirty-bitmap-sha256 >> >> From right before errors to right after >> >> * blockdev-snapshot-delete-internal-sync >> >> From right before errors to right after >> >> * query-xen-replication-status >> >> From between intro and example to the end >> >> * query-colo-status >> >> From between intro and example to the end >> >> * query-balloon >> >> From right before errors to right after >> >> * query-hv-balloon-status-report >> >> From right before errors to right after >> >> * query-yank >> >> From between intro and example to the end >> >> * add-fd >> >> From between arguments and errors to between last note and example >> >> I don't like any of these :) >> > > So it goes ... > > >> >> Undesirable: >> >> * query-xen-replication-status, query-yank, and query-colo-status now >> have return after the example instead of before. I figure they now >> need the TODO hack to separate intro and example. >> > > Yes > > >> >> * add-fd now has a note between arguments and return. Same placement >> weakness as for query-stats above. >> >> Debatable: >> >> * x-debug-block-dirty-bitmap-sha256, >> blockdev-snapshot-delete-internal-sync, query-colo-status, and >> query-hv-balloon-status-report now have return after errors instead of >> before. I'd prefer before. >> >> What's the stupidest acceptable placement algorithm? Maybe this one: >> >> 1. If we have arguments, return goes right after them. >> >> 2. Else if we have errors, return goes right before them. >> >> 3. Else if we have features, return goes right before them. >> >> 4. Else return goes right after the intro (to make this work, we need >> a few more TODO hacks). >> >> Would you be willing to give this a try? >> > > Probably ... > > So this algorithm seems to imply a preference for this ordering: > > 1. Intro > 2. Arguments > 3. Return > 4. Errors > 5. Features > 6. Details > > Do I have that correct? Yes, with 7. Since although a case could also be made for 1. Intro 2. Arguments 3. Return 4. Errors 5. Features 6. Since 7. Details
© 2016 - 2025 Red Hat, Inc.