All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: qemu-devel@nongnu.org,  Peter Maydell <peter.maydell@linaro.org>,
	Michael Roth <michael.roth@amd.com>
Subject: Re: [PATCH 01/23] docs/qapidoc: support header-less freeform sections
Date: Mon, 16 Dec 2024 14:15:06 +0100	[thread overview]
Message-ID: <877c7zdb1h.fsf@pond.sub.org> (raw)
In-Reply-To: <20241213021827.2956769-2-jsnow@redhat.com> (John Snow's message of "Thu, 12 Dec 2024 21:18:04 -0500")

John Snow <jsnow@redhat.com> writes:

> The code as written can't handle if a header isn't found, because `node`
> will be uninitialized.

Yes, we initialize @node only if we have a heading.

Made me wonder what happens when we don't.  So I deleted the = from the
"# = Subsection" line in doc-good.json, and got:

    Exception occurred:
      File "/work/armbru/qemu/docs/sphinx/qapidoc.py", line 425, in freeform
        self._parse_text_into_node(text, node)
                                         ^^^^
    UnboundLocalError: cannot access local variable 'node' where it is not associated with a value

So you're fixing a crash bug, but that's perhaps less than clear from
the commit message.

>                        If we don't have a section title, create a
> generic block to insert text into instead.
>
> This patch removes a lingering pylint warning in the QAPIDoc implementation

Can you show me the warning?  My pylint doesn't...

> 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

What version of pylint?  Mine chokes on too-many-positional-arguments.

> 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(+)
>
> 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

Plausible enough (and I acked a similar fix previously, commit
2664f3176a8), but I'm a Sphinx ignoramus :)



  reply	other threads:[~2024-12-16 13:16 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-13  2:18 [PATCH 00/23] docs: add basic sphinx-domain rST generator to qapidoc John Snow
2024-12-13  2:18 ` [PATCH 01/23] docs/qapidoc: support header-less freeform sections John Snow
2024-12-16 13:15   ` Markus Armbruster [this message]
2025-01-13 19:12     ` John Snow
2025-01-14  9:19       ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 02/23] qapi/parser: adjust info location for doc body section John Snow
2024-12-13  2:18 ` [PATCH 03/23] docs/qapidoc: remove example section support John Snow
2024-12-18 12:27   ` Markus Armbruster
2024-12-18 15:15     ` John Snow
2025-01-09  7:50       ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 04/23] qapi: expand tags to all doc sections John Snow
2024-12-18 10:58   ` Markus Armbruster
2024-12-18 15:14     ` John Snow
2025-01-09  7:51       ` Markus Armbruster
2024-12-20 13:13   ` Markus Armbruster
2025-01-09 18:49     ` John Snow
2025-01-10  7:33       ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 05/23] qapi/schema: add __repr__ to QAPIDoc.Section John Snow
2024-12-13  2:18 ` [PATCH 06/23] docs/qapidoc: add transmogrifier stub John Snow
2024-12-13  2:18 ` [PATCH 07/23] docs/qapidoc: add transmogrifier class stub John Snow
2024-12-13  2:18 ` [PATCH 08/23] docs/qapidoc: add visit_module() method John Snow
2024-12-13  2:18 ` [PATCH 09/23] qapi/source: allow multi-line QAPISourceInfo advancing John Snow
2024-12-20 13:22   ` Markus Armbruster
2025-01-08 21:18     ` John Snow
2025-01-09  8:00       ` Markus Armbruster
2025-01-09 17:19         ` John Snow
2025-01-10  7:37           ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 10/23] docs/qapidoc: add visit_freeform() method John Snow
2024-12-20 13:25   ` Markus Armbruster
2025-01-13 20:14     ` John Snow
2024-12-13  2:18 ` [PATCH 11/23] docs/qapidoc: add preamble() method John Snow
2024-12-20 14:15   ` Markus Armbruster
2025-01-08 22:58     ` John Snow
2025-01-09 10:34       ` Markus Armbruster
2025-01-09 18:04         ` John Snow
2025-01-10 12:19           ` Markus Armbruster
2025-01-13 20:53             ` John Snow
2025-01-14  9:30               ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 12/23] docs/qapidoc: add visit_paragraph() method John Snow
2024-12-13  2:18 ` [PATCH 13/23] docs/qapidoc: add visit_errors() method John Snow
2024-12-13  2:18 ` [PATCH 14/23] docs/qapidoc: add format_type() method John Snow
2024-12-13  2:18 ` [PATCH 15/23] docs/qapidoc: add add_field() and generate_field() helper methods John Snow
2024-12-13  2:18 ` [PATCH 16/23] docs/qapidoc: add visit_feature() method John Snow
2024-12-20 14:21   ` Markus Armbruster
2025-01-08 22:46     ` John Snow
2024-12-13  2:18 ` [PATCH 17/23] docs/qapidoc: record current documented entity in transmogrifier John Snow
2024-12-20 14:23   ` Markus Armbruster
2025-01-08 21:11     ` John Snow
2025-01-09 10:35       ` Markus Armbruster
2024-12-13  2:18 ` [PATCH 18/23] docs/qapidoc: add visit_returns() method John Snow
2024-12-13  2:18 ` [PATCH 19/23] docs/qapidoc: add visit_member() method John Snow
2024-12-13  2:18 ` [PATCH 20/23] docs/qapidoc: add visit_sections() method John Snow
2024-12-13  2:18 ` [PATCH 21/23] docs/qapidoc: add visit_entity() John Snow
2024-12-13  2:18 ` [PATCH 22/23] docs/qapidoc: implement transmogrify() method John Snow
2024-12-13  2:18 ` [PATCH 23/23] docs/qapidoc: add transmogrifier test document John Snow
2024-12-19 12:31 ` [PATCH 00/23] docs: add basic sphinx-domain rST generator to qapidoc Markus Armbruster
2025-01-08 21:08   ` John Snow
2025-01-09 11:48     ` Markus Armbruster
2025-01-09 17:26       ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877c7zdb1h.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.