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

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:

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list