Message-ID: <20250617154049.104ef6ff@sal.lan>
Date: Tue, 17 Jun 2025 15:40:49 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Donald Hunter <donald.hunter@...il.com>
Cc: Linux Doc Mailing List <linux-doc@...r.kernel.org>, Jonathan Corbet
 <corbet@....net>, "Akira Yokosawa" <akiyks@...il.com>, "Breno Leitao"
 <leitao@...ian.org>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, "Ignacio Encinas Rubio"
 <ignacio@...cinas.com>, "Jan Stancek" <jstancek@...hat.com>, "Marco Elver"
 <elver@...gle.com>, "Paolo Abeni" <pabeni@...hat.com>, "Ruben Wauters"
 <rubenru09@....com>, "Shuah Khan" <skhan@...uxfoundation.org>,
 joel@...lfernandes.org, linux-kernel-mentees@...ts.linux.dev,
 linux-kernel@...r.kernel.org, lkmm@...ts.linux.dev, netdev@...r.kernel.org,
 peterz@...radead.org, stern@...land.harvard.edu
Subject: Re: [PATCH v5 10/15] docs: sphinx: add a parser for yaml files for
 Netlink specs

Em Tue, 17 Jun 2025 13:35:50 +0100
Donald Hunter <donald.hunter@...il.com> escreveu:

> Mauro Carvalho Chehab <mchehab+huawei@...nel.org> writes:
> 
> > Add a simple sphinx.Parser to handle yaml files and add the
> > the code to handle Netlink specs. All other yaml files are
> > ignored.
> >
> > The code was written in a way that parsing yaml for different
> > subsystems and even for different parts of Netlink are easy.
> >
> > All it takes to have a different parser is to add an
> > import line similar to:
> >
> > 	from netlink_yml_parser import YnlDocGenerator
> >
> > adding the corresponding parser somewhere at the extension:
> >
> > 	netlink_parser = YnlDocGenerator()
> >
> > And then add a logic inside parse() to handle different
> > doc outputs, depending on the file location, similar to:
> >
> >         if "/netlink/specs/" in fname:
> >             msg = self.netlink_parser.parse_yaml_file(fname)
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
> > ---
> >  Documentation/sphinx/parser_yaml.py | 76 +++++++++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> >  create mode 100755 Documentation/sphinx/parser_yaml.py
> >
> > diff --git a/Documentation/sphinx/parser_yaml.py b/Documentation/sphinx/parser_yaml.py
> > new file mode 100755
> > index 000000000000..635945e1c5ba
> > --- /dev/null
> > +++ b/Documentation/sphinx/parser_yaml.py
> > @@ -0,0 +1,76 @@
> > +"""
> > +Sphinx extension for processing YAML files
> > +"""
> > +
> > +import os
> > +import re
> > +import sys
> > +
> > +from pprint import pformat
> > +
> > +from docutils.parsers.rst import Parser as RSTParser
> > +from docutils.statemachine import ViewList
> > +
> > +from sphinx.util import logging
> > +from sphinx.parsers import Parser
> > +
> > +srctree = os.path.abspath(os.environ["srctree"])
> > +sys.path.insert(0, os.path.join(srctree, "tools/net/ynl/pyynl"))
> > +
> > +from netlink_yml_parser import YnlDocGenerator        # pylint: disable=C0413
> > +
> > +logger = logging.getLogger(__name__)
> > +
> > +class YamlParser(Parser):
> > +    """Custom parser for YAML files."""  
> 
> Would be good to say that this is a common YAML parser that calls
> different subsystems, e.g. how you described it in the commit message.

Makes sense. Will fix at the next version.

> 
> > +
> > +    # Need at least two elements on this set  
> 
> I think you can drop this comment. It's not that it must be two
> elements, it's that supported needs to be a list and the python syntax
> to force parsing as a list would be ('item', )

Ah, ok.

> > +    supported = ('yaml', 'yml')
> > +
> > +    netlink_parser = YnlDocGenerator()
> > +
> > +    def do_parse(self, inputstring, document, msg):  
> 
> Maybe a better name for this is parse_rst?

