qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 08/20] qapi/parser: differentiate intro and outro paragraphs


From: John Snow
Subject: Re: [PATCH 08/20] qapi/parser: differentiate intro and outro paragraphs
Date: Thu, 16 May 2024 13:43:37 -0400



On Thu, May 16, 2024 at 11:06 AM John Snow <jsnow@redhat.com> wrote:


On Thu, May 16, 2024, 5:34 AM Markus Armbruster <armbru@redhat.com> wrote:
John Snow <jsnow@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 <jsnow@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)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]