Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
qapi: Relax doc string @name: description indentation rules
The QAPI schema doc comment language provides special syntax for
command and event arguments, struct and union members, alternate
branches, enumeration values, and features: descriptions starting with
"@name:".

By convention, we format them like this:

    # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit,
    #        sed do eiusmod tempor incididunt ut labore et dolore
    #        magna aliqua.

Okay for names as short as "name", but we have much longer ones.  Their
description gets squeezed against the right margin, like this:

    # @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization could
    #                               not avoid copying dirty pages. This is between
    #                               0 and @dirty-sync-count * @multifd-channels.
    #                               (since 7.1)

The description text is effectively just 50 characters wide.  Easy
enough to read, but can be cumbersome to write.

The awkward squeeze against the right margin makes people go beyond it,
which produces two undesirables: arguments about style, and descriptions
that are unnecessarily hard to read, like this one:

    # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.  This is
    #                           only present when the postcopy-blocktime migration capability
    #                           is enabled. (Since 3.0)

We could instead format it like

    # @postcopy-vcpu-blocktime:
    # list of the postcopy blocktime per vCPU.  This is only present
    # when the postcopy-blocktime migration capability is
    # enabled. (Since 3.0)

or, since the commit before previous, like

    # @postcopy-vcpu-blocktime:
    # 	  list of the postcopy blocktime per vCPU.  This is only present
    # 	  when the postcopy-blocktime migration capability is
    # 	  enabled. (Since 3.0)

However, I'd rather have

    # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU.
    #     This is only present when the postcopy-blocktime migration
    #     capability is enabled.  (Since 3.0)

because this is how rST field and option lists work.

To get this, we need to let the first non-blank line after the
"@name:" line determine expected indentation.

This fills up the indentation pitfall mentioned in
docs/devel/qapi-code-gen.rst.  A related pitfall still exists.  Update
the text to show it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230428105429.1687850-14-armbru@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
[Work around lack of walrus operator in Python 3.7 and older]
  • Loading branch information
Markus Armbruster committed May 10, 2023
1 parent 3e32dca commit 0834978
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 62 deletions.
10 changes: 7 additions & 3 deletions docs/devel/qapi-code-gen.rst
Expand Up @@ -1074,10 +1074,14 @@ Indentation matters. Bad example::
# @none: None (no memory side cache in this proximity domain,
# or cache associativity unknown)
# (since 5.0)
The description is parsed as a definition list with term "None (no
memory side cache in this proximity domain," and definition "or cache
associativity unknown)".
The last line's de-indent is wrong. The second and subsequent lines
need to line up with each other, like this::
# @none: None (no memory side cache in this proximity domain,
# or cache associativity unknown)
# (since 5.0)
Section tags are case-sensitive and end with a colon. Good example::
Expand Down
73 changes: 18 additions & 55 deletions scripts/qapi/parser.py
Expand Up @@ -474,25 +474,29 @@ def __init__(self, parser: QAPISchemaParser,
self._parser = parser
# optional section name (argument/member or section name)
self.name = name
# section text without section name
self.text = ''
# the expected indent level of the text of this section
self._indent = indent
# indentation to strip (None means indeterminate)
self._indent = None if self.name else 0

def append(self, line: str) -> None:
# Strip leading spaces corresponding to the expected indent level
# Blank lines are always OK.
line = line.rstrip()

if line:
indent = must_match(r'\s*', line).end()
if self._indent < 0:
self._indent = indent
if self._indent is None:
# indeterminate indentation
if self.text != '':
# non-blank, non-first line determines indentation
self._indent = indent
elif indent < self._indent:
raise QAPIParseError(
self._parser,
"unexpected de-indent (expected at least %d spaces)" %
self._indent)
line = line[self._indent:]

self.text += line.rstrip() + '\n'
self.text += line + '\n'

class ArgSection(Section):
def __init__(self, parser: QAPISchemaParser,
Expand Down Expand Up @@ -622,22 +626,8 @@ def _append_args_line(self, line: str) -> None:
"""
match = self._match_at_name_colon(line)
if match:
# 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.
name = match.group(1)
indent = match.end()
line = line[indent:]
if not line:
# Line was just the "@arg:" header
# The next non-blank line determines expected indent
indent = -1
else:
line = ' ' * indent + line
self._start_args_section(name, indent)
line = line[match.end():]
self._start_args_section(match.group(1), 0)
elif self._match_section_tag(line):
self._append_line = self._append_various_line
self._append_various_line(line)
Expand All @@ -657,22 +647,8 @@ def _append_args_line(self, line: str) -> None:
def _append_features_line(self, line: str) -> None:
match = self._match_at_name_colon(line)
if match:
# 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.
name = match.group(1)
indent = match.end()
line = line[indent:]
if not line:
# Line was just the "@arg:" header
# The next non-blank line determines expected indent
indent = -1
else:
line = ' ' * indent + line
self._start_features_section(name, indent)
line = line[match.end():]
self._start_features_section(match.group(1), 0)
elif self._match_section_tag(line):
self._append_line = self._append_various_line
self._append_various_line(line)
Expand Down Expand Up @@ -704,21 +680,8 @@ def _append_various_line(self, line: str) -> None:
% (match.group(1), self.sections[0].name))
match = self._match_section_tag(line)
if match:
# 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:"
# 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()
line = line[indent:]
if not line:
# Line was just the "Section:" header
# The next non-blank line determines expected indent
indent = 0
else:
line = ' ' * indent + line
self._start_section(match.group(1), indent)
line = line[match.end():]
self._start_section(match.group(1), 0)

self._append_freeform(line)

Expand Down Expand Up @@ -754,7 +717,7 @@ def _start_section(self, name: Optional[str] = None,
self.sections.append(new_section)

def _switch_section(self, new_section: 'QAPIDoc.Section') -> None:
text = self._section.text = self._section.text.strip()
text = self._section.text = self._section.text.strip('\n')

# Only the 'body' section is allowed to have an empty body.
# All other sections, including anonymous ones, must have text.
Expand Down
2 changes: 1 addition & 1 deletion tests/qapi-schema/doc-bad-indent.err
@@ -1 +1 @@
doc-bad-indent.json:6:1: unexpected de-indent (expected at least 4 spaces)
doc-bad-indent.json:7:1: unexpected de-indent (expected at least 2 spaces)
3 changes: 2 additions & 1 deletion tests/qapi-schema/doc-bad-indent.json
Expand Up @@ -3,6 +3,7 @@
##
# @foo:
# @a: line one
# line two is wrongly indented
# line two
# line three is wrongly indented
##
{ 'command': 'foo', 'data': { 'a': 'int' } }
3 changes: 2 additions & 1 deletion tests/qapi-schema/doc-good.json
Expand Up @@ -144,7 +144,8 @@
# description starts on a new line,
# indented
#
# @arg2: the second argument
# @arg2: description starts on the same line
# remainder indented differently
#
# Features:
# @cmd-feat1: a feature
Expand Down
3 changes: 2 additions & 1 deletion tests/qapi-schema/doc-good.out
Expand Up @@ -161,7 +161,8 @@ doc symbol=cmd
description starts on a new line,
indented
arg=arg2
the second argument
description starts on the same line
remainder indented differently
arg=arg3

feature=cmd-feat1
Expand Down

0 comments on commit 0834978

Please sign in to comment.