Ok.

> 
> > +        """Parse YAML and generate a document tree."""  
> 
> Also update comment.

Ok.

> > +
> > +        self.setup_parse(inputstring, document)
> > +
> > +        result = ViewList()
> > +
> > +        try:
> > +            # Parse message with RSTParser
> > +            for i, line in enumerate(msg.split('\n')):
> > +                result.append(line, document.current_source, i)  
> 
> This has the effect of associating line numbers from the generated ReST
> with the source .yaml file, right? So errors will be reported against
> the wrong place in the file. Is there any way to show the cause of the
> error in the intermediate ReST?

Yes, but this will require modifying the parser. I prefer merging this
series without such change, and then having a separate changeset
addressing it.

There are two ways we can do that:

1. The parser can add a ReST comment with the line number. This
   is what it is done by kerneldoc.py Sphinx extension:

	lineoffset = 0
	line_regex = re.compile(r"^\.\. LINENO ([0-9]+)$")
        for line in lines:
            match = line_regex.search(line)
            if match:
                lineoffset = int(match.group(1)) - 1 # sphinx counts lines from 0
            else:
                doc = str(env.srcdir) + "/" + env.docname + ":" + str(self.lineno)
                result.append(line, doc + ": " + filename, lineoffset)
                lineoffset += 1

   I kept the same way after its conversion to Python, as right now,
   it supports both a Python class and a command lin command. I may
   eventually clean it up in the future.

2. making the parser return a tuple. At kernel_abi.py, as the parser
   returns content from multiple files, such tuple is:

		 (rst_output, filename, line_number)

   and the code for it is (cleaned up):

	for msg, f, ln in kernel_abi.doc(show_file=show_file,
                                         show_symbols=show_symbols,
                                         filter_path=abi_type):

            lines = statemachine.string2lines(msg, tab_width,
                                              convert_whitespace=True)

            for line in lines:
                content.append(line, f, ln - 1) # sphinx counts lines from 0

(2) is cleaner and faster, but (1) is easier to implement on an 
already-existing code.

> As an example if I modify tc.yaml like this:
> 
> diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
> index 4cc1f6a45001..c36d86d2dc72 100644
> --- a/Documentation/netlink/specs/tc.yaml
> +++ b/Documentation/netlink/specs/tc.yaml
> @@ -4044,7 +4044,9 @@ operations:
>              - chain
>      -
>        name: getchain
> -      doc: Get / dump tc chain information.
> +      doc: |
> +        Get / dump tc chain information.
> +        .. bogus-directive:: 
>        attribute-set: attrs
>        fixed-header: tcmsg
>        do:
> 
> This is the resuting error which will be really hard to track down:
> 
> /home/donaldh/net-next/Documentation/netlink/specs/tc.yaml:216: ERROR: Unexpected indentation. [docutils]
> 
> > +
> > +            rst_parser = RSTParser()
> > +            rst_parser.parse('\n'.join(result), document)
> > +
> > +        except Exception as e:
> > +            document.reporter.error("YAML parsing error: %s" % pformat(e))
> > +
> > +        self.finish_parse()
> > +
> > +    # Overrides docutils.parsers.Parser. See sphinx.parsers.RSTParser
> > +    def parse(self, inputstring, document):
> > +        """Check if a YAML is meant to be parsed."""
> > +
> > +        fname = document.current_source
> > +
> > +        # Handle netlink yaml specs
> > +        if "/netlink/specs/" in fname:
> > +            msg = self.netlink_parser.parse_yaml_file(fname)
> > +            self.do_parse(inputstring, document, msg)
> > +
> > +        # All other yaml files are ignored
> > +
> > +def setup(app):
> > +    """Setup function for the Sphinx extension."""
> > +
> > +    # Add YAML parser
> > +    app.add_source_parser(YamlParser)
> > +    app.add_source_suffix('.yaml', 'yaml')
> > +
> > +    return {
> > +        'version': '1.0',
> > +        'parallel_read_safe': True,
> > +        'parallel_write_safe': True,
> > +    }  

Powered by blists - more mailing lists