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

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 :|


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