| Message ID | 20250205231208.1480762-2-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | docs: add sphinx-domain rST generator to qapidoc | expand |
John Snow <jsnow@redhat.com> writes: > The code as written can't handle if a header isn't found and will crash, > because `node` will be uninitialized. If we don't have a section title, > create a generic block to insert text into instead. Suggest to show input that makes it crash. Something like The code as written crashes when a free-form documentation block doesn't start with a heading or subheading, like so ## # Just text, no heading. ## The code then uses `node` uninitialized. To fix, create a generic block to insert the doc text into. > (This patch also removes a lingering pylint warning in the QAPIDoc > implementation that prevents getting a clean baseline to use for > forthcoming additions.) > > I am not attempting to *fully* clean up the existing QAPIDoc > implementation in pylint because I intend to delete it anyway; this > patch merely accomplishes a baseline under a specific pylint > configuration: > > PYTHONPATH=../../scripts/ pylint --disable=fixme,too-many-lines,\ > consider-using-f-string,missing-docstring,unused-argument,\ > too-many-arguments,too-many-positional-arguments,\ > too-many-public-methods \ > qapidoc.py > > (under at least pylint 3.3.1; more robust tamping down of the > environment needed to consistently perform checks will happen later - > hopefully soon, sorry for the inconvenience.) > > This at least ensures there aren't regressions outside of these general > warnings in the new qapidoc.py code to be committed. > > Signed-off-by: John Snow <jsnow@redhat.com> Fixes: 43e0d14ee09a (docs/sphinx: fix extra stuff in TOC after freeform QMP sections)
On Tue, Feb 11, 2025, 9:51 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > The code as written can't handle if a header isn't found and will crash, > > because `node` will be uninitialized. If we don't have a section title, > > create a generic block to insert text into instead. > > Suggest to show input that makes it crash. Something like > > The code as written crashes when a free-form documentation block > doesn't start with a heading or subheading, like so > > ## > # Just text, no heading. > ## > > The code then uses `node` uninitialized. > > To fix, create a generic block to insert the doc text into. > Okeydokey. Simple enough, thanks. > > (This patch also removes a lingering pylint warning in the QAPIDoc > > implementation that prevents getting a clean baseline to use for > > forthcoming additions.) > > > > I am not attempting to *fully* clean up the existing QAPIDoc > > implementation in pylint because I intend to delete it anyway; this > > patch merely accomplishes a baseline under a specific pylint > > configuration: > > > > PYTHONPATH=../../scripts/ pylint --disable=fixme,too-many-lines,\ > > consider-using-f-string,missing-docstring,unused-argument,\ > > too-many-arguments,too-many-positional-arguments,\ > > too-many-public-methods \ > > qapidoc.py > > > > (under at least pylint 3.3.1; more robust tamping down of the > > environment needed to consistently perform checks will happen later - > > hopefully soon, sorry for the inconvenience.) > > > > This at least ensures there aren't regressions outside of these general > > warnings in the new qapidoc.py code to be committed. > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > Fixes: 43e0d14ee09a (docs/sphinx: fix extra stuff in TOC after freeform > QMP sections) > Ah, yep. You got it. Thanks for reviewing this mammoth project. >
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index 5f96b46270b..5a4d7388b29 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -421,6 +421,8 @@ def freeform(self, doc): node = self._start_new_heading(heading, len(leader)) if text == '': return + else: + node = nodes.container() self._parse_text_into_node(text, node) self._cur_doc = None
The code as written can't handle if a header isn't found and will crash, because `node` will be uninitialized. If we don't have a section title, create a generic block to insert text into instead. (This patch also removes a lingering pylint warning in the QAPIDoc implementation that prevents getting a clean baseline to use for forthcoming additions.) I am not attempting to *fully* clean up the existing QAPIDoc implementation in pylint because I intend to delete it anyway; this patch merely accomplishes a baseline under a specific pylint configuration: PYTHONPATH=../../scripts/ pylint --disable=fixme,too-many-lines,\ consider-using-f-string,missing-docstring,unused-argument,\ too-many-arguments,too-many-positional-arguments,\ too-many-public-methods \ qapidoc.py (under at least pylint 3.3.1; more robust tamping down of the environment needed to consistently perform checks will happen later - hopefully soon, sorry for the inconvenience.) This at least ensures there aren't regressions outside of these general warnings in the new qapidoc.py code to be committed. Signed-off-by: John Snow <jsnow@redhat.com> --- docs/sphinx/qapidoc.py | 2 ++ 1 file changed, 2 insertions(+)