[libvirt] [PATCH] libvirt-domain.h: Fix enum description placement

[Martin Kletzander]

On Fri, Jul 21, 2017 at 02:42:05PM +0100, Daniel P. Berrange wrote:
>On Fri, Jul 21, 2017 at 03:00:20PM +0200, Martin Kletzander wrote:
>> On Fri, Jul 21, 2017 at 11:58:45AM +0100, Daniel P. Berrange wrote:
>> > On Fri, Jul 21, 2017 at 12:52:06PM +0200, Michal Privoznik wrote:
>> > > There are only two acceptable places for describing enum values.
>> > > 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, during review of a patch sent upstream I realized that
>> > > is not always the case. I went through all the public header
>> > > files and identified all the offenders. Luckily there were just
>> > > two of them.
>> > >
>> > > Yes, this makes our HTML generated documentation broken, but
>> > > that's bug of the generator. Our header files shouldn't be forced
>> > > to use something we don't want to.
>> >
>> > FWIW, the problem is in the parseEnumBLock() method in apibuild.py
>> >
>> > Once it has completed parsing an enum item, it delays adding that
>> > enum to the list until it sees the next item, so that it can capture
>> > the trailing comment.
>> >
>> > The only way we can distinguish between a comment that comes before
>> > the enum vs a comment after the enum, on the same line, is by detecting
>> > whitespace (newline) before the trailing comment. Unfortunately I don't
>> > thing this is exposed by the lexer right, so its not entirely easy
>> > to solve.
>> >
>>
>> I was under the impression that this worked, we only broke it by some
>> recent commit.  I looked at the code and got pretty confused by it, but
>> shouldn't it be pretty easy (from a big picture view)?  You read until
>> you have both comment and a member of the struct.
>>
>> If it's really harder than I think, then we can start using some helper
>> characters for the comments (at least for now) so that we can properly
>> identify them, e.g.:
>>
>> struct meh {
>>       /*# This is comment for the following member foo */
>>       unsigned int foo;
>>       int bar; /*< This is for member bar that's on the same line */
>> }
>>
>> and so on.  If that doesn't help either and it never worked,
>> then... it's a pity :-/
>
>That is ambiguous - without seeing whitespace, the parser cannot
>distinguish between these two scenarios:
>

Oh, you're right, so the only other easy workaround option that I can
think of is to just document every single thing, then the parser can
always just first two things (comment and member) together.

Or, as Jirka suggested, just make it mandatory to put the comments
before and fix the script to accept just that order.

> struct meh {
>       unsigned int foo; /*# This is comment for the following member foo */
>       int bar;
> }
>
> struct meh {
>       unsigned int foo;
>
>        /*# This is comment for the following member foo */
>       int bar;
> }
>
>
>
>
>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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170721/a3373b44/attachment-0001.sig>