After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 706898 - Support lowercase typenames ending with _t
Support lowercase typenames ending with _t
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 720090
 
 
Reported: 2013-08-27 14:51 UTC by Behdad Esfahbod
Modified: 2015-02-07 16:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
scanner: Add --identifier-filter-cmd (9.74 KB, patch)
2014-01-04 16:37 UTC, Simon Feltman
none Details | Review
scanner: Add --identifier-filter-cmd (9.79 KB, patch)
2014-05-12 19:09 UTC, Simon Feltman
none Details | Review
scanner: Add --identifier-filter-cmd (10.35 KB, patch)
2014-05-13 00:41 UTC, Simon Feltman
none Details | Review
scanner: Add --identifier-filter-cmd (12.32 KB, patch)
2014-05-16 00:10 UTC, Simon Feltman
committed Details | Review

Description Behdad Esfahbod 2013-08-27 14:51:27 UTC
In cairo, HarfBuzz, etc, we don't use camelcase type names, but use lowercase underscore-separated names ending in '_t'.  Because of it, g-i cannot associate methods for those types back to the type.

Any way you can drop the _t from the name when parsing?
Comment 1 Simon Feltman 2014-01-04 16:37:33 UTC
Created attachment 265313 [details] [review]
scanner: Add --identifier-filter-cmd

Add the command line flag --identifier-filter-cmd to g-ir-scanner which
allows running identifier names through a filtering shell command. The
identifier is sent as stdin to the filter command and expects a filtered
result written to stdout.

https://bugzilla.gnome.org/show_bug.cgi?706898

Notes:
This approach is non-invasive to the library being scanned as well as 
generic in terms of what g-ir-scanner is doing. It would also allow a way to 
fix bug 628496. See the added test in test_transform.py for an 
example of it using sed to strip off the "_t" and convert the struct names 
to TitleCase.
Comment 2 Gian Mario Tagliaretti 2014-01-07 19:41:30 UTC
Simon's patch could also be pretty useful (I guess) with GooCanvas-2, there are some types like GooCairoMatrix, GooCairoPattern that do not work if --identifier-prefix is set to GooCanvas.
Comment 3 Behdad Esfahbod 2014-01-08 02:48:15 UTC
Can someone post an example please?
Comment 4 Simon Feltman 2014-01-08 03:17:32 UTC
(In reply to comment #3)
> Can someone post an example please?

Take a look at the tests and Makefile.am:
https://bugzilla.gnome.org/review?bug=706898&attachment=265313
Comment 5 Simon Feltman 2014-05-12 19:09:05 UTC
Created attachment 276403 [details] [review]
scanner: Add --identifier-filter-cmd

Rebased Makefile.am.

One problem with the approach here is portability. If we start relying
on something like sed to build GIRs, it might not be very portable to
say msvc builds... I originally tried to use Pythons builtin regex,
but it does not support lowercase to upper case replacements.
Comment 6 Emmanuele Bassi (:ebassi) 2014-05-12 23:22:28 UTC
> Created an attachment (id=276403) [details] [review]
> scanner: Add --identifier-filter-cmd
> 
> Rebased Makefile.am.

I can confirm that this works, and works fairly well. it actually made it possible to get introspection into Graphene, which uses the <type_name>_t convention shared by Cairo and Harfbuzz:

https://github.com/ebassi/graphene/commit/7e2a27ea2c67543a74a1dc3508cfb1a591197c7d

I'd really like to land this.

> One problem with the approach here is portability. If we start relying
> on something like sed to build GIRs, it might not be very portable to
> say msvc builds... I originally tried to use Pythons builtin regex,
> but it does not support lowercase to upper case replacements.

we could have a python script that does the right thing, and maybe without a regexp, but for the time being I kinda like the idea of having a filter command — like git-filter-branch. we could even ship with a couple of built-in transformations to simplify the rules of common library.(In reply to comment #5)
Comment 7 Simon Feltman 2014-05-13 00:41:37 UTC
Created attachment 276418 [details] [review]
scanner: Add --identifier-filter-cmd

Added tests for failure conditions.
Comment 8 Simon Feltman 2014-05-13 00:48:40 UTC
(In reply to comment #6)
> I can confirm that this works, and works fairly well.

Awesome, thanks for testing.

> > One problem with the approach here is portability.
> > ...
> we could have a python script that does the right thing, and maybe without a
> regexp, but for the time being I kinda like the idea of having a filter command
> — like git-filter-branch. we could even ship with a couple of built-in
> transformations to simplify the rules of common library.(In reply to comment
> #5)

A small Python script passed as the filter command would solve the problem. I'm not sure how useful builtin filter types would be in practice though. Cairo for example would need custom bits to translate cairo_t into CairoContext and it looks like you also needed some customization for point3d_t to Point3D.
Comment 9 Emmanuele Bassi (:ebassi) 2014-05-13 08:03:12 UTC
we could ship a small python script that did the equivalent of this:

https://git.gnome.org/browse/clutter/tree/clutter/clutter-script-parser.c?h=clutter-1.18#n104

but instead of adding "_get_type" it adds "_t" at the end.

in the end, though, it's mostly academical: I think the current implementation is perfectly fine, and can be refined later.

I think we should get this in now, since we are still at the beginning of the cycle.
Comment 10 Emmanuele Bassi (:ebassi) 2014-05-13 08:05:13 UTC
(In reply to comment #9)
> we could ship a small python script that did the equivalent of this:
> 
> https://git.gnome.org/browse/clutter/tree/clutter/clutter-script-parser.c?h=clutter-1.18#n104
> 
> but instead of adding "_get_type" it adds "_t" at the end.

I meant the reverse operation, obviously.
Comment 11 Simon Feltman 2014-05-16 00:10:27 UTC
Created attachment 276635 [details] [review]
scanner: Add --identifier-filter-cmd

Updated patch which replaces usage of sed with a Python script.
This should allow portability to Windows for the test as well
as give developers guidance on a recommended approach.
Comment 12 Emmanuele Bassi (:ebassi) 2014-06-02 16:26:29 UTC
Review of attachment 276635 [details] [review]:

okay, since nobody is reviewing g-i patches and we're getting late in the cycle, I'm going to take executive action, and mark this as accepted.

we can revert the decision later, but this already enables introspecting harfbuzz and graphene, and I really want these two libraries to get introspection.

given that this patch has even tests, and the design is self-contained and sensible, I don't foresee any issue with it.
Comment 13 Simon Feltman 2014-06-03 20:40:09 UTC
Comment on attachment 276635 [details] [review]
scanner: Add --identifier-filter-cmd

Committed. There ended up being a minor mistake causing distcheck to fail:
https://git.gnome.org/browse/gobject-introspection/commit/?id=2208d0f0ed92e
Comment 14 André Klapper 2015-02-07 16:56:32 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]