Re: [PATCH v2 14/21] docs/qapidoc: factor out do_parse()

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> Factor out the compatibility parser helper so it can be shared by other
> directives.

Suggest "Factor out the compatibility parser helper into a base class,
so it can be shared by other directives."

>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/sphinx/qapidoc.py | 64 +++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index efcd84656fa..43dd99e21e6 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -494,7 +494,41 @@ def visit_module(self, name):
>          super().visit_module(name)
>  
>  
> -class QAPIDocDirective(Directive):
> +class NestedDirective(Directive):
> +    def run(self):
> +        raise NotImplementedError

Should this class be abstract?

> +
> +    def do_parse(self, rstlist, node):
> +        """
> +        Parse rST source lines and add them to the specified node
> +
> +        Take the list of rST source lines rstlist, parse them as
> +        rST, and add the resulting docutils nodes as children of node.
> +        The nodes are parsed in a way that allows them to include
> +        subheadings (titles) without confusing the rendering of
> +        anything else.
> +        """
> +        # This is from kerneldoc.py -- it works around an API change in
> +        # Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use
> +        # sphinx.util.nodes.nested_parse_with_titles() rather than the
> +        # 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:
> +            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
> +            )
> +            try:
> +                nested_parse_with_titles(self.state, rstlist, node)
> +            finally:
> +                self.state.memo.reporter = save
> +
> +
> +class QAPIDocDirective(NestedDirective):
>      """Extract documentation from the specified QAPI .json file"""
>  
>      required_argument = 1
> @@ -532,34 +566,6 @@ def run(self):
>              # so they are displayed nicely to the user
>              raise ExtensionError(str(err)) from err
>  
> -    def do_parse(self, rstlist, node):
> -        """Parse rST source lines and add them to the specified node
> -
> -        Take the list of rST source lines rstlist, parse them as
> -        rST, and add the resulting docutils nodes as children of node.
> -        The nodes are parsed in a way that allows them to include
> -        subheadings (titles) without confusing the rendering of
> -        anything else.
> -        """
> -        # This is from kerneldoc.py -- it works around an API change in
> -        # Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use
> -        # sphinx.util.nodes.nested_parse_with_titles() rather than the
> -        # 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:
> -            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
> -            )
> -            try:
> -                nested_parse_with_titles(self.state, rstlist, node)
> -            finally:
> -                self.state.memo.reporter = save
> -
>  
>  def setup(app):
>      """Register qapi-doc directive with Sphinx"""

Reviewed-by: Markus Armbruster <armbru@redhat.com>