On Mon, May 4, 2026 at 5:54 AM Markus Armbruster <[email protected]> wrote:
>
> John Snow <[email protected]> writes:
>
> > These fields are used to provide error checking and internal logistics
> > and should not be used by a user of the library to directly access
> > documentation sections, so make them private.
> >
> > The "since" field alone is left public, as the qapidoc generator does
> > use this field to pull that section out of the regular flow of the
> > document.
> >
> > Signed-off-by: John Snow <[email protected]>
> > ---
> > scripts/qapi/parser.py | 54 +++++++++++++++++++++++-------------------
> > 1 file changed, 29 insertions(+), 25 deletions(-)
> >
> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> > index b33edbba74f..da4756a7424 100644
> > --- a/scripts/qapi/parser.py
> > +++ b/scripts/qapi/parser.py
> > @@ -555,7 +555,7 @@ def get_doc(self) -> 'QAPIDoc':
> > break
> > # Non-blank line, first of a section
> > if line == 'Features:':
> > - if doc.features:
> > + if doc.has_features:
> > raise QAPIParseError(
> > self, "duplicated 'Features:' line")
> > self.accept(False)
> > @@ -570,7 +570,7 @@ def get_doc(self) -> 'QAPIDoc':
> > if text:
> > doc.append_line(text)
> > line = self.get_doc_indented(doc)
> > - if not doc.features:
> > + if not doc.has_features:
> > raise QAPIParseError(
> > self, 'feature descriptions expected')
> > no_more_args = True
>
> These two hunks along with the one adding .has_features() below make the
> patch not purely mechanical. Separate patch, please.
And here I thought I already split this all to the Nth degree. :)
>
> Have you considered using a local bool variable instead?
I prefer having less state and more logic, to avoid having to keep
related state synchronized.
>
> > @@ -733,14 +733,18 @@ def __init__(self, info: QAPISourceInfo, symbol:
> > Optional[str] = None):
> > QAPIDoc.Section(info, QAPIDoc.Kind.PLAIN)
> > ]
> > # dicts mapping parameter/feature names to their description
> > - self.args: Dict[str, QAPIDoc.ArgSection] = {}
> > - self.features: Dict[str, QAPIDoc.ArgSection] = {}
> > + self._args: Dict[str, QAPIDoc.ArgSection] = {}
> > + self._features: Dict[str, QAPIDoc.ArgSection] = {}
> > # a command's "Returns" and "Errors" section
> > - self.returns: Optional[QAPIDoc.Section] = None
> > - self.errors: Optional[QAPIDoc.Section] = None
> > + self._returns: Optional[QAPIDoc.Section] = None
> > + self._errors: Optional[QAPIDoc.Section] = None
> > # "Since" section
> > self.since: Optional[QAPIDoc.Section] = None
> >
> > + @property
> > + def has_features(self) -> bool:
> > + return bool(self._features)
> > +
> > def end(self) -> None:
> > for section in self.all_sections:
> > section.text = section.text.strip('\n')
> > @@ -771,15 +775,15 @@ def new_tagged_section(
> > ) -> None:
> > section = self.Section(info, kind)
> > if kind == QAPIDoc.Kind.RETURNS:
> > - if self.returns:
> > + if self._returns:
> > raise QAPISemError(
> > info, "duplicated '%s' section" % kind)
> > - self.returns = section
> > + self._returns = section
> > elif kind == QAPIDoc.Kind.ERRORS:
> > - if self.errors:
> > + if self._errors:
> > raise QAPISemError(
> > info, "duplicated '%s' section" % kind)
> > - self.errors = section
> > + self._errors = section
> > elif kind == QAPIDoc.Kind.SINCE:
> > if self.since:
> > raise QAPISemError(
> > @@ -803,16 +807,16 @@ def _new_description(
> > desc[name] = section
> >
> > def new_argument(self, info: QAPISourceInfo, name: str) -> None:
> > - self._new_description(info, name, QAPIDoc.Kind.MEMBER, self.args)
> > + self._new_description(info, name, QAPIDoc.Kind.MEMBER, self._args)
> >
> > def new_feature(self, info: QAPISourceInfo, name: str) -> None:
> > - self._new_description(info, name, QAPIDoc.Kind.FEATURE,
> > self.features)
> > + self._new_description(info, name, QAPIDoc.Kind.FEATURE,
> > self._features)
> >
> > def append_line(self, line: str) -> None:
> > self.all_sections[-1].append_line(line)
> >
> > def connect_member(self, member: 'QAPISchemaMember') -> None:
> > - if member.name not in self.args:
> > + if member.name not in self._args:
> > assert member.info
> > if self.symbol not in
> > member.info.pragma.documentation_exceptions:
> > raise QAPISemError(member.info,
> > @@ -823,7 +827,7 @@ def connect_member(self, member: 'QAPISchemaMember') ->
> > None:
> >
> > section = QAPIDoc.ArgSection(
> > self.info, QAPIDoc.Kind.MEMBER, member.name)
> > - self.args[member.name] = section
> > + self._args[member.name] = section
> >
> > # Determine where to insert stub doc - it should go at the
> > # end of the members section(s), if any. Note that index 0
> > @@ -835,14 +839,14 @@ def connect_member(self, member: 'QAPISchemaMember')
> > -> None:
> > index += 1
> > self.all_sections.insert(index, section)
> >
> > - self.args[member.name].connect(member)
> > + self._args[member.name].connect(member)
> >
> > def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
> > - if feature.name not in self.features:
> > + if feature.name not in self._features:
> > raise QAPISemError(feature.info,
> > "feature '%s' lacks documentation"
> > % feature.name)
> > - self.features[feature.name].connect(feature)
> > + self._features[feature.name].connect(feature)
> >
> > def ensure_returns(self, info: QAPISourceInfo) -> None:
> >
> > @@ -883,18 +887,18 @@ def _insert_near_kind(
> >
> > def check_expr(self, expr: QAPIExpression) -> None:
> > if 'command' in expr:
> > - if self.returns and 'returns' not in expr:
> > + if self._returns and 'returns' not in expr:
> > raise QAPISemError(
> > - self.returns.info,
> > + self._returns.info,
> > "'Returns' section, but command doesn't return
> > anything")
> > else:
> > - if self.returns:
> > + if self._returns:
> > raise QAPISemError(
> > - self.returns.info,
> > + self._returns.info,
> > "'Returns' section is only valid for commands")
> > - if self.errors:
> > + if self._errors:
> > raise QAPISemError(
> > - self.errors.info,
> > + self._errors.info,
> > "'Errors' section is only valid for commands")
> >
> > def check(self) -> None:
> > @@ -914,5 +918,5 @@ def check_args_section(
> > "do" if len(bogus) > 1 else "does"
> > ))
> >
> > - check_args_section(self.args, 'member')
> > - check_args_section(self.features, 'feature')
> > + check_args_section(self._args, 'member')
> > + check_args_section(self._features, 'feature')
>
