Re: [PATCH 09/20] qapi/parser: add undocumented stub members to all_sections
John Snow writes:
> On Fri, Jun 14, 2024 at 4:53 AM Markus Armbruster wrote:
>
>> John Snow writes:
>>
>> > This helps simplify the doc generator if it doesn't have to check for
>> > undocumented members.
>> >
>> > Signed-off-by: John Snow
>> > ---
>> > scripts/qapi/parser.py | 20 ++--
>> > 1 file changed, 18 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>> > index b1794f71e12..3cd8e7ee295 100644
>> > --- a/scripts/qapi/parser.py
>> > +++ b/scripts/qapi/parser.py
>> > @@ -740,8 +740,24 @@ 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, 'member')
>> > +
>> > +# Insert stub documentation section for missing member docs.
>> > +section = QAPIDoc.ArgSection(
>> > +self.info, f"@{member.name}", "member")
>>
>> Although I like f-strings in general, I'd pefer to stick to '@' +
>> member.name here, because it's simpler.
>
> Tomayto, Tomahto. (OK.)
Apropos healthy vegetables: at some time, we might want to mass-convert
to f-strings where they are easier to read.
>> Also, let's not change 'member' to "member". Existing practice: single
>> quotes for string literals unless double quotes avoid escapes. Except
>> English prose (like error messages) is always in double quotes.
>>
>
> OK. I realize I'm not consistent in this patch either, but I'll explain
> that my using double quotes here is a black-ism that is sneaking in the
> more I use it to auto-format my patches :)
>
> Maybe time for a flag day when I move scripts/qapi to python/qemu/qapi ...
>
> (Sorry, this type of stuff is ... invisible to me, and I really do rely on
> the linters to make sure I don't do this kind of thing.)
>
>
>>
>> > +self.args[member.name] = section
>> > +
>> > +# Determine where to insert stub doc.
>>
>> If we have some member documentation, the member doc stubs clearly must
>> go there. Inserting them at the end makes sense.
>>
>> Else we want to put them where the parser would accept real member
>> documentation.
>>
>> "The parser" is .get_doc(). This is what it accepts (I'm prepared to
>> explain this in detail if necessary):
>>
>> One untagged section
>>
>> Member documentation, if any
>>
>> Zero ore more tagged or untagged sections
>>
>> Feature documentation, if any
>>
>> Zero or more tagged or untagged sections
>>
>> If we there is no member documentation, this is
>>
>> One untagged section
>>
>> Zero ore more tagged or untagged sections
>>
>> Feature documentation, if any
>>
>> Zero or more tagged or untagged sections
>>
>> Note that we cannot have two adjacent untagged sections (we only create
>> one if the current section isn't untagged; if it is, we extend it
>> instead). Thus, the second section must be tagged or feature
>> documentation.
>>
>> Therefore, the member doc stubs must go right after the first section.
>>
>> This is also where qapidoc.py inserts member documentation.
>>
>> > +index = 0
>> > +for i, sect in enumerate(self.all_sections):
>> > +# insert after these:
>> > +if sect.kind in ('intro-paragraph', 'member'):
>> > +index = i + 1
>> > +# but before these:
>> > +elif sect.kind in ('tagged', 'feature',
>> > 'outro-paragraph'):
>> > +index = i
>> > +break
>>
>> Can you describe what this does in English? As a specification; simply
>> paraphrasing the code is cheating. I tried, and gave up.
>>
>
> It inserts after any intro-paragraph or member section it finds, but before
> any tagged, feature, or outro-paragraph it finds.
>
> The loop breaks on the very first instance of tagged/feature/outro, exiting
> immediately and leaving the insertion index set to the first occurrence of
> such a section, so that the insertion will place the member documentation
> prior to that section.
>
> The loop doesn't break when it finds intro-paragraph or members, so it'll
> continue to tick upwards until it reaches the end of the list or it finds
> something disqualifying.
>
>
>>
>> Above, I derived what I believe we need to do. It's simple enough: if
>> we have member documentation, it starts right after the first (untagged)
>> section, and the stub goes to the end of the member documentation.
>> Else, the stub goes right after the first section.
>>
>> Code:
>>
>> index = 1;
>> while self.all_sections[index].kind == 'member':
>> index += 1
>>
>
> Wellp, yeah. That's certainly less code :)
>
> I tossed in your algorithm alongside mine and as
Re: [PATCH 09/20] qapi/parser: add undocumented stub members to all_sections
On Fri, Jun 14, 2024 at 4:53 AM Markus Armbruster wrote:
> John Snow writes:
>
> > This helps simplify the doc generator if it doesn't have to check for
> > undocumented members.
> >
> > Signed-off-by: John Snow
> > ---
> > scripts/qapi/parser.py | 20 ++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> > index b1794f71e12..3cd8e7ee295 100644
> > --- a/scripts/qapi/parser.py
> > +++ b/scripts/qapi/parser.py
> > @@ -740,8 +740,24 @@ 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, 'member')
> > +
> > +# Insert stub documentation section for missing member docs.
> > +section = QAPIDoc.ArgSection(
> > +self.info, f"@{member.name}", "member")
>
> Although I like f-strings in general, I'd pefer to stick to '@' +
> member.name here, because it's simpler.
>
Tomayto, Tomahto. (OK.)
>
> Also, let's not change 'member' to "member". Existing practice: single
> quotes for string literals unless double quotes avoid escapes. Except
> English prose (like error messages) is always in double quotes.
>
OK. I realize I'm not consistent in this patch either, but I'll explain
that my using double quotes here is a black-ism that is sneaking in the
more I use it to auto-format my patches :)
Maybe time for a flag day when I move scripts/qapi to python/qemu/qapi ...
(Sorry, this type of stuff is ... invisible to me, and I really do rely on
the linters to make sure I don't do this kind of thing.)
>
> > +self.args[member.name] = section
> > +
> > +# Determine where to insert stub doc.
>
> If we have some member documentation, the member doc stubs clearly must
> go there. Inserting them at the end makes sense.
>
> Else we want to put them where the parser would accept real member
> documentation.
>
> "The parser" is .get_doc(). This is what it accepts (I'm prepared to
> explain this in detail if necessary):
>
> One untagged section
>
> Member documentation, if any
>
> Zero ore more tagged or untagged sections
>
> Feature documentation, if any
>
> Zero or more tagged or untagged sections
>
> If we there is no member documentation, this is
>
> One untagged section
>
> Zero ore more tagged or untagged sections
>
> Feature documentation, if any
>
> Zero or more tagged or untagged sections
>
> Note that we cannot have two adjacent untagged sections (we only create
> one if the current section isn't untagged; if it is, we extend it
> instead). Thus, the second section must be tagged or feature
> documentation.
>
> Therefore, the member doc stubs must go right after the first section.
>
> This is also where qapidoc.py inserts member documentation.
>
> > +index = 0
> > +for i, sect in enumerate(self.all_sections):
> > +# insert after these:
> > +if sect.kind in ('intro-paragraph', 'member'):
> > +index = i + 1
> > +# but before these:
> > +elif sect.kind in ('tagged', 'feature',
> 'outro-paragraph'):
> > +index = i
> > +break
>
> Can you describe what this does in English? As a specification; simply
> paraphrasing the code is cheating. I tried, and gave up.
>
It inserts after any intro-paragraph or member section it finds, but before
any tagged, feature, or outro-paragraph it finds.
The loop breaks on the very first instance of tagged/feature/outro, exiting
immediately and leaving the insertion index set to the first occurrence of
such a section, so that the insertion will place the member documentation
prior to that section.
The loop doesn't break when it finds intro-paragraph or members, so it'll
continue to tick upwards until it reaches the end of the list or it finds
something disqualifying.
>
> Above, I derived what I believe we need to do. It's simple enough: if
> we have member documentation, it starts right after the first (untagged)
> section, and the stub goes to the end of the member documentation.
> Else, the stub goes right after the first section.
>
> Code:
>
> index = 1;
> while self.all_sections[index].kind == 'member':
> index += 1
>
Wellp, yeah. That's certainly less code :)
I tossed in your algorithm alongside mine and asserted they were always
equal, and they are, so... yup. I think the only possible concern here is
if there is precisely one and only one section and 1 is beyond EOL, but
that's easy to fix. It apparently doesn't happen in practice, but I can't
presently imagine why it *couldn't* happen.
I'
Re: [PATCH 09/20] qapi/parser: add undocumented stub members to all_sections
John Snow writes:
> This helps simplify the doc generator if it doesn't have to check for
> undocumented members.
>
> Signed-off-by: John Snow
> ---
> scripts/qapi/parser.py | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index b1794f71e12..3cd8e7ee295 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -740,8 +740,24 @@ 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, 'member')
> +
> +# Insert stub documentation section for missing member docs.
> +section = QAPIDoc.ArgSection(
> +self.info, f"@{member.name}", "member")
Although I like f-strings in general, I'd pefer to stick to '@' +
member.name here, because it's simpler.
Also, let's not change 'member' to "member". Existing practice: single
quotes for string literals unless double quotes avoid escapes. Except
English prose (like error messages) is always in double quotes.
> +self.args[member.name] = section
> +
> +# Determine where to insert stub doc.
If we have some member documentation, the member doc stubs clearly must
go there. Inserting them at the end makes sense.
Else we want to put them where the parser would accept real member
documentation.
"The parser" is .get_doc(). This is what it accepts (I'm prepared to
explain this in detail if necessary):
One untagged section
Member documentation, if any
Zero ore more tagged or untagged sections
Feature documentation, if any
Zero or more tagged or untagged sections
If we there is no member documentation, this is
One untagged section
Zero ore more tagged or untagged sections
Feature documentation, if any
Zero or more tagged or untagged sections
Note that we cannot have two adjacent untagged sections (we only create
one if the current section isn't untagged; if it is, we extend it
instead). Thus, the second section must be tagged or feature
documentation.
Therefore, the member doc stubs must go right after the first section.
This is also where qapidoc.py inserts member documentation.
> +index = 0
> +for i, sect in enumerate(self.all_sections):
> +# insert after these:
> +if sect.kind in ('intro-paragraph', 'member'):
> +index = i + 1
> +# but before these:
> +elif sect.kind in ('tagged', 'feature', 'outro-paragraph'):
> +index = i
> +break
Can you describe what this does in English? As a specification; simply
paraphrasing the code is cheating. I tried, and gave up.
Above, I derived what I believe we need to do. It's simple enough: if
we have member documentation, it starts right after the first (untagged)
section, and the stub goes to the end of the member documentation.
Else, the stub goes right after the first section.
Code:
index = 1;
while self.all_sections[index].kind == 'member':
index += 1
Of course future patches I haven't seen might change the invariants in
ways that break my simple code. We'll see.
> +self.all_sections.insert(index, section)
> +
> self.args[member.name].connect(member)
>
> def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
[PATCH 09/20] qapi/parser: add undocumented stub members to all_sections
This helps simplify the doc generator if it doesn't have to check for
undocumented members.
Signed-off-by: John Snow
---
scripts/qapi/parser.py | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index b1794f71e12..3cd8e7ee295 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -740,8 +740,24 @@ 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, 'member')
+
+# Insert stub documentation section for missing member docs.
+section = QAPIDoc.ArgSection(
+self.info, f"@{member.name}", "member")
+self.args[member.name] = section
+
+# Determine where to insert stub doc.
+index = 0
+for i, sect in enumerate(self.all_sections):
+# insert after these:
+if sect.kind in ('intro-paragraph', 'member'):
+index = i + 1
+# but before these:
+elif sect.kind in ('tagged', 'feature', 'outro-paragraph'):
+index = i
+break
+self.all_sections.insert(index, section)
+
self.args[member.name].connect(member)
def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
--
2.44.0
