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