On Thu, May 16, 2024, 1:58 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes:
>
> > Instead of using the info object for the doc block as a whole, update
> > the info pointer for each call to ensure_untagged_section when the
> > existing section is otherwise empty. This way, Sphinx error information
> > will match precisely to where the text actually starts.
> >
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > scripts/qapi/parser.py | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> > index 8cdd5334ec6..41b9319e5cb 100644
> > --- a/scripts/qapi/parser.py
> > +++ b/scripts/qapi/parser.py
> > @@ -662,8 +662,13 @@ def end(self) -> None:
> >
> > def ensure_untagged_section(self, info: QAPISourceInfo) -> None:
> > if self.all_sections and not self.all_sections[-1].tag:
> > - # extend current section
> > - self.all_sections[-1].text += '\n'
>
> Before, we always append a newline.
>
> > + section = self.all_sections[-1]
> > + # Section is empty so far; update info to start *here*.
> > + if not section.text:
> > + section.info = info
> > + else:
> > + # extend current section
> > + self.all_sections[-1].text += '\n'
>
> Afterwards, we append it only when the section already has some text.
>
> The commit message claims the patch only adjusts section.info. That's a
> lie :)
>
Well. It wasn't intentional, so it wasn't a lie... it was just wrong :)
> I believe the change makes no difference because .end() strips leading
> and trailing newline.
>
> > return
> > # start new section
> > section = self.Section(info)
>
> You could fix the commit message, but I think backing out the
> no-difference change is easier. The appended patch works in my testing.
>
> Next one. Your patch changes the meaning of section.info. Here's its
> initialization:
>
> class Section:
> # pylint: disable=too-few-public-methods
> def __init__(self, info: QAPISourceInfo,
> tag: Optional[str] = None):
> ---> # section source info, i.e. where it begins
> self.info = info
> # section tag, if any ('Returns', '@name', ...)
> self.tag = tag
> # section text without tag
> self.text = ''
>
> The comment is now wrong. Calls for a thorough review of .info's uses.
>
Hmm... Did I really change its meaning? I guess it's debatable what "where
it begins" means. Does the tagless section start...
## <-- Here?
# Hello! <-- Or here?
##
I assert the *section* starts wherever the first line of text it contains
starts. Nothing else makes any sense.
There is value in recording where the doc block starts, but that's not a
task for the *section* info.
I don't think I understand your feedback.
> The alternative to changing .info's meaning is to add another member
> with the meaning you need. Then we have to review .info's uses to find
> out which ones to switch to the new one.
> Left for later.
>
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 8cdd5334ec..abeae1ca77 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -663,7 +663,10 @@ def end(self) -> None:
> def ensure_untagged_section(self, info: QAPISourceInfo) -> None:
> if self.all_sections and not self.all_sections[-1].tag:
> # extend current section
> - self.all_sections[-1].text += '\n'
> + section = self.all_sections[-1]
> + if not section.text:
> + section.info = info
> + section.text += '\n'
> return
> # start new section
> section = self.Section(info)
>
>
