From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A73DC2BD09 for ; Fri, 28 Jun 2024 15:17:08 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sNDLC-0002GI-Rp; Fri, 28 Jun 2024 11:16:46 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sNDL9-0002FT-28 for qemu-devel@nongnu.org; Fri, 28 Jun 2024 11:16:43 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sNDKt-0007Fa-Mo for qemu-devel@nongnu.org; Fri, 28 Jun 2024 11:16:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1719587786; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2iLfQdunI+W6b861bI9ZhihlIYR3+t0330Br9FNKzT8=; b=CG0u/TnIAHUUNyf+I3FRw7yMIQXjmefKP8g3yA9F+F9/WiDpKe1DZm2sFCZx8W8FVX479x 20iFSKCM0tpuELspu43L0fO68k3A9j7dwGajRSq50qL2JyHSioegsRXaNV8gKgyAMcusVA SbKDULAd916Nxr0A0iHuqI+vj3PlowY= Received: from mail-pg1-f199.google.com (mail-pg1-f199.google.com [209.85.215.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-650-MNF7FI5UPC-hWl5-4JoPkQ-1; Fri, 28 Jun 2024 11:16:21 -0400 X-MC-Unique: MNF7FI5UPC-hWl5-4JoPkQ-1 Received: by mail-pg1-f199.google.com with SMTP id 41be03b00d2f7-70ab3bc4a69so647345a12.3 for ; Fri, 28 Jun 2024 08:16:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719587780; x=1720192580; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=2iLfQdunI+W6b861bI9ZhihlIYR3+t0330Br9FNKzT8=; b=D1ACPwynBSFHZ4RKiZ1kSMcQXAfec8UoSdfXzdspWEIgVUM8QPsFRVmZy0ROxzULst /4/3f4KE8m8J/rXVX22lEjUsWeruG0qw+A5Bj6qWpHc0Ke4zoutP4/gr4GSfTpl5U7U0 7vqZ8nRt4GvjQ8f0ZbunzVKHeXNOer/gvTJ9UYQLgU9HM3rFsTJLda7MPWW2WNDWYmkl YmEQj13UMBiG961BVQjQ/DuhMDAlTrgkHqjq/cUZUbneR8axUSeNRihDuHh9dugFcuCv Fh/BxLvHsujVZdmBoMa8t8aSrDeoy2bVtX2bAIoEItAr8B2wJuTGDJbYCaQo9eomqYtA xGfw== X-Gm-Message-State: AOJu0YxbMVlYRcp2rFW37LON05IrvPTOFOtzM9xkmJsMa/+c53wEWobs bGVNZcKWe/roo/73jucYc0djLIp7MGAS4hI3rdMXJ/HYzYxMZlBLbqH+v/9FY7uqUgz2WN6RR/8 jzxDi7rW0RWJDLS8L9NAWZKnxtVImYOcMn1Uhq1BKaYaICXD0ULbtjk3PitdTg8TAiC+jKBww7+ 2QGgSnaszsNXgUHziN211XSbVLWMs= X-Received: by 2002:a05:6a20:3c86:b0:1be:da68:1f26 with SMTP id adf61e73a8af0-1beda6820b8mr5693766637.59.1719587780398; Fri, 28 Jun 2024 08:16:20 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGLzUJ5EWePs5mtK7heg1VlwDNNF2OB1DHGdJfcL7favy6lsultVLTXFSfUgJprCqi3pUBZSh77Um+GsjxQa9o= X-Received: by 2002:a05:6a20:3c86:b0:1be:da68:1f26 with SMTP id adf61e73a8af0-1beda6820b8mr5693726637.59.1719587779864; Fri, 28 Jun 2024 08:16:19 -0700 (PDT) MIME-Version: 1.0 References: <20240626222128.406106-1-jsnow@redhat.com> <20240626222128.406106-15-jsnow@redhat.com> <87tthdxli7.fsf@pond.sub.org> In-Reply-To: <87tthdxli7.fsf@pond.sub.org> From: John Snow Date: Fri, 28 Jun 2024 11:16:08 -0400 Message-ID: Subject: Re: [PATCH v2 14/21] docs/qapidoc: factor out do_parse() To: Markus Armbruster Cc: qemu-devel , Mads Ynddal , Jiri Pirko , Stefan Hajnoczi , Eric Blake , Peter Maydell , Michael Roth , "Michael S. Tsirkin" , Alex Williamson , Pavel Dovgalyuk , Victor Toso de Carvalho , =?UTF-8?Q?C=C3=A9dric_Le_Goater?= , =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= , Qemu-block , Ani Sinha , Fabiano Rosas , Marcel Apfelbaum , =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= , Gerd Hoffmann , Paolo Bonzini , Kevin Wolf , Peter Xu , Eduardo Habkost , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Lukas Straub , Igor Mammedov , Jason Wang , Yanan Wang , Hanna Reitz , Konstantin Kostiuk Content-Type: multipart/alternative; boundary="000000000000c78aef061bf4badd" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -22 X-Spam_score: -2.3 X-Spam_bar: -- X-Spam_report: (-2.3 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.206, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --000000000000c78aef061bf4badd Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jun 28, 2024, 9:09=E2=80=AFAM Markus Armbruster = wrote: > John Snow writes: > > > Factor out the compatibility parser helper so it can be shared by other > > directives. > > Suggest "Factor out the compatibility parser helper into a base class, > so it can be shared by other directives." Sure. Haven't read the other mails yet. I'll make the change if you want a v3, otherwise feel free to edit. > > > > Signed-off-by: John Snow > > --- > > docs/sphinx/qapidoc.py | 64 +++++++++++++++++++++++------------------- > > 1 file changed, 35 insertions(+), 29 deletions(-) > > > > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py > > index efcd84656fa..43dd99e21e6 100644 > > --- a/docs/sphinx/qapidoc.py > > +++ b/docs/sphinx/qapidoc.py > > @@ -494,7 +494,41 @@ def visit_module(self, name): > > super().visit_module(name) > > > > > > -class QAPIDocDirective(Directive): > > +class NestedDirective(Directive): > > + def run(self): > > + raise NotImplementedError > > Should this class be abstract? > It could be ... *sneezes* I plan to delete it by the end of the qapi-domain series anyway, or perhaps I could even delete it *before* with a dedicated "require sphinx >=3D 3.x" miniseries. Actually, that's probably a really good idea... > > + > > + def do_parse(self, rstlist, node): > > + """ > > + Parse rST source lines and add them to the specified node > > + > > + Take the list of rST source lines rstlist, parse them as > > + rST, and add the resulting docutils nodes as children of node. > > + The nodes are parsed in a way that allows them to include > > + subheadings (titles) without confusing the rendering of > > + anything else. > > + """ > > + # This is from kerneldoc.py -- it works around an API change i= n > > + # Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use > > + # sphinx.util.nodes.nested_parse_with_titles() rather than the > > + # plain self.state.nested_parse(), and so we can drop the savi= ng > > + # of title_styles and section_level that kerneldoc.py does, > > + # because nested_parse_with_titles() does that for us. > > + if USE_SSI: > > + with switch_source_input(self.state, rstlist): > > + nested_parse_with_titles(self.state, rstlist, node) > > + else: > > + save =3D self.state.memo.reporter > > + self.state.memo.reporter =3D AutodocReporter( > > + rstlist, self.state.memo.reporter > > + ) > > + try: > > + nested_parse_with_titles(self.state, rstlist, node) > > + finally: > > + self.state.memo.reporter =3D save > > + > > + > > +class QAPIDocDirective(NestedDirective): > > """Extract documentation from the specified QAPI .json file""" > > > > required_argument =3D 1 > > @@ -532,34 +566,6 @@ def run(self): > > # so they are displayed nicely to the user > > raise ExtensionError(str(err)) from err > > > > - def do_parse(self, rstlist, node): > > - """Parse rST source lines and add them to the specified node > > - > > - Take the list of rST source lines rstlist, parse them as > > - rST, and add the resulting docutils nodes as children of node. > > - The nodes are parsed in a way that allows them to include > > - subheadings (titles) without confusing the rendering of > > - anything else. > > - """ > > - # This is from kerneldoc.py -- it works around an API change i= n > > - # Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use > > - # sphinx.util.nodes.nested_parse_with_titles() rather than the > > - # plain self.state.nested_parse(), and so we can drop the savi= ng > > - # of title_styles and section_level that kerneldoc.py does, > > - # because nested_parse_with_titles() does that for us. > > - if USE_SSI: > > - with switch_source_input(self.state, rstlist): > > - nested_parse_with_titles(self.state, rstlist, node) > > - else: > > - save =3D self.state.memo.reporter > > - self.state.memo.reporter =3D AutodocReporter( > > - rstlist, self.state.memo.reporter > > - ) > > - try: > > - nested_parse_with_titles(self.state, rstlist, node) > > - finally: > > - self.state.memo.reporter =3D save > > - > > > > def setup(app): > > """Register qapi-doc directive with Sphinx""" > > Reviewed-by: Markus Armbruster > > --000000000000c78aef061bf4badd Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Fri, Jun 28, 2024, 9:09=E2=80=AFAM Markus Armbruste= r <armbru@redhat.com> wrote:=
John Snow <jsnow@redhat.com&g= t; writes:

> Factor out the compatibility parser helper so it can be shared by othe= r
> directives.

Suggest "Factor out the compatibility parser helper into a base class,=
so it can be shared by other directives."

Sure. Haven't read the other = mails yet. I'll make the change if you want a v3, otherwise feel free t= o edit.


>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 docs/sphinx/qapidoc.py | 64 +++++++++++++++++++++++-------------= ------
>=C2=A0 1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index efcd84656fa..43dd99e21e6 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -494,7 +494,41 @@ def visit_module(self, name):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 super().visit_module(name)
>=C2=A0
>=C2=A0
> -class QAPIDocDirective(Directive):
> +class NestedDirective(Directive):
> +=C2=A0 =C2=A0 def run(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 raise NotImplementedError

Should this class be abstract?

It could be ...=C2=A0
=
*sneezes*

I plan to delete it by the end of the qapi-domain series anyw= ay, or perhaps I could even delete it *before* with a dedicated "requi= re sphinx >=3D 3.x" miniseries.

Actually, that's probably a really good idea...

> +
> +=C2=A0 =C2=A0 def do_parse(self, rstlist, node):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Parse rST source lines and add them to th= e specified node
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Take the list of rST source lines rstlist= , parse them as
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 rST, and add the resulting docutils nodes= as children of node.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 The nodes are parsed in a way that allows= them to include
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 subheadings (titles) without confusing th= e rendering of
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 anything else.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # This is from kerneldoc.py -- it works a= round an API change in
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Sphinx between 1.6 and 1.7. Unlike kern= eldoc.py, we use
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # sphinx.util.nodes.nested_parse_with_tit= les() rather than the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # plain self.state.nested_parse(), and so= we can drop the saving
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # of title_styles and section_level that = kerneldoc.py does,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # because nested_parse_with_titles() does= that for us.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if USE_SSI:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with switch_source_input(se= lf.state, rstlist):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nested_parse_= with_titles(self.state, rstlist, node)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 save =3D self.state.memo.re= porter
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.state.memo.reporter = =3D AutodocReporter(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rstlist, self= .state.memo.reporter
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 try:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nested_parse_= with_titles(self.state, rstlist, node)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 finally:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.state.me= mo.reporter =3D save
> +
> +
> +class QAPIDocDirective(NestedDirective):
>=C2=A0 =C2=A0 =C2=A0 """Extract documentation from the s= pecified QAPI .json file"""
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 required_argument =3D 1
> @@ -532,34 +566,6 @@ def run(self):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # so they are displaye= d nicely to the user
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise ExtensionError(s= tr(err)) from err
>=C2=A0
> -=C2=A0 =C2=A0 def do_parse(self, rstlist, node):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Parse rST source lines = and add them to the specified node
> -
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 Take the list of rST source lines rstlist= , parse them as
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 rST, and add the resulting docutils nodes= as children of node.
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 The nodes are parsed in a way that allows= them to include
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 subheadings (titles) without confusing th= e rendering of
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 anything else.
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # This is from kerneldoc.py -- it works a= round an API change in
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # Sphinx between 1.6 and 1.7. Unlike kern= eldoc.py, we use
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # sphinx.util.nodes.nested_parse_with_tit= les() rather than the
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # plain self.state.nested_parse(), and so= we can drop the saving
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # of title_styles and section_level that = kerneldoc.py does,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # because nested_parse_with_titles() does= that for us.
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if USE_SSI:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with switch_source_input(se= lf.state, rstlist):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nested_parse_= with_titles(self.state, rstlist, node)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 save =3D self.state.memo.re= porter
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.state.memo.reporter = =3D AutodocReporter(
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rstlist, self= .state.memo.reporter
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 try:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nested_parse_= with_titles(self.state, rstlist, node)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 finally:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.state.me= mo.reporter =3D save
> -
>=C2=A0
>=C2=A0 def setup(app):
>=C2=A0 =C2=A0 =C2=A0 """Register qapi-doc directive with= Sphinx"""

Reviewed-by: Markus Armbruster <armbru@redhat.com>

--000000000000c78aef061bf4badd--