Re: [PATCH v9 12/13] docs: parser_yaml.py: add support for line numbers from the parser

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Fri, 11 Jul 2025 10:51:37 +0100
Donald Hunter <donald.hunter@gmail.com> escreveu:

> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:
> 
> > Em Thu, 10 Jul 2025 15:25:20 +0100
> > Donald Hunter <donald.hunter@gmail.com> escreveu:
> >  
> >> Donald Hunter <donald.hunter@gmail.com> writes:
> >>   
> >> >>              # Parse message with RSTParser
> >> >> -            for i, line in enumerate(msg.split('\n')):
> >> >> -                result.append(line, document.current_source, i)
> >> >> +            lineoffset = 0;
> >> >> +            for line in msg.split('\n'):
> >> >> +                match = self.re_lineno.match(line)
> >> >> +                if match:
> >> >> +                    lineoffset = int(match.group(1))
> >> >> +                    continue
> >> >> +
> >> >> +                result.append(line, document.current_source, lineoffset)    
> >> >
> >> > I expect this would need to be source=document.current_source, offset=lineoffset    
> >> 
> >> Ignore that. I see it's not kwargs. It's just the issue below.
> >>   
> >> >>              rst_parser = RSTParser()
> >> >>              rst_parser.parse('\n'.join(result), document)    
> >> >
> >> > But anyway this discards any line information by just concatenating the
> >> > lines together again.    
> >> 
> >> Looks to me like there's no Parser() API that works with ViewList() so
> >> it would be necessary to directly use the docutils RSTStateMachine() for
> >> this approach to work.  
> >
> > It sounds so.
> >
> > The enclosed patch seems to address it:
> >
> > 	$ make cleandocs; make SPHINXDIRS="netlink/specs" htmldocs
> > 	...
> > 	Using alabaster theme
> > 	source directory: netlink/specs
> > 	Using Python kernel-doc
> > 	/new_devel/v4l/docs/Documentation/netlink/specs/rt-neigh.yaml:13: ERROR: Unknown directive type "bogus".
> >
> > 	.. bogus:: [docutils]
> >
> > Please notice that I added a hunk there to generate the error, just
> > to make easier to test - I'll drop it at the final version, and add
> > the proper reported-by/closes/... tags once you test it.
> >
> > Regards,
> > Mauro  
> 
> Awesome!
> 
> Tested-by: Donald Hunter <donald.hunter@gmail.com>
> 
> Patch comments below.
> 
> > [PATCH RFC] sphinx: parser_yaml.py: preserve line numbers
> >
> > Instead of converting viewlist to text, use it directly, if
> > docutils supports it.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> >
> > diff --git a/Documentation/netlink/specs/rt-neigh.yaml b/Documentation/netlink/specs/rt-neigh.yaml
> > index e9cba164e3d1..937d2563f151 100644
> > --- a/Documentation/netlink/specs/rt-neigh.yaml
> > +++ b/Documentation/netlink/specs/rt-neigh.yaml
> > @@ -11,6 +11,7 @@ doc:
> >  definitions:
> >    -
> >      name: ndmsg
> > +    doc: ".. bogus::"
> >      type: struct
> >      members:
> >        -
> > diff --git a/Documentation/sphinx/parser_yaml.py b/Documentation/sphinx/parser_yaml.py
> > index 1602b31f448e..2a2faaf759ef 100755
> > --- a/Documentation/sphinx/parser_yaml.py
> > +++ b/Documentation/sphinx/parser_yaml.py
> > @@ -11,7 +11,9 @@ import sys
> >  
> >  from pprint import pformat
> >  
> > +from docutils import nodes, statemachine  
> 
> nodes is not used

I dropped it on patch 14/13.

> 
> >  from docutils.parsers.rst import Parser as RSTParser  
> 
> This import is no longer needed

I'll drop on a next spin.
> 
> > +from docutils.parsers.rst import states
> >  from docutils.statemachine import ViewList
> >  
> >  from sphinx.util import logging
> > @@ -66,10 +68,24 @@ class YamlParser(Parser):  
> 
> I'm wondering if it makes much sense for this to inherit from Parser any
> more?

Yes. It still needs other things from the Parser class.

> >          result = ViewList()
> >  
> > +        tab_width = 8
> > +
> > +        self.state_classes = states.state_classes
> > +        self.initial_state = 'Body'
> > +
> > +        self.statemachine = states.RSTStateMachine(
> > +              state_classes=self.state_classes,
> > +              initial_state=self.initial_state,
> > +              debug=document.reporter.debug_flag)  
> 
> I don't think 'self.' is needed for any of these. They can be local to
> the method. You could just inline states.state_classes and 'Body' into
> the parameter list.

I dropped from most stuff, but self.statemachine is still needed.

I suspect that because of some other stuff inside the Parser class.

> 
> > +
> >          try:
> >              # Parse message with RSTParser  
> 
> Comment is out of date.
> 
> >              lineoffset = 0;  
> 
> Rogue semicolon

I dropped at patch 14/13.

> 
> > -            for line in msg.split('\n'):
> > +
> > +            lines = statemachine.string2lines(msg, tab_width,
> > +                                            convert_whitespace=True)
> > +
> > +            for line in lines:
> >                  match = self.re_lineno.match(line)
> >                  if match:
> >                      lineoffset = int(match.group(1))
> > @@ -77,12 +93,7 @@ class YamlParser(Parser):
> >  
> >                  result.append(line, document.current_source, lineoffset)
> >  
> > -            # Fix backward compatibility with docutils < 0.17.1
> > -            if "tab_width" not in vars(document.settings):
> > -                document.settings.tab_width = 8
> > -
> > -            rst_parser = RSTParser()
> > -            rst_parser.parse('\n'.join(result), document)
> > +            self.statemachine.run(result, document, inliner=None)
> >  
> >          except Exception as e:  
> 
> I think you could catch StateMachineError here.

Good point. will try that.

> >              document.reporter.error("YAML parsing error: %s" % pformat(e))  
> 
> Can you change this to an f"" string.

I prefer f-strings as well, but usually logger classes are recommended
to use the old way. I guess this came from a previous check with pylint.

Thanks,
Mauro