On Tue, Mar 25, 2025 at 5:42 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes: > > > The new qapidoc transmogrifier can generate "Returns" statements with > > type information just fine, so we can remove it from the source where it > > doesn't add anything particularly novel or helpful and just repeats the > > type info. > > > > This patch does not touch Returns: lines that add some information > > (potentially helpful, potentially not) but repeats the type information > > to remove that type. > > > > Signed-off-by: John Snow <js...@redhat.com> > > This is a clear improvement for the generated docs. For instance, > blockdev-snapshot-delete-internal-sync goes from > > Return: > "SnapshotInfo" -- SnapshotInfo > > to > > Return: > "SnapshotInfo" > > However, I see that *triplicated* in my testing. I observed similar > issues with the previous patch, so let's discuss that there and ignore > it here. > > The impact on schema file egonomics is less clear. > > This patch removes a bunch of "Returns:" sections that make the > generated docs look bad. How can we stop people from writing such > sections? > > Developers tend to refer to the schema file instead of the generated > documentation. Information is spread across doc comment and schema > code. Both describe the syntactic structure. Only the schema code has > types, optional, and such. The doc comment describes semantics. In > practice, skimming the doc comment is often enough. > > Except for the return value. The doc comment's "Returns:" section is > optional. When it's absent, the generated docs are bad (but this patch > fixes that). Moreover, the doc comment doesn't fully describe the > syntactic structure then. Unwary readers may not be aware of that trap, > and miss the return value. > > The inliner you posted before needs to know where the inlined stuff > goes. Obvious when there are argument descriptions or a "Returns:". > For the cases where we have nothing useful, you proposed an explicit > marker "Details:" (how exactly it's spelled doesn't matter here, only > that an explicit marker can be necessary). Could removing "Returns:" > make the marker necessary more often? Can our tooling reliably detect > the need for the marker? > Well, tooling can at least be certain when it isn't certain. The warning I have in my inliner branch-fork-whatever now basically just looks at the sections and if there's non-plaintext sections between the start and the ending, it treats the beginning as intro and the ending as details. In the case there is *nothing else at all*, i.e. no returns, no arguments/members, no errors, no features - i.e. it's a single QAPIDoc Section - the inliner will count the *paragraphs*. If it's *one* paragraph, it deduces that it's an intro section and does not consider it ambiguous. If there are multiple paragraphs, however, that's when it emits a warning. A computer is never going to be able to reliably determine *intent*, but syntactically I think that's a pretty narrow circumstance to yelp over: "Documentation contains only a single plaintext section that consists of two or more paragraphs". In practice, that's a reasonably rare occurrence and is most likely to occur with query commands that take no arguments, have no features, and do not document return value semantics.