[libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums

Peter Krempa posted 1 patch 5 years, 3 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/804c2654cbe6fa7e6881de0b03b49ca629255ac8.1548340995.git.pkrempa@redhat.com
docs/apibuild.py |  4 ++++
docs/libvirt.css |  4 ++++
docs/newapi.xsl  | 15 +++++++++++++--
3 files changed, 21 insertions(+), 2 deletions(-)
[libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums
Posted by Peter Krempa 5 years, 3 months ago
Big number itself does not make much sense in some cases. Format the
bitshift format as well.

Changes our web page docs from:

VIR_MIGRATE_POSTCOPY = 32768 : Setting the VIR_MIGRATE_POSTCOPY...
VIR_MIGRATE_TLS      = 65536 : Setting the VIR_MIGRATE_TLS flag...

to:

VIR_MIGRATE_POSTCOPY = 32768 (1<<15) : Setting the VIR_MIGRATE_POSTCOPY...
VIR_MIGRATE_TLS      = 65536 (1<<16) : Setting the VIR_MIGRATE_TLS flag...

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/apibuild.py |  4 ++++
 docs/libvirt.css |  4 ++++
 docs/newapi.xsl  | 15 +++++++++++++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/docs/apibuild.py b/docs/apibuild.py
index 3ef5d0f554..a57f995189 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -2123,6 +2123,10 @@ class docBuilder:
                 except:
                     val = info[0]
                 output.write(" value='%s'" % (val))
+                valuestr = re.sub("<", "&lt;", info[0])
+                valuestr = re.sub("[\(\)]", "", valuestr)
+                output.write(" valuestr='%s'" % valuestr)
+
             if info[2] is not None and info[2] != '':
                 output.write(" type='%s'" % info[2])
             if info[1] is not None and info[1] != '':
diff --git a/docs/libvirt.css b/docs/libvirt.css
index e590b33cfb..c5fe27fa3f 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -537,3 +537,7 @@ dl.mail dt a:hover {
     color:  rgb(255, 230, 0);
     text-decoration: none;
 }
+
+td.enumvalue {
+    white-space: nowrap;
+}
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index 8d4c032c03..0a3658c9a5 100644
--- a/docs/newapi.xsl
+++ b/docs/newapi.xsl
@@ -288,6 +288,17 @@
     </xsl:choose>
   </xsl:template>

+  <xsl:template name="enumvalue">
+    <xsl:param name="value" select="@value"/>
+    <xsl:param name="valuestr" select="@valuestr"/>
+    <xsl:value-of select="@value"/>
+    <xsl:if test="$value != $valuestr">
+      <xsl:text> (</xsl:text>
+      <xsl:value-of select="@valuestr"/>
+      <xsl:text>)</xsl:text>
+    </xsl:if>
+  </xsl:template>
+
   <xsl:template match="typedef[@type = 'enum']">
     <xsl:variable name="name" select="string(@name)"/>
     <h3><a name="{$name}"><code><xsl:value-of select="$name"/></code></a></h3>
@@ -306,7 +317,7 @@
             <td><xsl:text> = </xsl:text></td>
             <xsl:choose>
               <xsl:when test="@info != ''">
-                <td><xsl:value-of select="@value"/></td>
+                <td class="enumvalue"><xsl:call-template name="enumvalue"/></td>
                 <td>
                   <div class="comment">
                     <xsl:call-template name="dumptext">
@@ -316,7 +327,7 @@
                 </td>
               </xsl:when>
               <xsl:otherwise>
-                <td colspan="2"><xsl:value-of select="@value"/></td>
+                <td colspan="2" class="enumvalue"><xsl:call-template name="enumvalue"/></td>
               </xsl:otherwise>
             </xsl:choose>
           </tr>
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums
Posted by John Ferlan 5 years, 2 months ago

On 1/24/19 9:43 AM, Peter Krempa wrote:
> Big number itself does not make much sense in some cases. Format the
> bitshift format as well.
> 
> Changes our web page docs from:
> 
> VIR_MIGRATE_POSTCOPY = 32768 : Setting the VIR_MIGRATE_POSTCOPY...
> VIR_MIGRATE_TLS      = 65536 : Setting the VIR_MIGRATE_TLS flag...
> 
> to:
> 
> VIR_MIGRATE_POSTCOPY = 32768 (1<<15) : Setting the VIR_MIGRATE_POSTCOPY...
> VIR_MIGRATE_TLS      = 65536 (1<<16) : Setting the VIR_MIGRATE_TLS flag...

Do you (or does anyone) think perhaps it should be (1 << 15)?

NB: There is one comment for VIR_DOMAIN_AFFECT_CONFIG which has 1 << 2
and that looks cleaner w/ the spacing.

> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  docs/apibuild.py |  4 ++++
>  docs/libvirt.css |  4 ++++
>  docs/newapi.xsl  | 15 +++++++++++++--
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums
Posted by Daniel P. Berrangé 5 years, 2 months ago
On Thu, Jan 24, 2019 at 03:43:15PM +0100, Peter Krempa wrote:
> Big number itself does not make much sense in some cases. Format the
> bitshift format as well.
> 
> Changes our web page docs from:
> 
> VIR_MIGRATE_POSTCOPY = 32768 : Setting the VIR_MIGRATE_POSTCOPY...
> VIR_MIGRATE_TLS      = 65536 : Setting the VIR_MIGRATE_TLS flag...
> 
> to:
> 
> VIR_MIGRATE_POSTCOPY = 32768 (1<<15) : Setting the VIR_MIGRATE_POSTCOPY...
> VIR_MIGRATE_TLS      = 65536 (1<<16) : Setting the VIR_MIGRATE_TLS flag...

I tend to question why we are reporting the value in the docs at all.

The whole point of enums/constants is that developers should never care
about the actual. They should exclusively use the names.

So if anything I'd suggest we remove the existing numbers from the docs,
instead of adding the bitshift values.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums
Posted by John Ferlan 5 years, 2 months ago

On 1/29/19 8:07 AM, Daniel P. Berrangé wrote:
> On Thu, Jan 24, 2019 at 03:43:15PM +0100, Peter Krempa wrote:
>> Big number itself does not make much sense in some cases. Format the
>> bitshift format as well.
>>
>> Changes our web page docs from:
>>
>> VIR_MIGRATE_POSTCOPY = 32768 : Setting the VIR_MIGRATE_POSTCOPY...
>> VIR_MIGRATE_TLS      = 65536 : Setting the VIR_MIGRATE_TLS flag...
>>
>> to:
>>
>> VIR_MIGRATE_POSTCOPY = 32768 (1<<15) : Setting the VIR_MIGRATE_POSTCOPY...
>> VIR_MIGRATE_TLS      = 65536 (1<<16) : Setting the VIR_MIGRATE_TLS flag...
> 
> I tend to question why we are reporting the value in the docs at all.
> 
> The whole point of enums/constants is that developers should never care
> about the actual. They should exclusively use the names.
> 
> So if anything I'd suggest we remove the existing numbers from the docs,
> instead of adding the bitshift values.
> 

I think it would look odd without some value, but I could be in the
minority opinion on that.

As a developer sometimes you have to debug things and wonder what a
value means. There's a few values which jump from "smaller" values to
much larger values (e.g. VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT) where
values go from 1, 2, 4, etc. jumping up to 536870912, 1073741824, etc.

While yes, open source someone can look it up, but perhaps the first
place to look is the docs. The visual cue of seeing (1 << 29), (1 <<
30), etc. would be enough for me.

John


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Format bit shift notation for bitwise flag enums
Posted by Daniel P. Berrangé 5 years, 2 months ago
On Tue, Jan 29, 2019 at 08:41:59AM -0500, John Ferlan wrote:
> 
> 
> On 1/29/19 8:07 AM, Daniel P. Berrangé wrote:
> > On Thu, Jan 24, 2019 at 03:43:15PM +0100, Peter Krempa wrote:
> >> Big number itself does not make much sense in some cases. Format the
> >> bitshift format as well.
> >>
> >> Changes our web page docs from:
> >>
> >> VIR_MIGRATE_POSTCOPY = 32768 : Setting the VIR_MIGRATE_POSTCOPY...
> >> VIR_MIGRATE_TLS      = 65536 : Setting the VIR_MIGRATE_TLS flag...
> >>
> >> to:
> >>
> >> VIR_MIGRATE_POSTCOPY = 32768 (1<<15) : Setting the VIR_MIGRATE_POSTCOPY...
> >> VIR_MIGRATE_TLS      = 65536 (1<<16) : Setting the VIR_MIGRATE_TLS flag...
> > 
> > I tend to question why we are reporting the value in the docs at all.
> > 
> > The whole point of enums/constants is that developers should never care
> > about the actual. They should exclusively use the names.
> > 
> > So if anything I'd suggest we remove the existing numbers from the docs,
> > instead of adding the bitshift values.
> > 
> 
> I think it would look odd without some value, but I could be in the
> minority opinion on that.
> 
> As a developer sometimes you have to debug things and wonder what a
> value means. There's a few values which jump from "smaller" values to
> much larger values (e.g. VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT) where
> values go from 1, 2, 4, etc. jumping up to 536870912, 1073741824, etc.
> 
> While yes, open source someone can look it up, but perhaps the first
> place to look is the docs. The visual cue of seeing (1 << 29), (1 <<
> 30), etc. would be enough for me.

Actually, yes, I was forgetting about the case of debugging where knowing
the real values is important.

For "flags" at least, the logs will typically have the values printed in
hex & you need to look them up.  So I wonder if we could usefully
print both decimal & hex in the docs (and the bit shift) ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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