On Tue, Sep 29, 2026, 9:28 AM Markus Armbruster <[email protected]> wrote:
> John Snow <[email protected]> writes: > > > This method is intended to become the sole point of appending new > > sections to the section list, prohibiting outside callers from > > accessing the list of sections directly. > > > > A forthcoming commit will actually convert the "all_sections" field to > > be a private field to begin enforcing that no outside callers are > > manipulating the section list from outside of this class. > > This suggests you merely add the method now. Not true, you also put it > to use. > Didn't mean to suggest such a thing! I'll try to reword. I mean only to convey that at this point, this doesn't *quite* become the sole access point as intended, due to some transitional code that this series is on its way to remove. I'm trying to justify the partial conversion. > > > > Signed-off-by: John Snow <[email protected]> > > --- > > scripts/qapi/parser.py | 50 ++++++++++++++++++++++++++---------------- > > 1 file changed, 31 insertions(+), 19 deletions(-) > > > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > > index 5fc9c18c81d..0dca1acba4c 100644 > > --- a/scripts/qapi/parser.py > > +++ b/scripts/qapi/parser.py > > @@ -542,7 +542,7 @@ def get_doc(self) -> 'QAPIDoc': > > if not symbol: > > raise QAPIParseError(self, "name required after '@'") > > doc = QAPIDoc(info, symbol) > > - doc.all_sections.append(QAPIDoc.Section(info, > QAPIDoc.Kind.INTRO)) > > + doc.new_tagged_section(info, QAPIDoc.Kind.INTRO) > > line = self.get_doc_indented(doc) > > no_more_args = False > > last_ordered_section = QAPIDoc.Kind.INTRO > > @@ -772,6 +772,33 @@ def __init__(self, info: QAPISourceInfo, symbol: > Optional[str] = None): > > def has_features(self) -> bool: > > return bool(self._features) > > > > + def _append(self, section: 'QAPIDoc.Section') -> None: > > Maybe name it _append_section()? We already have append_line()... > Eh, why not, sure. > > + """ > > + Primary internal interface for adding a new section. > > + > > + It is intended to become the sole access point for adding new > > + sections to the QAPIDoc section list, but we are currently in > > + a transitional phase where other methods access the section > > + list directly to do insertions of generated sections at > > + targeted points. > > Yes: ._insert_near_kind(). > > > + """ > > This comment will soon change to > > """ > Primary internal interface for adding a new section. > > All other methods/helpers/etc ultimately use this and only > this interface for appending a new section. > """ > > There's a bit of redundancy. Suggest > > """ > Add a new section at the end. > > It is intended to ... > """ > > now, and > > """ > Add a new section at the end. > > All other ... > """ > > later. > More or less correct, though "at the end" isn't really true, it's contextually sensitive. "Add a new section" is more correct. Actually, why don't we name it _add_section() to make it clear that it isn't *always* an append? > > + if section.kind == QAPIDoc.Kind.RETURNS: > > + if self._returns: > > + raise QAPISemError( > > + section.info, "duplicated '%s' section" % > section.kind) > > + self._returns = section > > + elif section.kind == QAPIDoc.Kind.ERRORS: > > + if self._errors: > > + raise QAPISemError( > > + section.info, "duplicated '%s' section" % > section.kind) > > + self._errors = section > > + elif section.kind == QAPIDoc.Kind.SINCE: > > + if self.since: > > + raise QAPISemError( > > + section.info, "duplicated '%s' section" % > section.kind) > > + self.since = section > > + self.all_sections.append(section) > > + > > def end(self) -> None: > > for section in self.all_sections: > > section.text = section.text.strip('\n') > > @@ -797,7 +824,7 @@ def ensure_untagged_section(self, info: > QAPISourceInfo) -> None: > > > > # start new section > > section = self.Section(info, kind) > > - self.all_sections.append(section) > > + self._append(section) > > > > def new_tagged_section( > > self, > > @@ -805,22 +832,7 @@ def new_tagged_section( > > kind: 'QAPIDoc.Kind', > > ) -> None: > > section = self.Section(info, kind) > > - if kind == QAPIDoc.Kind.RETURNS: > > - if self._returns: > > - raise QAPISemError( > > - info, "duplicated '%s' section" % kind) > > - self._returns = section > > - elif kind == QAPIDoc.Kind.ERRORS: > > - if self._errors: > > - raise QAPISemError( > > - info, "duplicated '%s' section" % kind) > > - self._errors = section > > - elif kind == QAPIDoc.Kind.SINCE: > > - if self.since: > > - raise QAPISemError( > > - info, "duplicated '%s' section" % kind) > > - self.since = section > > - self.all_sections.append(section) > > + self._append(section) > > > > def _new_description( > > self, > > @@ -834,7 +846,7 @@ def _new_description( > > if name in desc: > > raise QAPISemError(info, "'%s' parameter name duplicated" % > name) > > section = self.ArgSection(info, kind, name) > > - self.all_sections.append(section) > > + self._append(section) > > desc[name] = section > > > > def new_argument(self, info: QAPISourceInfo, name: str) -> None: > >
