[libvirt] [PATCH 0/2] docs: Fix enum documentation
So this patch sent to the list got me roll up my sleeves and get working: https://www.redhat.com/archives/libvir-list/2017-July/msg00835.html It wasn't that bad after all. Michal Privoznik (2): apibuild.py: Handle enum comments properly docs: Span cells if there's not doc text for enum val docs/apibuild.py | 8 +++++++- docs/newapi.xsl | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-) -- 2.13.0
After f4cb85c6aff7c1d90 we only have two options for placing enum values descriptions. It's either: typedef enum { /* Some long description. Therefore it's placed before * the value. */ VIR_ENUM_A_VAL = 1, } virEnumA; or: typedef enum { VIR_ENUM_B_VAL = 1, /* Some short description */ } virEnumB; However, our apibuild.py script is not able to deal with the former one. It messes up comments. To fix this couple of things needs to be done: a) DO NOT reset self.comment in parseEnumBlock(). This is a result from our tokenizer. Upon calling token() if it finds a comment block it stores it in self.comment and returns the next token (which is not comment). Therefore, if we reset self.comment we might lose the first comment in the enum block. b) we need a variable to track if the current enum block uses value descriptions before or after values. That is if it's type virEnumA or virEnumB. Depending on that, it we're dealing with virEnumA type and the current token is a comma ',' we can add the value into the list as we already have everything needed: comment, name and value. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/apibuild.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 47f340c7d..3a8f5d449 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -1365,9 +1365,9 @@ class CParser: def parseEnumBlock(self, token): self.enums = [] name = None - self.comment = None comment = "" value = "-1" + commentsBeforeVal = self.comment is not None while token is not None: if token[0] == "sep" and token[1] == "{": token = self.token() @@ -1408,6 +1408,10 @@ class CParser: self.warning("Failed to compute value of enum %s" % (name)) value="" if token[0] == "sep" and token[1] == ",": + if commentsBeforeVal and self.comment is not None: + self.cleanupComment() + self.enums.append((name, value, self.comment)) + name = comment = self.comment = None token = self.token() else: token = self.token() @@ -1652,6 +1656,8 @@ class CParser: self.enums = [] token = self.token() if token is not None and token[0] == "sep" and token[1] == "{": + # drop comments before the enum block + self.comment = None token = self.token() token = self.parseEnumBlock(token) else: -- 2.13.0
When generating HTML documentation we put enum values into a table so that we can display the value's name, numerical value and description (if it has one). Now the last part is problem. If the value doesn't have description the table row has just two cells and if it has one the row counts three cells. This makes HTML engines render the description into very little space - for instance see: html/libvirt-libvirt-domain.html#virDomainMemoryStatTags We can avoid this problem if we let the cell that corresponds to numerical value span over two cells if there's no description. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/newapi.xsl | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/newapi.xsl b/docs/newapi.xsl index 815c1b9d7..9dd961507 100644 --- a/docs/newapi.xsl +++ b/docs/newapi.xsl @@ -307,16 +307,21 @@ <tr> <td><a name="{@name}"><xsl:value-of select="@name"/></a></td> <td><xsl:text> = </xsl:text></td> - <td><xsl:value-of select="@value"/></td> - <xsl:if test="@info != ''"> - <td> - <div class="comment"> - <xsl:call-template name="dumptext"> - <xsl:with-param name="text" select="@info"/> - </xsl:call-template> - </div> - </td> - </xsl:if> + <xsl:choose> + <xsl:when test="@info != ''"> + <td><xsl:value-of select="@value"/></td> + <td> + <div class="comment"> + <xsl:call-template name="dumptext"> + <xsl:with-param name="text" select="@info"/> + </xsl:call-template> + </div> + </td> + </xsl:when> + <xsl:otherwise> + <td colspan="2"><xsl:value-of select="@value"/></td> + </xsl:otherwise> + </xsl:choose> </tr> </xsl:for-each> </table> -- 2.13.0
On Sat, Jul 22, 2017 at 11:04:34AM +0200, Michal Privoznik wrote:
When generating HTML documentation we put enum values into a table so that we can display the value's name, numerical value and description (if it has one). Now the last part is problem. If the value doesn't have description the table row has just two cells and if it has one the row counts three cells. This makes HTML engines render the description into very little space - for instance see:
html/libvirt-libvirt-domain.html#virDomainMemoryStatTags
We can avoid this problem if we let the cell that corresponds to numerical value span over two cells if there's no description.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/newapi.xsl | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
On Sat, 22 Jul 2017 11:04:32 +0200 Michal Privoznik <mprivozn@redhat.com> wrote:
So this patch sent to the list got me roll up my sleeves and get working:
https://www.redhat.com/archives/libvir-list/2017-July/msg00835.html
It wasn't that bad after all.
Thanks for giving it a try! There is a slight progress, if we can call it so, but we're not there yet. Looking at the generated docs for virDomainMemoryStatTags, there is an issue, although different from the original. The comment for VIR_DOMAIN_MEMORY_STAT_SWAP_IN is now picked up correctly. But VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT has now the comment that belongs to VIR_DOMAIN_MEMORY_STAT_UNUSED. Tomas
Michal Privoznik (2): apibuild.py: Handle enum comments properly docs: Span cells if there's not doc text for enum val
docs/apibuild.py | 8 +++++++- docs/newapi.xsl | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-)
-- 2.13.0
-- Tomáš Golembiovský <tgolembi@redhat.com>
On Sun, Jul 23, 2017 at 11:16:21PM +0200, Tomáš Golembiovský wrote:
On Sat, 22 Jul 2017 11:04:32 +0200 Michal Privoznik <mprivozn@redhat.com> wrote:
So this patch sent to the list got me roll up my sleeves and get working:
https://www.redhat.com/archives/libvir-list/2017-July/msg00835.html
It wasn't that bad after all.
Thanks for giving it a try!
There is a slight progress, if we can call it so, but we're not there yet. Looking at the generated docs for virDomainMemoryStatTags, there is an issue, although different from the original.
The comment for VIR_DOMAIN_MEMORY_STAT_SWAP_IN is now picked up correctly. But VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT has now the comment that belongs to VIR_DOMAIN_MEMORY_STAT_UNUSED.
Yeah, it doesn't fix everything, but thankfully it is strictly better than before. Reviewed-by: Martin Kletzander <mkletzan@redhat.com> What Tomas is explaining here looks like a problem with parsing or cleaning up the comments. It happens when there is a multiline comment for one enum value and the next one does not have any. I tried looking at the code yet again and it hurts my brain. But I'd still rather fix that than just work around it (add missing comments where it doesn't make much sense).
Tomas
Michal Privoznik (2): apibuild.py: Handle enum comments properly docs: Span cells if there's not doc text for enum val
docs/apibuild.py | 8 +++++++- docs/newapi.xsl | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-)
-- 2.13.0
-- Tomáš Golembiovský <tgolembi@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/24/2017 11:17 AM, Martin Kletzander wrote:
On Sun, Jul 23, 2017 at 11:16:21PM +0200, Tomáš Golembiovský wrote:
On Sat, 22 Jul 2017 11:04:32 +0200 Michal Privoznik <mprivozn@redhat.com> wrote:
So this patch sent to the list got me roll up my sleeves and get working:
https://www.redhat.com/archives/libvir-list/2017-July/msg00835.html
It wasn't that bad after all.
Thanks for giving it a try!
There is a slight progress, if we can call it so, but we're not there yet. Looking at the generated docs for virDomainMemoryStatTags, there is an issue, although different from the original.
The comment for VIR_DOMAIN_MEMORY_STAT_SWAP_IN is now picked up correctly. But VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT has now the comment that belongs to VIR_DOMAIN_MEMORY_STAT_UNUSED.
Yeah, it doesn't fix everything, but thankfully it is strictly better than before.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
What Tomas is explaining here looks like a problem with parsing or cleaning up the comments. It happens when there is a multiline comment for one enum value and the next one does not have any. I tried looking at the code yet again and it hurts my brain. But I'd still rather fix that than just work around it (add missing comments where it doesn't make much sense).
Actually, the fix was quite easy. In patch 1/2 I'm only appending to the list of enums if both @commentsBeforeVal and @self.comment are set. However, there's no real reason for testing self.comment. If the current value the script is processing doesn't have a comment (like in this case) then we should just not add one. To say it with patch: diff --git i/docs/apibuild.py w/docs/apibuild.py index 3a8f5d449..87e81f5c3 100755 --- i/docs/apibuild.py +++ w/docs/apibuild.py @@ -1408,7 +1408,7 @@ class CParser: self.warning("Failed to compute value of enum %s" % (name)) value="" if token[0] == "sep" and token[1] == ",": - if commentsBeforeVal and self.comment is not None: + if commentsBeforeVal: self.cleanupComment() self.enums.append((name, value, self.comment)) name = comment = self.comment = None Anyway, what is still going to be broken are cases where we mix comments before and after values. But well, I haven't found any just yet (haven't looked hard though) and we can fix that later if we want. I'm squashing that diff into 1/2 before pushing. Thanks for all the fish! Michal
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Tomáš Golembiovský