diff mbox series

[12/17] qapi: Rewrite parsing of doc comment section symbols and tags

Message ID 20230428105429.1687850-13-armbru@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: Reformat doc comments | expand

Commit Message

Markus Armbruster April 28, 2023, 10:54 a.m. UTC 
To recognize a line starting with a section symbol and or tag, we
first split it at the first space, then examine the part left of the
space.  We can just as well examine the unsplit line, so do that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/parser.py | 51 +++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

Comments

Juan Quintela April 28, 2023, 6:30 p.m. UTC | #1 
Markus Armbruster <armbru@redhat.com> wrote:
> To recognize a line starting with a section symbol and or tag, we
> first split it at the first space, then examine the part left of the
> space.  We can just as well examine the unsplit line, so do that.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

My python regexp kungfu is at the limit with this patch, so take it as
you wish.  I understand the intent (very good commit message) and "more
or less" how it is done.  Important words on the previous sentence are
"more or less".  O:-)
Markus Armbruster May 9, 2023, 7:27 a.m. UTC | #2 
Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> To recognize a line starting with a section symbol and or tag, we
>> first split it at the first space, then examine the part left of the
>> space.  We can just as well examine the unsplit line, so do that.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> My python regexp kungfu is at the limit with this patch, so take it as
> you wish.  I understand the intent (very good commit message) and "more
> or less" how it is done.  Important words on the previous sentence are
> "more or less".  O:-)

Tests pass, so what could possibly go wrong (famous last words).

Thanks!
Markus Armbruster May 10, 2023, 7:31 a.m. UTC | #3 
Markus Armbruster <armbru@redhat.com> writes:

> To recognize a line starting with a section symbol and or tag, we
> first split it at the first space, then examine the part left of the
> space.  We can just as well examine the unsplit line, so do that.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi/parser.py | 51 +++++++++++++++++++-----------------------
>  1 file changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index ddc14ceaba..fc04c4573e 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -560,12 +560,12 @@ def end_comment(self) -> None:
>          self._switch_section(QAPIDoc.NullSection(self._parser))
>  
>      @staticmethod
> -    def _is_section_tag(name: str) -> bool:
> -        return name in ('Returns:', 'Since:',
> -                        # those are often singular or plural
> -                        'Note:', 'Notes:',
> -                        'Example:', 'Examples:',
> -                        'TODO:')
> +    def _match_at_name_colon(string: str) -> re.Match:
> +        return re.match(r'@([^:]*): *', string)
> +
> +    @staticmethod
> +    def _match_section_tag(string: str) -> re.Match:
> +        return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', string)
>  
>      def _append_body_line(self, line: str) -> None:
>          """
> @@ -581,7 +581,6 @@ def _append_body_line(self, line: str) -> None:
>  
>          Else, append the line to the current section.
>          """
> -        name = line.split(' ', 1)[0]
>          # FIXME not nice: things like '#  @foo:' and '# @foo: ' aren't
>          # recognized, and get silently treated as ordinary text
>          if not self.symbol and not self.body.text and line.startswith('@'):
> @@ -595,12 +594,12 @@ def _append_body_line(self, line: str) -> None:
>                      self._parser, "name required after '@'")
>          elif self.symbol:
>              # This is a definition documentation block
> -            if name.startswith('@') and name.endswith(':'):
> +            if self._match_at_name_colon(line):
>                  self._append_line = self._append_args_line
>                  self._append_args_line(line)
>              elif line == 'Features:':
>                  self._append_line = self._append_features_line
> -            elif self._is_section_tag(name):
> +            elif self._match_section_tag(line):
>                  self._append_line = self._append_various_line
>                  self._append_various_line(line)
>              else:
> @@ -621,16 +620,15 @@ def _append_args_line(self, line: str) -> None:
>          Else, append the line to the current section.
>  
>          """
> -        name = line.split(' ', 1)[0]
> -
> -        if name.startswith('@') and name.endswith(':'):
> +        if match := self._match_at_name_colon(line):
>              # If line is "@arg:   first line of description", find
>              # the index of 'f', which is the indent we expect for any
>              # following lines.  We then remove the leading "@arg:"
>              # from line and replace it with spaces so that 'f' has the
>              # same index as it did in the original line and can be
>              # handled the same way we will handle following lines.
> -            indent = must_match(r'@\S*:\s*', line).end()
> +            name = match.group(1)
> +            indent = match.end()
>              line = line[indent:]
>              if not line:
>                  # Line was just the "@arg:" header
> @@ -638,8 +636,8 @@ def _append_args_line(self, line: str) -> None:
>                  indent = -1
>              else:
>                  line = ' ' * indent + line
> -            self._start_args_section(name[1:-1], indent)
> -        elif self._is_section_tag(name):
> +            self._start_args_section(name, indent)
> +        elif self._match_section_tag(line):
>              self._append_line = self._append_various_line
>              self._append_various_line(line)
>              return
> @@ -656,16 +654,15 @@ def _append_args_line(self, line: str) -> None:
>          self._append_freeform(line)
>  
>      def _append_features_line(self, line: str) -> None:
> -        name = line.split(' ', 1)[0]
> -
> -        if name.startswith('@') and name.endswith(':'):
> +        if match := self._match_at_name_colon(line):
>              # If line is "@arg:   first line of description", find
>              # the index of 'f', which is the indent we expect for any
>              # following lines.  We then remove the leading "@arg:"
>              # from line and replace it with spaces so that 'f' has the
>              # same index as it did in the original line and can be
>              # handled the same way we will handle following lines.
> -            indent = must_match(r'@\S*:\s*', line).end()
> +            name = match.group(1)
> +            indent = match.end()
>              line = line[indent:]
>              if not line:
>                  # Line was just the "@arg:" header
> @@ -673,8 +670,8 @@ def _append_features_line(self, line: str) -> None:
>                  indent = -1
>              else:
>                  line = ' ' * indent + line
> -            self._start_features_section(name[1:-1], indent)
> -        elif self._is_section_tag(name):
> +            self._start_features_section(name, indent)
> +        elif self._match_section_tag(line):
>              self._append_line = self._append_various_line
>              self._append_various_line(line)
>              return
> @@ -698,13 +695,11 @@ def _append_various_line(self, line: str) -> None:
>  
>          Else, append the line to the current section.
>          """
> -        name = line.split(' ', 1)[0]
> -
> -        if name.startswith('@') and name.endswith(':'):
> +        if match := self._match_at_name_colon(line):
>              raise QAPIParseError(self._parser,
> -                                 "'%s' can't follow '%s' section"
> -                                 % (name, self.sections[0].name))
> -        if self._is_section_tag(name):
> +                                 "'@%s:' can't follow '%s' section"
> +                                 % (match.group(1), self.sections[0].name))
> +        if match := self._match_section_tag(line):
>              # If line is "Section:   first line of description", find
>              # the index of 'f', which is the indent we expect for any
>              # following lines.  We then remove the leading "Section:"
> @@ -719,7 +714,7 @@ def _append_various_line(self, line: str) -> None:
>                  indent = 0
>              else:
>                  line = ' ' * indent + line
> -            self._start_section(name[:-1], indent)
> +            self._start_section(match.group(1), indent)
>  
>          self._append_freeform(line)

