On Wed, Jun 19, 2024, 2:28 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <js...@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. > Oh! I didn't realize it wasn't being used. That's certainly easier :) > > 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... ;) > Mildly. However, I do think black is "close enough" on most style issues that I have absolutely no regret or hesitation using it for all new python development. (I've been using it a lot in hobby code the last year and I find it to be remarkably helpful for my own consistency in style issues, it's indispensable for me.) So in this series, I start using it because I essentially wind up rewriting this entire file and wanted an autoformatter so I could shut my brain off for stuff like this. A "flag day" as you call it is likely coming soon to python/ where I'll start enforcing black standards. It just makes development easier to have a tool that just always DTRT. When I move QAPI there, it'll get the same treatment. > > 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 <arm...@redhat.com> > ありがとう >