Re: [PATCH v7 07/17] docs: sphinx: add a parser for yaml files for Netlink specs

Donald Hunter <[email protected]>
Newsgroups dev.linux.lists.lkmm,dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
Mauro Carvalho Chehab <[email protected]> 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 <[email protected]>

Looks like you didn't address my comments from v5:

    > > +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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.