Need to squash in the appended patch for Python 3.7 and older.

My job description doesn't include "collect paper cuts", but it totally
should.


diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 22ee631198..4923a59d60 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -623,7 +623,8 @@ def _append_args_line(self, line: str) -> None:
         Else, append the line to the current section.
 
         """
-        if match := self._match_at_name_colon(line):
+        match = self._match_at_name_colon(line)
+        if match:
             line = line[match.end():]
             self._start_args_section(match.group(1))
         elif self._match_section_tag(line):
@@ -643,7 +644,8 @@ def _append_args_line(self, line: str) -> None:
         self._append_freeform(line)
 
     def _append_features_line(self, line: str) -> None:
-        if match := self._match_at_name_colon(line):
+        match = self._match_at_name_colon(line)
+        if match:
             line = line[match.end():]
             self._start_features_section(match.group(1))
         elif self._match_section_tag(line):
@@ -670,11 +672,13 @@ def _append_various_line(self, line: str) -> None:
 
         Else, append the line to the current section.
         """
-        if match := self._match_at_name_colon(line):
+        match = self._match_at_name_colon(line)
+        if match:
             raise QAPIParseError(self._parser,
                                  "'@%s:' can't follow '%s' section"
                                  % (match.group(1), self.sections[0].name))
-        if match := self._match_section_tag(line):
+        match = self._match_section_tag(line)
+        if match:
             line = line[match.end():]
             self._start_section(match.group(1))
diff mbox series

Patch

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index ddc14ceaba..fc04c4573e 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -560,12 +560,12 @@  def end_comment(self) -> None:
         self._switch_section(QAPIDoc.NullSection(self._parser))
 
     @staticmethod
