John Snow <jsnow@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 <jsnow@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)