[libvirt] [PATCH] Move comment after enum members

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@av-test.de> --- 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 + /* + * 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; @@ -2742,12 +2742,12 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ #ifdef VIR_ENUM_SENTINELS + VIR_STORAGE_VOL_WIPE_ALG_LAST /* * NB: this enum value will increase over time as new algorithms are * added to the libvirt API. It reflects the last algorithm supported * by this version of the libvirt API. */ - VIR_STORAGE_VOL_WIPE_ALG_LAST #endif } virStorageVolWipeAlgorithm; @@ -2974,12 +2974,12 @@ typedef enum { VIR_KEYCODE_SET_RFB = 9, #ifdef VIR_ENUM_SENTINELS + VIR_KEYCODE_SET_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last keycode set supported * by this version of the libvirt API. */ - VIR_KEYCODE_SET_LAST #endif } virKeycodeSet; @@ -3533,12 +3533,12 @@ typedef enum { VIR_SECRET_USAGE_TYPE_CEPH = 2, #ifdef VIR_ENUM_SENTINELS + VIR_SECRET_USAGE_TYPE_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last secret owner ID * supported by this version of the libvirt API. */ - VIR_SECRET_USAGE_TYPE_LAST #endif } virSecretUsageType; @@ -4443,12 +4443,12 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ #ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_EVENT_ID_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last event ID supported * by this version of the libvirt API. */ - VIR_DOMAIN_EVENT_ID_LAST #endif } virDomainEventID; -- 1.7.9.5

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@av-test.de> --- 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.
+ * 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 */ But I guess I could live with this patch, if the style is forced on us by the doc generator. Anyone else with an opinion? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

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@av-test.de> --- 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@av-test.de> --- 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

At Thu, 10 Jan 2013 12:42:14 +0100, Claudio Bley wrote:
At Wed, 09 Jan 2013 18:06:51 -0700, Eric Blake wrote:
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 oxygen):
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.
Um, this doesn't work so well. Sometimes "short" comments span several lines, sometimes "long" comments are quite short... The only difference between them is the comment is on the same line as the enum member for "short" comments, otherwise on a line of its own. If the parser would track line numbers this would be easy... Claudio -- 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

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@av-test.de> --- Changes to v1: * rebased on master * move some short comments after the member include/libvirt/libvirt.h.in | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c1233f6..558aea2 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 + VIR_DOMAIN_LAST /* * 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 #endif } virDomainState; @@ -351,8 +351,7 @@ typedef enum { VIR_NODE_SUSPEND_TARGET_HYBRID = 2, #ifdef VIR_ENUM_SENTINELS - /* This constant is subject to change */ - VIR_NODE_SUSPEND_TARGET_LAST + VIR_NODE_SUSPEND_TARGET_LAST /* This constant is subject to change */ #endif } virNodeSuspendTarget; @@ -1186,8 +1185,7 @@ typedef enum { VIR_CRED_EXTERNAL = 9, /* Externally managed credential */ #ifdef VIR_ENUM_SENTINELS - /* More may be added - expect the unexpected */ - VIR_CRED_LAST + VIR_CRED_LAST /* More may be added - expect the unexpected */ #endif } virConnectCredentialType; @@ -1662,8 +1660,7 @@ typedef enum { VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE = 2, #ifdef VIR_ENUM_SENTINELS - /* This constant is subject to change */ - VIR_DOMAIN_NUMATUNE_MEM_LAST + VIR_DOMAIN_NUMATUNE_MEM_LAST /* This constant is subject to change */ #endif } virDomainNumatuneMemMode; @@ -2742,12 +2739,12 @@ typedef enum { VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random */ #ifdef VIR_ENUM_SENTINELS + VIR_STORAGE_VOL_WIPE_ALG_LAST /* * NB: this enum value will increase over time as new algorithms are * added to the libvirt API. It reflects the last algorithm supported * by this version of the libvirt API. */ - VIR_STORAGE_VOL_WIPE_ALG_LAST #endif } virStorageVolWipeAlgorithm; @@ -2974,12 +2971,12 @@ typedef enum { VIR_KEYCODE_SET_RFB = 9, #ifdef VIR_ENUM_SENTINELS + VIR_KEYCODE_SET_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last keycode set supported * by this version of the libvirt API. */ - VIR_KEYCODE_SET_LAST #endif } virKeycodeSet; @@ -3533,12 +3530,12 @@ typedef enum { VIR_SECRET_USAGE_TYPE_CEPH = 2, #ifdef VIR_ENUM_SENTINELS + VIR_SECRET_USAGE_TYPE_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last secret owner ID * supported by this version of the libvirt API. */ - VIR_SECRET_USAGE_TYPE_LAST #endif } virSecretUsageType; @@ -4443,12 +4440,12 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ #ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_EVENT_ID_LAST /* * NB: this enum value will increase over time as new events are * added to the libvirt API. It reflects the last event ID supported * by this version of the libvirt API. */ - VIR_DOMAIN_EVENT_ID_LAST #endif } virDomainEventID; -- 1.7.9.5

At Fri, 11 Jan 2013 15:05:33 +0100, 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.
Since I got principal blessing from Erik and this patch qualifies as being trivial, I'm going to push it real soon if nobody objects in, say, the next day or so. Claudio -- 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

At Mon, 14 Jan 2013 09:08:56 +0100, Claudio Bley wrote:
At Fri, 11 Jan 2013 15:05:33 +0100, 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.
Since I got principal blessing from Erik and this patch qualifies as being trivial, I'm going to push it real soon if nobody objects in, say, the next day or so.
Took a bit longer, pushed now. Claudio -- 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
participants (2)
-
Claudio Bley
-
Eric Blake