Re: [PATCH 03/13] docs/qapidoc: delint a tiny portion of the module

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> In a forthcoming series that adds a new QMP documentation generator, it
> will be helpful to have a linting baseline. However, there's no need to
> shuffle around the deck chairs too much, because most of this code will
> be removed once that new qapidoc generator (the "transmogrifier") is in
> place.
>
> To ease my pain: just turn off the black auto-formatter for most, but
> not all, of qapidoc.py. This will help ensure that *new* code follows a
> coding standard without bothering too much with cleaning up the existing
> code.
>
> Code that I intend to keep is still subject to the delinting beam.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/sphinx/qapidoc.py | 66 +++++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index f270b494f01..e675966defa 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -28,33 +28,42 @@
>  import re
>  
>  from docutils import nodes
> +from docutils.parsers.rst import Directive, directives
>  from docutils.statemachine import ViewList
> -from docutils.parsers.rst import directives, Directive
> -from sphinx.errors import ExtensionError
> -from sphinx.util.nodes import nested_parse_with_titles
> -import sphinx
> -from qapi.gen import QAPISchemaVisitor
>  from qapi.error import QAPIError, QAPISemError
> +from qapi.gen import QAPISchemaVisitor
>  from qapi.schema import QAPISchema
>  
> +import sphinx
> +from sphinx.errors import ExtensionError
> +from sphinx.util.nodes import nested_parse_with_titles
> +
>  
>  # Sphinx up to 1.6 uses AutodocReporter; 1.7 and later
>  # use switch_source_input. Check borrowed from kerneldoc.py.
> -Use_SSI = sphinx.__version__[:3] >= '1.7'
> -if Use_SSI:
> +USE_SSI = sphinx.__version__[:3] >= "1.7"
> +if USE_SSI:
>      from sphinx.util.docutils import switch_source_input
>  else:
> -    from sphinx.ext.autodoc import AutodocReporter
> +    from sphinx.ext.autodoc import (  # pylint: disable=no-name-in-module
> +        AutodocReporter,
> +    )
>  
>  
> -__version__ = '1.0'
> +__version__ = "1.0"
>  
>  
> +# Disable black auto-formatter until re-enabled:
> +# fmt: off
> +
>  # Function borrowed from pydash, which is under the MIT license
>  def intersperse(iterable, separator):
>      """Yield the members of *iterable* interspersed with *separator*."""
>      iterable = iter(iterable)
> -    yield next(iterable)
> +    try:
> +        yield next(iterable)
> +    except StopIteration:
> +        return

This gets rid of pylint's

    docs/sphinx/qapidoc.py:82:10: R1708: Do not raise StopIteration in generator, use return statement instead (stop-iteration-return)

I considered the same change some time ago, and decided against it to
avoid deviating from pydash.  StopIteration would be a programming error
here.

Please *delete* the function instead: commit fd62bff901b removed its
last use.  I'd do it in a separate commit, but that's up to you.

>      for item in iterable:
>          yield separator
>          yield item
> @@ -451,6 +460,10 @@ def get_document_nodes(self):
>          return self._top_node.children
>  
>  
> +# Turn the black formatter on for the rest of the file.
> +# fmt: on
> +
> +
>  class QAPISchemaGenDepVisitor(QAPISchemaVisitor):
>      """A QAPI schema visitor which adds Sphinx dependencies each module
>  
> @@ -458,34 +471,34 @@ class QAPISchemaGenDepVisitor(QAPISchemaVisitor):
>      that the generated documentation output depends on the input
>      schema file associated with each module in the QAPI input.
>      """
> +
>      def __init__(self, env, qapidir):
>          self._env = env
>          self._qapidir = qapidir
>  
>      def visit_module(self, name):
>          if name != "./builtin":
> -            qapifile = self._qapidir + '/' + name
> +            qapifile = self._qapidir + "/" + name

The string literal quote changes are mildly annoying.  But since by your
good work you're effectively appointing yourself maintainer of this
file...  ;)

>              self._env.note_dependency(os.path.abspath(qapifile))
>          super().visit_module(name)
>  
>  
>  class QAPIDocDirective(Directive):
>      """Extract documentation from the specified QAPI .json file"""
> +
>      required_argument = 1
>      optional_arguments = 1
> -    option_spec = {
> -        'qapifile': directives.unchanged_required
> -    }
> +    option_spec = {"qapifile": directives.unchanged_required}
>      has_content = False
>  
>      def new_serialno(self):
>          """Return a unique new ID string suitable for use as a node's ID"""
>          env = self.state.document.settings.env
> -        return 'qapidoc-%d' % env.new_serialno('qapidoc')
> +        return "qapidoc-%d" % env.new_serialno("qapidoc")
>  
>      def run(self):
>          env = self.state.document.settings.env
> -        qapifile = env.config.qapidoc_srctree + '/' + self.arguments[0]
> +        qapifile = env.config.qapidoc_srctree + "/" + self.arguments[0]
>          qapidir = os.path.dirname(qapifile)
>  
>          try:
> @@ -523,13 +536,14 @@ def do_parse(self, rstlist, node):
>          # plain self.state.nested_parse(), and so we can drop the saving
>          # of title_styles and section_level that kerneldoc.py does,
>          # because nested_parse_with_titles() does that for us.
> -        if Use_SSI:
> +        if USE_SSI:
>              with switch_source_input(self.state, rstlist):
>                  nested_parse_with_titles(self.state, rstlist, node)
>          else:
>              save = self.state.memo.reporter
>              self.state.memo.reporter = AutodocReporter(
> -                rstlist, self.state.memo.reporter)
> +                rstlist, self.state.memo.reporter
> +            )
>              try:
>                  nested_parse_with_titles(self.state, rstlist, node)
>              finally:
> @@ -537,12 +551,12 @@ def do_parse(self, rstlist, node):
>  
>  
>  def setup(app):
> -    """ Register qapi-doc directive with Sphinx"""
> -    app.add_config_value('qapidoc_srctree', None, 'env')
> -    app.add_directive('qapi-doc', QAPIDocDirective)
> +    """Register qapi-doc directive with Sphinx"""
> +    app.add_config_value("qapidoc_srctree", None, "env")
> +    app.add_directive("qapi-doc", QAPIDocDirective)
>  
> -    return dict(
> -        version=__version__,
> -        parallel_read_safe=True,
> -        parallel_write_safe=True
> -    )
> +    return {
> +        "version": __version__,
> +        "parallel_read_safe": True,
> +        "parallel_write_safe": True,
> +    }

With intersperse() deleted
Reviewed-by: Markus Armbruster <armbru@redhat.com>