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
Attachment:
signature.asc
Description: PGP signature
