Re: [PATCH 10/14] qapi/parser: add _append() method

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Tue, Sep 29, 2026, 9:28 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> 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 <jsnow@redhat.com>
>> > ---
>> >  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?

Sold!

[...]