Re: [PATCH] Move comment after enum members

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



At Wed, 09 Jan 2013 18:06:51 -0700,
Eric Blake wrote:
> 
> On 01/09/2013 07:20 AM, Claudio Bley wrote:
> > The api builder always associates comments to the last member it read,
> > not to the current member even if there was a comment for the previous
> > member and a comma was already seen.
> > 
> > This has the effect that the comment for the previous member gets
> > overwritten and the current member has no comment at all.
> > 
> > Signed-off-by: Claudio Bley <cbley@xxxxxxxxxx>
> > ---
> >  include/libvirt/libvirt.h.in |   18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> > index 09c89c5..9110fcf 100644
> > --- a/include/libvirt/libvirt.h.in
> > +++ b/include/libvirt/libvirt.h.in
> > @@ -127,12 +127,12 @@ typedef enum {
> >                                      power management */
> >  
> >  #ifdef VIR_ENUM_SENTINELS
> > -    /*
> > -     * NB: this enum value will increase over time as new events are
> > -     * added to the libvirt API. It reflects the last state supported
> > -     * by this version of the libvirt API.
> > -     */
> >       VIR_DOMAIN_LAST
> > +     /*
> 
> You changed indentation from 4 to 5; please fix that.  Oh, I see why -
> VIR_DOMAIN_LAST is incorrectly indented to begin with.

Apparently, all of the members of enum virDomainState have wrong
indentation.

> > +      * NB: this enum value will increase over time as new events are
> > +      * added to the libvirt API. It reflects the last state supported
> > +      * by this version of the libvirt API.
> > +      */
> >  #endif
> >  } virDomainState;
> 
> Hmm, I wonder if we should instead fix the doc-generation parser.  I'm
> used to the style:
> 
> ONE, /* short comment for one */
> /* longer comment for two */
> TWO,
> THREE, /* and short for three again */

I think this won't work reliably. All you see in the parser is COMMENT
and NAME tokens.

You could only special case on two consecutive comments and then
assign the second to the current member.

Otherwise, what we could do is change the style rules and use the
comma as a separator:

ONE /* short comment for one */,
/* longer comment for two */
TWO,
THREE /* and short for three again */,

That way the association would be unambiguous. But I could imagine
what you're saying - that's ugly... It would require changing all
existing comments.

Another alternative I can think of would be to use a special marker
for "postfix" comments (like doxygen):

ONE, /*< short comment for one */
/* longer comment for two */
TWO,
THREE, /*< and short for three again */

This would also require changing all existing comments, unless we
introduce a special marker for "prefix" comments...

So, having said all that, here's another idea:

--- >8 ----
Subject: [PATCH] docs: Differentiate long and short comments inside enums

Regard a comment containing newline characters as a long comment
associating to the current member, otherwise it's just a short comment
associated with the previous member.

Signed-off-by: Claudio Bley <cbley@xxxxxxxxxx>
---
 docs/apibuild.py |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/docs/apibuild.py b/docs/apibuild.py
index 7b336b3..2c2b2e3 100755
--- a/docs/apibuild.py
+++ b/docs/apibuild.py
@@ -592,6 +592,7 @@ class CParser:
         self.top_comment = ""
         self.last_comment = ""
         self.comment = None
+        self.comments = []
         self.collect_ref = 0
         self.no_error = 0
         self.conditionals = []
@@ -705,8 +706,10 @@ class CParser:
             self.top_comment = com
         if self.comment == None or com[0] == '*':
             self.comment = com;
+            self.comments.append(self.comment)
         else:
             self.comment = self.comment + com
+            self.comments[-1] = self.comment
         token = self.lexer.token()

         if string.find(self.comment, "DOC_DISABLE") != -1:
@@ -1072,6 +1075,9 @@ class CParser:
         global ignored_words

         token = self.lexer.token()
+
+        if not self.comment: self.comments = []
+
         while token != None:
             if token[0] == 'comment':
                 token = self.parseComment(token)
@@ -1323,14 +1329,27 @@ class CParser:
                 token = self.token()
                 return token
             elif token[0] == "name":
+                    is_long_comment = (self.comment and self.comment.find('\n') != -1)
+                    long_comment = None
+
                     self.cleanupComment()
+
+                    if is_long_comment:
+                        long_comment = self.comment
+
+                        if len(self.comments) > 1:
+                            self.comment = self.comments[-2]
+                            self.cleanupComment()
+                        else:
+                            self.comment = None
+
                     if name != None:
                         if self.comment != None:
                             comment = string.strip(self.comment)
                             self.comment = None
                         self.enums.append((name, value, comment))
                     name = token[1]
-                    comment = ""
+                    comment = long_comment if is_long_comment else ""
                     token = self.token()
                     if token[0] == "op" and token[1][0] == "=":
                         value = ""
--
1.7.9.5

-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:<http://www.av-test.org>

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]