-    def _is_section_tag(name: str) -> bool:
-        return name in ('Returns:', 'Since:',
-                        # those are often singular or plural
-                        'Note:', 'Notes:',
-                        'Example:', 'Examples:',
-                        'TODO:')
+    def _match_at_name_colon(string: str) -> re.Match:
+        return re.match(r'@([^:]*): *', string)
+
+    @staticmethod
+    def _match_section_tag(string: str) -> re.Match:
+        return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', string)
 
     def _append_body_line(self, line: str) -> None:
         """
@@ -581,7 +581,6 @@  def _append_body_line(self, line: str) -> None:
 
         Else, append the line to the current section.
         """
-        name = line.split(' ', 1)[0]
         # FIXME not nice: things like '#  @foo:' and '# @foo: ' aren't
         # recognized, and get silently treated as ordinary text
         if not self.symbol and not self.body.text and line.startswith('@'):
@@ -595,12 +594,12 @@  def _append_body_line(self, line: str) -> None:
                     self._parser, "name required after '@'")
         elif self.symbol:
             # This is a definition documentation block
-            if name.startswith('@') and name.endswith(':'):
+            if self._match_at_name_colon(line):
                 self._append_line = self._append_args_line
                 self._append_args_line(line)
             elif line == 'Features:':
                 self._append_line = self._append_features_line
-            elif self._is_section_tag(name):
+            elif self._match_section_tag(line):
                 self._append_line = self._append_various_line
                 self._append_various_line(line)
             else:
@@ -621,16 +620,15 @@  def _append_args_line(self, line: str) -> None:
         Else, append the line to the current section.
 
         """
-        name = line.split(' ', 1)[0]
-
-        if name.startswith('@') and name.endswith(':'):
+        if match := self._match_at_name_colon(line):
             # If line is "@arg:   first line of description", find
             # the index of 'f', which is the indent we expect for any
             # following lines.  We then remove the leading "@arg:"
             # from line and replace it with spaces so that 'f' has the
             # same index as it did in the original line and can be
             # handled the same way we will handle following lines.
-            indent = must_match(r'@\S*:\s*', line).end()
+            name = match.group(1)
+            indent = match.end()
             line = line[indent:]
             if not line:
                 # Line was just the "@arg:" header
@@ -638,8 +636,8 @@  def _append_args_line(self, line: str) -> None:
                 indent = -1
             else:
                 line = ' ' * indent + line
-            self._start_args_section(name[1:-1], indent)
-        elif self._is_section_tag(name):
+            self._start_args_section(name, indent)
+        elif self._match_section_tag(line):
             self._append_line = self._append_various_line
             self._append_various_line(line)
             return
@@ -656,16 +654,15 @@  def _append_args_line(self, line: str) -> None:
         self._append_freeform(line)
 
     def _append_features_line(self, line: str) -> None:
-        name = line.split(' ', 1)[0]
-
-        if name.startswith('@') and name.endswith(':'):
+        if match := self._match_at_name_colon(line):
             # If line is "@arg:   first line of description", find
             # the index of 'f', which is the indent we expect for any
             # following lines.  We then remove the leading "@arg:"
             # from line and replace it with spaces so that 'f' has the
             # same index as it did in the original line and can be
             # handled the same way we will handle following lines.
-            indent = must_match(r'@\S*:\s*', line).end()
+            name = match.group(1)
+            indent = match.end()
             line = line[indent:]
             if not line:
                 # Line was just the "@arg:" header
@@ -673,8 +670,8 @@  def _append_features_line(self, line: str) -> None:
                 indent = -1
             else:
                 line = ' ' * indent + line
-            self._start_features_section(name[1:-1], indent)
-        elif self._is_section_tag(name):
+            self._start_features_section(name, indent)
+        elif self._match_section_tag(line):
             self._append_line = self._append_various_line
             self._append_various_line(line)
             return
@@ -698,13 +695,11 @@  def _append_various_line(self, line: str) -> None:
 
         Else, append the line to the current section.
         """
-        name = line.split(' ', 1)[0]
-
-        if name.startswith('@') and name.endswith(':'):
+        if match := self._match_at_name_colon(line):
             raise QAPIParseError(self._parser,
-                                 "'%s' can't follow '%s' section"
-                                 % (name, self.sections[0].name))
-        if self._is_section_tag(name):
+                                 "'@%s:' can't follow '%s' section"
+                                 % (match.group(1), self.sections[0].name))
+        if match := self._match_section_tag(line):
             # If line is "Section:   first line of description", find
             # the index of 'f', which is the indent we expect for any
             # following lines.  We then remove the leading "Section:"
@@ -719,7 +714,7 @@  def _append_various_line(self, line: str) -> None:
                 indent = 0
             else:
                 line = ' ' * indent + line
-            self._start_section(name[:-1], indent)
+            self._start_section(match.group(1), indent)
 
         self._append_freeform(line)