GNOME Bugzilla – Bug 631530
support glib-mkenums comment /*< flags *>/
Last modified: 2015-02-07 16:53:21 UTC
See http://library.gnome.org/devel/gobject/unstable/glib-mkenums.html Specifically: typedef enum /*< flags,prefix=PREFIX >*/ {
Relevant IRC discussion: <walters> jdahlin: did you review d15e386c ? <walters> (did anyone?) <jdahlin> walters: yes, looked good to me <walters> but "bitfield" is not a valid (type) <walters> now our type parsing his hacky and awful <jdahlin> what's a valid type then? <walters> GLib.Object <walters> guint32 <walters> anything defined in ast.py <jdahlin> so we should use "GFlags" ? <jdahlin> which is a valid GType name <walters> well, glib-mkenums has a magic comment for this <walters> /*< flags >*/ i think <jdahlin> okay, I can revert that so we parse the comment instead <walters> disregarding syncing up with the glib-mkenums comment, if we were to do an annotation I'd much prefer (flags) or something <jdahlin> we should support the gtk-doc comment first and foremost * Piotras has quit (Ping timeout: 600 seconds) <walters> yep * jrb (~jrb@nat-pool-3-rdu.redhat.com) has joined #introspection <CIA-1> johan * r24d989b151b1 gobject-introspection/ (3 files in 2 dirs): Revert "Apply `(type bitfield)' annotations for enums" <CIA-1> johan * r57213017a4e3 gobject-introspection/giscanner/maintransformer.py: Revert "block can be None, fixup last commit" * nemequ has quit (Remote closed the connection) * nemequ (~nemequ@ip68-111-223-191.sd.sd.cox.net) has joined #introspection <jdahlin> rotty: okay, you need to redo that patch so it parses gtk-doc <flags> comments, see 8398ce7 for a hint on how to do that <jdahlin> rotty: eg 1) parse <flags> in the source lexer 2) add a flags field and attach it to enum symbols in the source parser 3) write python bindings for it 4) use it in transformer.py (_not_ in main-transformer)
Created attachment 171907 [details] [review] Support glib-mkenums comment /*< flags *>/ - Modify the lexer to consider all "trigraph" comments specially, and parse them for "flags" as well as "private" and "public" (which were previously hardcoded). This change allows for future support of multiple annotations inside a single trigraph comment. - Change the parser to consider the additional field "flags" set by the lexer when constructing enums. - Add a test case for the "flags" trigraph comment to the scanner annotation tests. See <https://bugzilla.gnome.org/show_bug.cgi?id=631530>.
Review of attachment 171907 [details] [review]: Two minor things, otherwise looks good to me. ::: giscanner/scannerlexer.l @@ +75,2 @@ "/*" { parse_comment(scanner); } +"/*"[\t ]*<[\t ,=A-Za-z0-9_]+>[\t ]*"*/" { parse_trigraph(scanner); } Allowing arbitrary space between /* and < seems too likely to hit false positives to me. How about "[\t ]?" instead of "[\t ]*" ? ::: giscanner/scannerparser.y @@ +1,3 @@ /* GObject introspection: C parser * + * Copyright (c) 1997, 2010 Sandro Sigala <ssigala@globalnet.it> Confused by this - you aren't Sandro Sigala, so why are you adding 2010 to his copyright? I'd expect the additional line to be Copyright (c) 2010 Andreas Rottmann <a.rottmann@gmx.at>
(In reply to comment #3) > Review of attachment 171907 [details] [review]: > > Two minor things, otherwise looks good to me. > > ::: giscanner/scannerlexer.l > @@ +75,2 @@ > "/*" { parse_comment(scanner); } > +"/*"[\t ]*<[\t ,=A-Za-z0-9_]+>[\t ]*"*/" { parse_trigraph(scanner); } > > Allowing arbitrary space between /* and < seems too likely to hit false > positives to me. > > How about "[\t ]?" instead of "[\t ]*" ? I agree, and would even be happier if I could leave out the space completely, but unfortunatly, gtk-doc (gtkdoc-mkdb.in) allows an arbitrary amount of space here. Should I change to single space nevertheless, and hence be more strict than gtk-doc? > > ::: giscanner/scannerparser.y > @@ +1,3 @@ > /* GObject introspection: C parser > * > + * Copyright (c) 1997, 2010 Sandro Sigala <ssigala@globalnet.it> > > Confused by this - you aren't Sandro Sigala, so why are you adding 2010 to his > copyright? > Uh, sorry; I didn't mean to modify that line at all; it was Emacs' update-copyright functionality which I apparently have accidentally answered "yes" when it asked about updating the copyright. > I'd expect the additional line to be Copyright (c) 2010 Andreas Rottmann > <a.rottmann@gmx.at> How is the policy of such things in GObject-Introspection? It was my impression that copyright headers seem not to be updated in general, and hence I did not add/update these when I modified a file in my patches. I'd like to know how I should handle this, so I can DTRT on future patches.
(In reply to comment #4) > > I agree, and would even be happier if I could leave out the space > completely, but unfortunatly, gtk-doc (gtkdoc-mkdb.in) allows an > arbitrary amount of space here. Should I change to single space > nevertheless, and hence be more strict than gtk-doc? I'd say go with to 0-1. > How is the policy of such things in GObject-Introspection? If you're making significant changes ( http://www.gnu.org/prep/maintain/html_node/Legally-Significant.html ), feel free to add a copyright. Or don't (the canonical data is really "git annotate", the header is just informative here). So it's up to you. > It was my > impression that copyright headers seem not to be updated in general, Yeah, I haven't been bothering typically myself, but I should do so more.
Created attachment 172770 [details] [review] I think I've addressed all the suggestions in this new version of the patch.
Review of attachment 172770 [details] [review]: Looks good, thanks!
Pushed as d85dbebee.., so this bug can be closed now, I hope.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]