On Thu, May 16, 2024 at 11:06 AM John Snow <js...@redhat.com> wrote:
> > > On Thu, May 16, 2024, 5:34 AM Markus Armbruster <arm...@redhat.com> wrote: > >> John Snow <js...@redhat.com> writes: >> >> > Add a semantic tag to paragraphs that appear *before* tagged >> > sections/members/features and those that appear after. This will control >> > how they are inlined when doc sections are merged and flattened. >> >> This future use is not obvious to me now. I guess the effective way to >> help me see it is actual patches, which will come in due time. >> > > Head recursion and tail recursion, respectively :) > > * intro > * inherited intro > * members [ancestor-descendent] > * features [ancestor-descendent] > * inherited outro > * outro > > Child gets the first and final words. Inherited stuff goes in the sandwich > fillings. > > It feels like a simple rule that's easy to internalize. As a bonus, you > can explain it by analogy to Americans as a burger, which is the only > metaphor we understand. > > >> > Signed-off-by: John Snow <js...@redhat.com> >> > --- >> > scripts/qapi/parser.py | 22 +++++++++++++++++----- >> > 1 file changed, 17 insertions(+), 5 deletions(-) >> > >> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py >> > index cf4cbca1c1f..b1794f71e12 100644 >> > --- a/scripts/qapi/parser.py >> > +++ b/scripts/qapi/parser.py >> > @@ -503,6 +503,10 @@ def get_doc(self) -> 'QAPIDoc': >> > self.accept(False) >> > line = self.get_doc_line() >> > no_more_args = False >> > + # Paragraphs before members/features/tagged are "intro" >> paragraphs. >> > + # Any appearing subsequently are "outro" paragraphs. >> > + # This is only semantic metadata for the doc generator. >> >> Not sure about the last sentence. Isn't it true for almost everything >> around here? >> > > I guess I was trying to say "There's no real difference between the two > mechanically, it's purely based on where it appears in the doc block, which > offers only a heuristic for its semantic value- introductory statements or > additional detail." > > In my mind: the other "kind" values have some more mechanical difference > to them, but intro/outro don't. > > >> Also, long line. >> >> > + intro = True >> > >> > while line is not None: >> > # Blank lines >> > @@ -532,6 +536,7 @@ def get_doc(self) -> 'QAPIDoc': >> > raise QAPIParseError( >> > self, 'feature descriptions expected') >> > no_more_args = True >> > + intro = False >> >> After feature descriptions. >> >> > elif match := self._match_at_name_colon(line): >> > # description >> > if no_more_args: >> > @@ -547,6 +552,7 @@ def get_doc(self) -> 'QAPIDoc': >> > doc.append_line(text) >> > line = self.get_doc_indented(doc) >> > no_more_args = True >> > + intro = False >> >> Or after member descriptions. >> >> > elif match := re.match( >> > >> r'(Returns|Errors|Since|Notes?|Examples?|TODO): *', >> > line): >> > @@ -557,13 +563,14 @@ def get_doc(self) -> 'QAPIDoc': >> > doc.append_line(text) >> > line = self.get_doc_indented(doc) >> > no_more_args = True >> > + intro = False >> >> Or after the first tagged section. >> >> Okay, it does what it says on the tin. >> >> > elif line.startswith('='): >> > raise QAPIParseError( >> > self, >> > "unexpected '=' markup in definition >> documentation") >> > else: >> > # tag-less paragraph >> > - doc.ensure_untagged_section(self.info) >> > + doc.ensure_untagged_section(self.info, intro) >> > doc.append_line(line) >> > line = self.get_doc_paragraph(doc) >> > else: >> > @@ -617,7 +624,7 @@ def __init__( >> > self, >> > info: QAPISourceInfo, >> > tag: Optional[str] = None, >> > - kind: str = 'paragraph', >> > + kind: str = 'intro-paragraph', >> >> The question "why is this optional?" crossed my mind when reviewing the >> previous patch. I left it unasked, because I felt challenging the >> overlap between @kind and @tag was more useful. However, the new >> default value 'intro-paragraph' feels more arbitrary to me than the old >> one 'paragraph', and that makes the question pop right back into my >> mind. >> > > Just "don't break API" habit, nothing more. I can make it mandatory. > > >> Unless I'm mistaken, all calls but one @tag and @kind. Making that one >> pass it too feels simpler to me. >> >> Moot if we fuse @tag and @kind, of course. > > >> > ): >> > # section source info, i.e. where it begins >> > self.info = info >> > @@ -625,7 +632,7 @@ def __init__( >> > self.tag = tag >> > # section text without tag >> > self.text = '' >> > - # section type - {paragraph, feature, member, tagged} >> > + # section type - {<intro|outro>-paragraph, feature, >> member, tagged} >> >> Long line. > > > Oops, default for black is longer. Forgot to enable the "I still use email > patches as part of my penance" setting > Oh, no, this is actually 79 columns on the button. Not picked up by *any* of our linters. Ehm... can you make the tools enforce them to your preference instead, please...? What's a "long line"? > > >> > self.kind = kind >> > >> > def append_line(self, line: str) -> None: >> > @@ -666,7 +673,11 @@ def end(self) -> None: >> > raise QAPISemError( >> > section.info, "text required after '%s:'" % >> section.tag) >> > >> > - def ensure_untagged_section(self, info: QAPISourceInfo) -> None: >> > + def ensure_untagged_section( >> > + self, >> > + info: QAPISourceInfo, >> > + intro: bool = True, >> >> Two callers, one passes @info, one doesn't. Passing it always might be >> simpler. >> > > Okeydokey. > > >> > + ) -> None: >> > if self.all_sections and not self.all_sections[-1].tag: >> > section = self.all_sections[-1] >> > # Section is empty so far; update info to start *here*. >> > @@ -677,7 +688,8 @@ def ensure_untagged_section(self, info: >> QAPISourceInfo) -> None: >> > self.all_sections[-1].text += '\n' >> > return >> > # start new section >> > - section = self.Section(info) >> > + kind = ("intro" if intro else "outro") + "-paragraph" >> > + section = self.Section(info, kind=kind) >> > self.sections.append(section) >> > self.all_sections.append(section) >> >>