[libvirt] [PATCH 0/2] docs: Fix enum documentation

Michal Privoznik posted 2 patches 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1500714065.git.mprivozn@redhat.com
docs/apibuild.py |  8 +++++++-
docs/newapi.xsl  | 25 +++++++++++++++----------
2 files changed, 22 insertions(+), 11 deletions(-)
[libvirt] [PATCH 0/2] docs: Fix enum documentation
Posted by Michal Privoznik 6 years, 8 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] docs: Fix enum documentation
Posted by Tomáš Golembiovský 6 years, 8 months ago
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>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] docs: Fix enum documentation
Posted by Martin Kletzander 6 years, 8 months ago
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] docs: Fix enum documentation
Posted by Michal Privoznik 6 years, 8 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list