[PATCH v3 29/30] scripts: apibuild: add parsing variable's comments

[Victor Toso]

Hi,

On Fri, Apr 22, 2022 at 09:36:30AM -0700, Andrea Bolognani wrote:
> On Wed, Apr 20, 2022 at 09:08:18PM +0200, Victor Toso wrote:
> >      def serialize_variable(self, output, name):
> >          id = self.idx.variables[name]
> > -        if id.info is not None:
> > -            output.write("    <variable name='%s' file='%s' type='%s'/>\n" % (
> > -                name, self.modulename_file(id.header), id.info))
> > +        (type, comment) = id.info
> > +        (since, comment, _) = self.retrieve_comment_tags(name, comment)
> > +        version_tag = len(since) > 0 and f" version='{since}'" or ""
> > +        output.write("    <variable name='%s' file='%s' type='%s'%s" % (
> > +            name, self.modulename_file(id.header), type, version_tag))
> > +        if len(comment) == 0:
> > +            output.write("/>\n")
> >          else:
> > -            output.write("    <variable name='%s' file='%s'/>\n" % (
> > -                name, self.modulename_file(id.header)))
> > +            output.write(">\n      <info><![CDATA[%s]]></info>\n" % (comment))
> 
> Note that, for variables, the comment will not have gone
> through the same sanifications as for other symbols, and so the
> output will look like
> 
>      <variable name='virConnectAuthPtrDefault' file='libvirt-host'
> type='virConnectAuthPtr' version='0.4.1'>
>        <info><![CDATA[virConnectAuthPtrDefault:
> 
>   A default implementation of the authentication callbacks. This
>   implementation is suitable for command line based tools. It will
>   prompt for username, passwords, realm and one time keys as needed.
>   It will print on STDOUT, and read from STDIN. If this is not
>   suitable for the application's needs an alternative implementation
>   should be provided.]]></info>
>      </variable>
> 
> The first two lines of the CDATA section should obviously not be
> there.

Ah, nice catch. You can see that I had to dig around to store
the variable's documentation but I didn't notice it was before
the cleanup. I actually added an extra commit just help in the
case of variable's docstring.

> Various functions have code like
> 
>   lines = self.comment.split('\n')
>   if lines[0] == '*':
>       del lines[0]
>   if lines[0] != "* %s:" % name:
>       if not quiet:
>           self.warning("Misformatted function comment for %s" % name)
>           self.warning("  Expecting '* %s:' got '%s'" % (name, lines[0]))
>       return (ret[0], retdesc), args, desc
>   del lines[0]
>   while lines[0] == '*':
>       del lines[0]
> 
> to deal with this scenario, but I'm unclear on where exactly
> you'd put the equivalent for variables. The whole script is a
> giant underdocumented[1] mess and I'm utterly impressed by your
> ability to understand it well enough to add features to it.

The itching to change the code all around is still here. I
actually created another (local) branch to put some fixes and
removing dead code... but I knew if I started reworking things it
would consume more time to reach the end goal,
libvirt-go-module's MR.

> [1] I can absolutely appreciate the irony in that ;)

Indeed :)

I'm finishing the last touches of v4 and I plan to send it
without this addressed. Later, perhaps Today, I'll add a patch on
top that can either be an +1 or squashed in to address this.

Many thanks for noticing this!

Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220422/448c7eca/attachment.sig>