[PATCH 04/23] qapi: expand tags to all doc sections

John Snow posted 23 patches 4 months, 3 weeks ago
There is a newer version of this series
[PATCH 04/23] qapi: expand tags to all doc sections
Posted by John Snow 4 months, 3 weeks ago
This patch adds an explicit section tag to all QAPIDoc
sections. Members/Features are now explicitly tagged as such, with the
name now being stored in a dedicated "name" field (which qapidoc.py was
not actually using anyway.)

WIP: Yeah, the difference between "tagged" and "untagged" sections is
now pretty poorly named, and explicitly giving "untagged" sections an
"UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
what I need for the series.

Please suggest better naming conventions, keeping in mind that I
currently have plans for a future patch that splits the "UNTAGGED" tag
into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
category of sections that are "formerly known as untagged" but cannot be
called "freeform" because that name is used for the category of
docblocks that are not attached to an entity (but happens to be
comprised entirely of "formerly known as untagged" sections.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapidoc.py |  7 ++--
 scripts/qapi/parser.py | 88 ++++++++++++++++++++++++++++++++----------
 2 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 61997fd21af..6abdcc884f5 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -35,6 +35,7 @@
 from docutils.statemachine import ViewList
 from qapi.error import QAPIError, QAPISemError
 from qapi.gen import QAPISchemaVisitor
+from qapi.parser import QAPIDoc
 from qapi.schema import QAPISchema
 
 from sphinx import addnodes
@@ -258,11 +259,11 @@ def _nodes_for_sections(self, doc):
         """Return list of doctree nodes for additional sections"""
         nodelist = []
         for section in doc.sections:
-            if section.tag and section.tag == 'TODO':
+            if section.tag == QAPIDoc.Tag.TODO:
                 # Hide TODO: sections
                 continue
 
-            if not section.tag:
+            if section.tag == QAPIDoc.Tag.UNTAGGED:
                 # Sphinx cannot handle sectionless titles;
                 # Instead, just append the results to the prior section.
                 container = nodes.container()
@@ -270,7 +271,7 @@ def _nodes_for_sections(self, doc):
                 nodelist += container.children
                 continue
 
-            snode = self._make_section(section.tag)
+            snode = self._make_section(section.tag.name.title())
             self._parse_text_into_node(dedent(section.text), snode)
             nodelist.append(snode)
         return nodelist
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 36cb64a677a..fd841725527 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -15,6 +15,7 @@
 # See the COPYING file in the top-level directory.
 
 from collections import OrderedDict
+import enum
 import os
 import re
 from typing import (
@@ -575,7 +576,10 @@ def get_doc(self) -> 'QAPIDoc':
                         )
                         raise QAPIParseError(self, emsg)
 
-                    doc.new_tagged_section(self.info, match.group(1))
+                    doc.new_tagged_section(
+                        self.info,
+                        QAPIDoc.Tag.from_string(match.group(1))
+                    )
                     text = line[match.end():]
                     if text:
                         doc.append_line(text)
@@ -635,10 +639,30 @@ class QAPIDoc:
     Free-form documentation blocks consist only of a body section.
     """
 
+    class Tag(enum.Enum):
+        UNTAGGED = 0
+        MEMBER = 1
+        FEATURE = 2
+        RETURNS = 3
+        ERRORS = 4
+        SINCE = 5
+        TODO = 6
+
+        @staticmethod
+        def from_string(tag: str) -> 'QAPIDoc.Tag':
+            return QAPIDoc.Tag[tag.upper()]
+
+        def text_required(self) -> bool:
+            # Only "untagged" sections can be empty
+            return self.value not in (0,)
+
     class Section:
         # pylint: disable=too-few-public-methods
-        def __init__(self, info: QAPISourceInfo,
-                     tag: Optional[str] = None):
+        def __init__(
+            self,
+            info: QAPISourceInfo,
+            tag: 'QAPIDoc.Tag',
+        ):
             # section source info, i.e. where it begins
             self.info = info
             # section tag, if any ('Returns', '@name', ...)
@@ -650,8 +674,14 @@ def append_line(self, line: str) -> None:
             self.text += line + '\n'
 
     class ArgSection(Section):
-        def __init__(self, info: QAPISourceInfo, tag: str):
+        def __init__(
+            self,
+            info: QAPISourceInfo,
+            tag: 'QAPIDoc.Tag',
+            name: str
+        ):
             super().__init__(info, tag)
+            self.name = name
             self.member: Optional['QAPISchemaMember'] = None
 
         def connect(self, member: 'QAPISchemaMember') -> None:
@@ -663,7 +693,9 @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None):
         # definition doc's symbol, None for free-form doc
         self.symbol: Optional[str] = symbol
         # the sections in textual order
-        self.all_sections: List[QAPIDoc.Section] = [QAPIDoc.Section(info)]
+        self.all_sections: List[QAPIDoc.Section] = [
+            QAPIDoc.Section(info, QAPIDoc.Tag.UNTAGGED)
+        ]
         # the body section
         self.body: Optional[QAPIDoc.Section] = self.all_sections[0]
         # dicts mapping parameter/feature names to their description
@@ -680,12 +712,17 @@ def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None):
     def end(self) -> None:
         for section in self.all_sections:
             section.text = section.text.strip('\n')
-            if section.tag is not None and section.text == '':
+            if section.tag.text_required() and section.text == '':
                 raise QAPISemError(
                     section.info, "text required after '%s:'" % section.tag)
 
-    def ensure_untagged_section(self, info: QAPISourceInfo) -> None:
-        if self.all_sections and not self.all_sections[-1].tag:
+    def ensure_untagged_section(
+        self,
+        info: QAPISourceInfo,
+    ) -> None:
+        tag = QAPIDoc.Tag.UNTAGGED
+
+        if self.all_sections and self.all_sections[-1].tag == tag:
             # extend current section
             section = self.all_sections[-1]
             if not section.text:
@@ -693,24 +730,29 @@ def ensure_untagged_section(self, info: QAPISourceInfo) -> None:
                 section.info = info
             section.text += '\n'
             return
+
         # start new section
-        section = self.Section(info)
+        section = self.Section(info, tag)
         self.sections.append(section)
         self.all_sections.append(section)
 
-    def new_tagged_section(self, info: QAPISourceInfo, tag: str) -> None:
+    def new_tagged_section(
+        self,
+        info: QAPISourceInfo,
+        tag: 'QAPIDoc.Tag',
+    ) -> None:
         section = self.Section(info, tag)
-        if tag == 'Returns':
+        if tag == QAPIDoc.Tag.RETURNS:
             if self.returns:
                 raise QAPISemError(
                     info, "duplicated '%s' section" % tag)
             self.returns = section
-        elif tag == 'Errors':
+        elif tag == QAPIDoc.Tag.ERRORS:
             if self.errors:
                 raise QAPISemError(
                     info, "duplicated '%s' section" % tag)
             self.errors = section
-        elif tag == 'Since':
+        elif tag == QAPIDoc.Tag.SINCE:
             if self.since:
                 raise QAPISemError(
                     info, "duplicated '%s' section" % tag)
@@ -718,21 +760,26 @@ def new_tagged_section(self, info: QAPISourceInfo, tag: str) -> None:
         self.sections.append(section)
         self.all_sections.append(section)
 
-    def _new_description(self, info: QAPISourceInfo, name: str,
-                         desc: Dict[str, ArgSection]) -> None:
+    def _new_description(
+        self,
+        info: QAPISourceInfo,
+        name: str,
+        tag: 'QAPIDoc.Tag',
+        desc: Dict[str, ArgSection]
+    ) -> None:
         if not name:
             raise QAPISemError(info, "invalid parameter name")
         if name in desc:
             raise QAPISemError(info, "'%s' parameter name duplicated" % name)
-        section = self.ArgSection(info, '@' + name)
+        section = self.ArgSection(info, tag, name)
         self.all_sections.append(section)
         desc[name] = section
 
     def new_argument(self, info: QAPISourceInfo, name: str) -> None:
-        self._new_description(info, name, self.args)
+        self._new_description(info, name, QAPIDoc.Tag.MEMBER, self.args)
 
     def new_feature(self, info: QAPISourceInfo, name: str) -> None:
-        self._new_description(info, name, self.features)
+        self._new_description(info, name, QAPIDoc.Tag.FEATURE, self.features)
 
     def append_line(self, line: str) -> None:
         self.all_sections[-1].append_line(line)
@@ -744,8 +791,9 @@ def connect_member(self, member: 'QAPISchemaMember') -> None:
                 raise QAPISemError(member.info,
                                    "%s '%s' lacks documentation"
                                    % (member.role, member.name))
-            self.args[member.name] = QAPIDoc.ArgSection(
-                self.info, '@' + member.name)
+            section = QAPIDoc.ArgSection(
+                self.info, QAPIDoc.Tag.MEMBER, member.name)
+            self.args[member.name] = section
         self.args[member.name].connect(member)
 
     def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
-- 
2.47.0
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by Markus Armbruster 4 months, 2 weeks ago
John Snow <jsnow@redhat.com> writes:

> This patch adds an explicit section tag to all QAPIDoc
> sections. Members/Features are now explicitly tagged as such, with the
> name now being stored in a dedicated "name" field (which qapidoc.py was
> not actually using anyway.)
>
> WIP: Yeah, the difference between "tagged" and "untagged" sections is
> now pretty poorly named, and explicitly giving "untagged" sections an
> "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
> what I need for the series.
>
> Please suggest better naming conventions, keeping in mind that I
> currently have plans for a future patch that splits the "UNTAGGED" tag
> into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
> category of sections that are "formerly known as untagged" but cannot be
> called "freeform" because that name is used for the category of
> docblocks that are not attached to an entity (but happens to be
> comprised entirely of "formerly known as untagged" sections.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>

A free-form doc comment consists of just one untagged section, actually.
I don't remember whether anything relies on "just one".

The term "tagged" is rooted in doc comment syntax.
docs/devel/qapi-code-gen.rst section "Definition documentation":

    Definition documentation starts with a line naming the definition,
    followed by an optional overview, a description of each argument (for
    commands and events), member (for structs and unions), branch (for
    alternates), or value (for enums), a description of each feature (if
    any), and finally optional tagged sections.

Sadly, this isn't fully accurate anymore.

    Descriptions start with '\@name:'.  The description text must be
    indented [...]

    A tagged section begins with a paragraph that starts with one of the
    following words: "Since:", "Returns:", "Errors:", "TODO:".  It ends with
    the start of a new section.

    The second and subsequent lines of tagged sections must be indented
    [...]

Nothing about untagged sections.  These are sections that aren't
descriptions or tagged.  Example:

    # @Returns: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
    #     sed do eiusmod tempor incididunt ut labore et dolore magna
    #     aliqua.
    #
    # Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris
    # nisi ut aliquip ex ea commodo consequat.
    #
    # Duis aute irure dolor in reprehenderit in voluptate velit esse
    # cillum dolore eu fugiat nulla pariatur.
    ##

Here, the tagged "Returns" section ends after "aliqua."  Why?  Because
"Ut enim" isn't indented.  The untagged section ends after "pariatur."

We parse a definition doc comment as a sequence of sections.

The first one is the overview.

Member / argument descriptions, if any, are next.

Then we may have any number of tagged or untagged sections.  If I
remember correctly, you'd like to banish them.  Let's pretend they can't
exist here.

Then we may have a "Features:" line followed by feature descriptions.

Finally, we may have any number of tagged or untagged sections.

Each of these sections is represented as an instance of type Section,
and the entire definition doc as an instance of type QAPIDoc.

Section has a member @tag of type str.

For tagged sections, it's the tag, i.e "Since", "Returns", ...  Obvious
enough.

For overview and other untagged sections, it's None.  Still obvious.

For descriptions, it's the name of the thing being described.  Less than
obvious.  Note that descriptions are actually instances of ArgSection, a
subtype of Section, which prevents confusion with tagged sections.

QAPIDoc has the overview in member @body, member / argument descriptions
in @args, feature descriptions in @features, and the remaining sections
in @sections.

I'm in favor of cleaning this up some.

I think we can keep the Section name.

Moving the name of the thing being described from @tag to @name is good.
What value to put into @tag then?  Whatever suits you.

Perhaps we should rename @tag to avoid undue ties to tagged sections.
@kind would work for me.

Value None for untagged sections is fine with me.  If a string suits you
better, that's fine, too.  "untagged", "plain", I don't know, propose
something.

@body, @args, and so forth aren't exactly great names.  If they truly
annoy or confuse you, feel free to propose better ones.
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by John Snow 3 months, 4 weeks ago
On Fri, Dec 20, 2024, 8:13 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This patch adds an explicit section tag to all QAPIDoc
> > sections. Members/Features are now explicitly tagged as such, with the
> > name now being stored in a dedicated "name" field (which qapidoc.py was
> > not actually using anyway.)
> >
> > WIP: Yeah, the difference between "tagged" and "untagged" sections is
> > now pretty poorly named, and explicitly giving "untagged" sections an
> > "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
> > what I need for the series.
> >
> > Please suggest better naming conventions, keeping in mind that I
> > currently have plans for a future patch that splits the "UNTAGGED" tag
> > into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
> > category of sections that are "formerly known as untagged" but cannot be
> > called "freeform" because that name is used for the category of
> > docblocks that are not attached to an entity (but happens to be
> > comprised entirely of "formerly known as untagged" sections.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> A free-form doc comment consists of just one untagged section, actually.
> I don't remember whether anything relies on "just one".
>

Sure, yes. Sorry, I keep thinking of documentation as containing "any
number of sections" but keep eliding the fact that our parser
implementation currently will never actually create multiple adjacent
"untagged" sections.

I don't even change this anywhere even in my offline WIP, so it's just me
being over-general.

(I don't think it winds up being relevant or mattering to anything in this
series or my larger project beyond some word choices.)


> The term "tagged" is rooted in doc comment syntax.
> docs/devel/qapi-code-gen.rst section "Definition documentation":
>
>     Definition documentation starts with a line naming the definition,
>     followed by an optional overview, a description of each argument (for
>     commands and events), member (for structs and unions), branch (for
>     alternates), or value (for enums), a description of each feature (if
>     any), and finally optional tagged sections.
>
> Sadly, this isn't fully accurate anymore.
>
>     Descriptions start with '\@name:'.  The description text must be
>     indented [...]
>
>     A tagged section begins with a paragraph that starts with one of the
>     following words: "Since:", "Returns:", "Errors:", "TODO:".  It ends
> with
>     the start of a new section.
>
>     The second and subsequent lines of tagged sections must be indented
>     [...]
>
> Nothing about untagged sections.  These are sections that aren't
> descriptions or tagged.  Example:
>
>     # @Returns: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
>     #     sed do eiusmod tempor incididunt ut labore et dolore magna
>     #     aliqua.
>     #
>     # Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris
>     # nisi ut aliquip ex ea commodo consequat.
>     #
>     # Duis aute irure dolor in reprehenderit in voluptate velit esse
>     # cillum dolore eu fugiat nulla pariatur.
>     ##
>
> Here, the tagged "Returns" section ends after "aliqua."  Why?  Because
> "Ut enim" isn't indented.  The untagged section ends after "pariatur."
>
> We parse a definition doc comment as a sequence of sections.
>
> The first one is the overview.
>
> Member / argument descriptions, if any, are next.
>
> Then we may have any number of tagged or untagged sections.  If I
> remember correctly, you'd like to banish them.  Let's pretend they can't
> exist here.
>

I think you're referring to my desire to banish "untagged" sections from
appearing *between* "tagged" sections. Yes, that's still a desire; though I
make no movement on it in this series as sent to list, and this change is
entirely unrelated to that desire.

(It's more related to being able to distinguish features from members, and
later, distinguishing intro/details. This patch still serves a purpose even
without the inliner or the complexities it brings, but serves both needs.)

((Reminder: the reason for this desire is because "tagged sections" are
rendered in html as a two-column list, and free paragraphs appearing
between list entries looks bad in the rendered documentation, because it
means ending the table, starting paragraph(s), then starting a new table.
If free text is meant to be associated with a specific
member/feature/section-group, it should be marked up (in SOME way) so that
the renderer can achieve that grouping visually.

(There will be a standalone patch that implements this restriction and we
can debate this there, I'm only giving you context here.)))


> Then we may have a "Features:" line followed by feature descriptions.
>
> Finally, we may have any number of tagged or untagged sections.
>
> Each of these sections is represented as an instance of type Section,
> and the entire definition doc as an instance of type QAPIDoc.
>
> Section has a member @tag of type str.
>
> For tagged sections, it's the tag, i.e "Since", "Returns", ...  Obvious
> enough.
>
> For overview and other untagged sections, it's None.  Still obvious.
>
> For descriptions, it's the name of the thing being described.  Less than
> obvious.  Note that descriptions are actually instances of ArgSection, a
> subtype of Section, which prevents confusion with tagged sections.
>

Note that this patch changes this as well; it becomes "member" or "feature"
as appropriate and the name is moved into a dedicated name field that
belongs to the ArgSection class.

(Turns out legacy qapidoc doesn't use this stored name at all anyway, it
fetches the name via the linked feature/member instead.)


> QAPIDoc has the overview in member @body, member / argument descriptions
> in @args, feature descriptions in @features, and the remaining sections
> in @sections.
>
> I'm in favor of cleaning this up some.
>
> I think we can keep the Section name.
>
> Moving the name of the thing being described from @tag to @name is good.
> What value to put into @tag then?  Whatever suits you.
>

What suits me is "member" and "feature". :)


> Perhaps we should rename @tag to avoid undue ties to tagged sections.
> @kind would work for me.
>

Sold!


> Value None for untagged sections is fine with me.  If a string suits you
> better, that's fine, too.  "untagged", "plain", I don't know, propose
> something.
>

For static typing reasons, an explicit tag is preferred to distinguish from
it being "optional".

I could cope with any of:

"plain",
"text",
"plaintext",
"paragraphs",
"unstructured",
"free"

... keeping in mind that I do intend to "split" this tag/kind into "intro"
and "details" later. i.e. this is a temporary tag/kind label.

I think I like "text" the most because it says the least. What about you?


> @body, @args, and so forth aren't exactly great names.  If they truly
> annoy or confuse you, feel free to propose better ones.
>

I believe they can be removed entirely once the old qapidoc is sunset,
leaving only .sections[] behind.

This removes the temptation to pick out sections "out of order".

We only need the list of sections in their source order to generate the
appropriate rST.

(Note: the inliner actually does need to filter sections somewhat to do its
inlining magic, but we'll talk about that later. All you need to know right
now is that my WIP does not utilize any field except .sections[], so the
others can in fact be dropped as redundant once we make the switch. This
patch helps enable the paradigm of "everything you need to render a section
is contained within the Section object itself" which lends itself well to
the new transmogrifier, the goal of always processing/rendering in source
order, and facilitating the mechanics of the inliner.)

...

In case I got too rambly, my action items for this patch are:

- fix the test (already done)
- rename tag to kind
- rename "untagged" to "text", possibly changing it again pending your
feedback.

--js

>
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by Markus Armbruster 3 months, 4 weeks ago
John Snow <jsnow@redhat.com> writes:

> On Fri, Dec 20, 2024, 8:13 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This patch adds an explicit section tag to all QAPIDoc
>> > sections. Members/Features are now explicitly tagged as such, with the
>> > name now being stored in a dedicated "name" field (which qapidoc.py was
>> > not actually using anyway.)
>> >
>> > WIP: Yeah, the difference between "tagged" and "untagged" sections is
>> > now pretty poorly named, and explicitly giving "untagged" sections an
>> > "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
>> > what I need for the series.
>> >
>> > Please suggest better naming conventions, keeping in mind that I
>> > currently have plans for a future patch that splits the "UNTAGGED" tag
>> > into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
>> > category of sections that are "formerly known as untagged" but cannot be
>> > called "freeform" because that name is used for the category of
>> > docblocks that are not attached to an entity (but happens to be
>> > comprised entirely of "formerly known as untagged" sections.)
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> A free-form doc comment consists of just one untagged section, actually.
>> I don't remember whether anything relies on "just one".
>>
>
> Sure, yes. Sorry, I keep thinking of documentation as containing "any
> number of sections" but keep eliding the fact that our parser
> implementation currently will never actually create multiple adjacent
> "untagged" sections.

Yes, the parsers grows an untagged section until another section starts.
An untagged section can consist of multiple paragraphs, just like tagged
sections.

> I don't even change this anywhere even in my offline WIP, so it's just me
> being over-general.
>
> (I don't think it winds up being relevant or mattering to anything in this
> series or my larger project beyond some word choices.)
>
>> The term "tagged" is rooted in doc comment syntax.
>> docs/devel/qapi-code-gen.rst section "Definition documentation":
>>
>>     Definition documentation starts with a line naming the definition,
>>     followed by an optional overview, a description of each argument (for
>>     commands and events), member (for structs and unions), branch (for
>>     alternates), or value (for enums), a description of each feature (if
>>     any), and finally optional tagged sections.
>>
>> Sadly, this isn't fully accurate anymore.
>>
>>     Descriptions start with '\@name:'.  The description text must be
>>     indented [...]
>>
>>     A tagged section begins with a paragraph that starts with one of the
>>     following words: "Since:", "Returns:", "Errors:", "TODO:".  It ends
>> with
>>     the start of a new section.
>>
>>     The second and subsequent lines of tagged sections must be indented
>>     [...]
>>
>> Nothing about untagged sections.  These are sections that aren't
>> descriptions or tagged.  Example:
>>
>>     # @Returns: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
>>     #     sed do eiusmod tempor incididunt ut labore et dolore magna
>>     #     aliqua.
>>     #
>>     # Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris
>>     # nisi ut aliquip ex ea commodo consequat.
>>     #
>>     # Duis aute irure dolor in reprehenderit in voluptate velit esse
>>     # cillum dolore eu fugiat nulla pariatur.
>>     ##
>>
>> Here, the tagged "Returns" section ends after "aliqua."  Why?  Because
>> "Ut enim" isn't indented.  The untagged section ends after "pariatur."
>>
>> We parse a definition doc comment as a sequence of sections.
>>
>> The first one is the overview.
>>
>> Member / argument descriptions, if any, are next.
>>
>> Then we may have any number of tagged or untagged sections.  If I
>> remember correctly, you'd like to banish them.  Let's pretend they can't
>> exist here.
>>
>
> I think you're referring to my desire to banish "untagged" sections from
> appearing *between* "tagged" sections.

Editing accident, sorry!  You interpreted it correctly.

>                                        Yes, that's still a desire; though I
> make no movement on it in this series as sent to list, and this change is
> entirely unrelated to that desire.
>
> (It's more related to being able to distinguish features from members, and
> later, distinguishing intro/details. This patch still serves a purpose even
> without the inliner or the complexities it brings, but serves both needs.)
>
> ((Reminder: the reason for this desire is because "tagged sections" are
> rendered in html as a two-column list, and free paragraphs appearing
> between list entries looks bad in the rendered documentation, because it
> means ending the table, starting paragraph(s), then starting a new table.
> If free text is meant to be associated with a specific
> member/feature/section-group, it should be marked up (in SOME way) so that
> the renderer can achieve that grouping visually.
>
> (There will be a standalone patch that implements this restriction and we
> can debate this there, I'm only giving you context here.)))
>
>
>> Then we may have a "Features:" line followed by feature descriptions.
>>
>> Finally, we may have any number of tagged or untagged sections.
>>
>> Each of these sections is represented as an instance of type Section,
>> and the entire definition doc as an instance of type QAPIDoc.
>>
>> Section has a member @tag of type str.
>>
>> For tagged sections, it's the tag, i.e "Since", "Returns", ...  Obvious
>> enough.
>>
>> For overview and other untagged sections, it's None.  Still obvious.
>>
>> For descriptions, it's the name of the thing being described.  Less than
>> obvious.  Note that descriptions are actually instances of ArgSection, a
>> subtype of Section, which prevents confusion with tagged sections.
>>
>
> Note that this patch changes this as well; it becomes "member" or "feature"
> as appropriate and the name is moved into a dedicated name field that
> belongs to the ArgSection class.
>
> (Turns out legacy qapidoc doesn't use this stored name at all anyway, it
> fetches the name via the linked feature/member instead.)
>
>
>> QAPIDoc has the overview in member @body, member / argument descriptions
>> in @args, feature descriptions in @features, and the remaining sections
>> in @sections.
>>
>> I'm in favor of cleaning this up some.
>>
>> I think we can keep the Section name.
>>
>> Moving the name of the thing being described from @tag to @name is good.
>> What value to put into @tag then?  Whatever suits you.
>>
>
> What suits me is "member" and "feature". :)

Okay.

Doesn't entirely clean up the terminology mess.  According to
docs/devel/qapi-code-gen.rst, struct and union types have members,
alternate types have alternatives, enum types have values, commands have
arguments, and events have event-specific data, which is a mouthful, so
we often say arguments.  Using one of them ("member") to refer to the
generalization of them all is suboptimal, but it's no worse than before.

ArgSection is even more general: it's features, too.  Again, no worse
than before.

I'm *not* asking you to clean this up.  I'm just pointing out we could
use fresh naming ideas here.

>> Perhaps we should rename @tag to avoid undue ties to tagged sections.
>> @kind would work for me.
>>
>
> Sold!
>
>
>> Value None for untagged sections is fine with me.  If a string suits you
>> better, that's fine, too.  "untagged", "plain", I don't know, propose
>> something.
>>
>
> For static typing reasons, an explicit tag is preferred to distinguish from
> it being "optional".
>
> I could cope with any of:
>
> "plain",
> "text",
> "plaintext",
> "paragraphs",
> "unstructured",
> "free"
>
> ... keeping in mind that I do intend to "split" this tag/kind into "intro"
> and "details" later. i.e. this is a temporary tag/kind label.
>
> I think I like "text" the most because it says the least. What about you?

Point, but the other kinds of section are text, too.  "plain"?

>> @body, @args, and so forth aren't exactly great names.  If they truly
>> annoy or confuse you, feel free to propose better ones.
>>
>
> I believe they can be removed entirely once the old qapidoc is sunset,
> leaving only .sections[] behind.
>
> This removes the temptation to pick out sections "out of order".

I've long wanted strict in-order processing, to avoid surprising
reordering of input in the output.

> We only need the list of sections in their source order to generate the
> appropriate rST.
>
> (Note: the inliner actually does need to filter sections somewhat to do its
> inlining magic, but we'll talk about that later. All you need to know right
> now is that my WIP does not utilize any field except .sections[], so the
> others can in fact be dropped as redundant once we make the switch. This
> patch helps enable the paradigm of "everything you need to render a section
> is contained within the Section object itself" which lends itself well to
> the new transmogrifier, the goal of always processing/rendering in source
> order, and facilitating the mechanics of the inliner.)
>
> ...
>
> In case I got too rambly, my action items for this patch are:
>
> - fix the test (already done)
> - rename tag to kind
> - rename "untagged" to "text", possibly changing it again pending your
> feedback.

Sounds good!
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by Markus Armbruster 4 months, 3 weeks ago
John Snow <jsnow@redhat.com> writes:

> This patch adds an explicit section tag to all QAPIDoc
> sections. Members/Features are now explicitly tagged as such, with the
> name now being stored in a dedicated "name" field (which qapidoc.py was
> not actually using anyway.)
>
> WIP: Yeah, the difference between "tagged" and "untagged" sections is
> now pretty poorly named, and explicitly giving "untagged" sections an
> "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
> what I need for the series.
>
> Please suggest better naming conventions, keeping in mind that I
> currently have plans for a future patch that splits the "UNTAGGED" tag
> into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
> category of sections that are "formerly known as untagged" but cannot be
> called "freeform" because that name is used for the category of
> docblocks that are not attached to an entity (but happens to be
> comprised entirely of "formerly known as untagged" sections.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Doesn't pass 'make check' for me.  Diff appended.  It shows the error
messages get worse.


diff --git a/tests/qapi-schema/doc-duplicated-return.err b/tests/qapi-schema/doc-duplicated-return.err
index 503b916b25..c0036fe8aa 100644
--- a/tests/qapi-schema/doc-duplicated-return.err
+++ b/tests/qapi-schema/doc-duplicated-return.err
@@ -1 +1 @@
-doc-duplicated-return.json:8: duplicated 'Returns' section
+doc-duplicated-return.json:8: duplicated 'Tag.RETURNS' section
diff --git a/tests/qapi-schema/doc-duplicated-since.err b/tests/qapi-schema/doc-duplicated-since.err
index a9b60c0c3d..1066040560 100644
--- a/tests/qapi-schema/doc-duplicated-since.err
+++ b/tests/qapi-schema/doc-duplicated-since.err
@@ -1 +1 @@
-doc-duplicated-since.json:8: duplicated 'Since' section
+doc-duplicated-since.json:8: duplicated 'Tag.SINCE' section
diff --git a/tests/qapi-schema/doc-empty-section.err b/tests/qapi-schema/doc-empty-section.err
index 711a0d629c..3eae1b93c9 100644
--- a/tests/qapi-schema/doc-empty-section.err
+++ b/tests/qapi-schema/doc-empty-section.err
@@ -1 +1 @@
-doc-empty-section.json:6: text required after 'Errors:'
+doc-empty-section.json:6: text required after 'Tag.ERRORS:'
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index ec277be91e..87d2f074cf 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -110,7 +110,7 @@ The _one_ {and only}, description on the same line
 Also _one_ {and only}
     feature=enum-member-feat
 a member feature
-    section=None
+    section=Tag.UNTAGGED
 @two is undocumented
 doc symbol=Base
     body=
@@ -168,15 +168,15 @@ description starts on the same line
 a feature
     feature=cmd-feat2
 another feature
-    section=None
+    section=Tag.UNTAGGED
 .. note:: @arg3 is undocumented
-    section=Returns
+    section=Tag.RETURNS
 @Object
-    section=Errors
+    section=Tag.ERRORS
 some
-    section=TODO
+    section=Tag.TODO
 frobnicate
-    section=None
+    section=Tag.UNTAGGED
 .. admonition:: Notes
 
  - Lorem ipsum dolor sit amet
@@ -200,7 +200,7 @@ Examples::
 
 Note::
     Ceci n'est pas une note
-    section=Since
+    section=Tag.SINCE
 2.10
 doc symbol=cmd-boxed
     body=
@@ -209,7 +209,7 @@ If you're bored enough to read this, go see a video of boxed cats
 a feature
     feature=cmd-feat2
 another feature
-    section=None
+    section=Tag.UNTAGGED
 .. qmp-example::
 
    -> "this example"
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by John Snow 4 months, 3 weeks ago
On Wed, Dec 18, 2024, 5:58 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This patch adds an explicit section tag to all QAPIDoc
> > sections. Members/Features are now explicitly tagged as such, with the
> > name now being stored in a dedicated "name" field (which qapidoc.py was
> > not actually using anyway.)
> >
> > WIP: Yeah, the difference between "tagged" and "untagged" sections is
> > now pretty poorly named, and explicitly giving "untagged" sections an
> > "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
> > what I need for the series.
> >
> > Please suggest better naming conventions, keeping in mind that I
> > currently have plans for a future patch that splits the "UNTAGGED" tag
> > into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
> > category of sections that are "formerly known as untagged" but cannot be
> > called "freeform" because that name is used for the category of
> > docblocks that are not attached to an entity (but happens to be
> > comprised entirely of "formerly known as untagged" sections.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> Doesn't pass 'make check' for me.  Diff appended.  It shows the error
> messages get worse.
>

Whoops! My per-patch tester wasn't running make check, only linters and
build testing. I'll fix this, but please keep reviewing in the meantime.

Apologies for the oversight.


>
> diff --git a/tests/qapi-schema/doc-duplicated-return.err
> b/tests/qapi-schema/doc-duplicated-return.err
> index 503b916b25..c0036fe8aa 100644
> --- a/tests/qapi-schema/doc-duplicated-return.err
> +++ b/tests/qapi-schema/doc-duplicated-return.err
> @@ -1 +1 @@
> -doc-duplicated-return.json:8: duplicated 'Returns' section
> +doc-duplicated-return.json:8: duplicated 'Tag.RETURNS' section
> diff --git a/tests/qapi-schema/doc-duplicated-since.err
> b/tests/qapi-schema/doc-duplicated-since.err
> index a9b60c0c3d..1066040560 100644
> --- a/tests/qapi-schema/doc-duplicated-since.err
> +++ b/tests/qapi-schema/doc-duplicated-since.err
> @@ -1 +1 @@
> -doc-duplicated-since.json:8: duplicated 'Since' section
> +doc-duplicated-since.json:8: duplicated 'Tag.SINCE' section
> diff --git a/tests/qapi-schema/doc-empty-section.err
> b/tests/qapi-schema/doc-empty-section.err
> index 711a0d629c..3eae1b93c9 100644
> --- a/tests/qapi-schema/doc-empty-section.err
> +++ b/tests/qapi-schema/doc-empty-section.err
> @@ -1 +1 @@
> -doc-empty-section.json:6: text required after 'Errors:'
> +doc-empty-section.json:6: text required after 'Tag.ERRORS:'
> diff --git a/tests/qapi-schema/doc-good.out
> b/tests/qapi-schema/doc-good.out
> index ec277be91e..87d2f074cf 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -110,7 +110,7 @@ The _one_ {and only}, description on the same line
>  Also _one_ {and only}
>      feature=enum-member-feat
>  a member feature
> -    section=None
> +    section=Tag.UNTAGGED
>  @two is undocumented
>  doc symbol=Base
>      body=
> @@ -168,15 +168,15 @@ description starts on the same line
>  a feature
>      feature=cmd-feat2
>  another feature
> -    section=None
> +    section=Tag.UNTAGGED
>  .. note:: @arg3 is undocumented
> -    section=Returns
> +    section=Tag.RETURNS
>  @Object
> -    section=Errors
> +    section=Tag.ERRORS
>  some
> -    section=TODO
> +    section=Tag.TODO
>  frobnicate
> -    section=None
> +    section=Tag.UNTAGGED
>  .. admonition:: Notes
>
>   - Lorem ipsum dolor sit amet
> @@ -200,7 +200,7 @@ Examples::
>
>  Note::
>      Ceci n'est pas une note
> -    section=Since
> +    section=Tag.SINCE
>  2.10
>  doc symbol=cmd-boxed
>      body=
> @@ -209,7 +209,7 @@ If you're bored enough to read this, go see a video of
> boxed cats
>  a feature
>      feature=cmd-feat2
>  another feature
> -    section=None
> +    section=Tag.UNTAGGED
>  .. qmp-example::
>
>     -> "this example"
>
>
Re: [PATCH 04/23] qapi: expand tags to all doc sections
Posted by Markus Armbruster 4 months ago
John Snow <jsnow@redhat.com> writes:

> On Wed, Dec 18, 2024, 5:58 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This patch adds an explicit section tag to all QAPIDoc
>> > sections. Members/Features are now explicitly tagged as such, with the
>> > name now being stored in a dedicated "name" field (which qapidoc.py was
>> > not actually using anyway.)
>> >
>> > WIP: Yeah, the difference between "tagged" and "untagged" sections is
>> > now pretty poorly named, and explicitly giving "untagged" sections an
>> > "UNTAGGED" tag is ... well, worse. but mechanically, this accomplishes
>> > what I need for the series.
>> >
>> > Please suggest better naming conventions, keeping in mind that I
>> > currently have plans for a future patch that splits the "UNTAGGED" tag
>> > into "INTRO" and "DETAILS" tags. But, we still need a meta-name for the
>> > category of sections that are "formerly known as untagged" but cannot be
>> > called "freeform" because that name is used for the category of
>> > docblocks that are not attached to an entity (but happens to be
>> > comprised entirely of "formerly known as untagged" sections.)
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> Doesn't pass 'make check' for me.  Diff appended.  It shows the error
>> messages get worse.
>
> Whoops! My per-patch tester wasn't running make check, only linters and
> build testing. I'll fix this, but please keep reviewing in the meantime.
>
> Apologies for the oversight.

No worries :)