Created attachment 169233 [details] [review]
Support classes and namespaces with the same name.

Created attachment 169234 [details] [review]
Support classes and namespaces with the same name.

This should help things like GdkPixbuf where there
is a type "GdkPixbuf", which is the same as the
namespace.

Created attachment 169268 [details] [review]
Support classes and namespaces with the same name.

This should help things like GdkPixbuf where there
is a type "GdkPixbuf", which is the same as the
namespace.

What's the bug?  GdkPixbuf seems to scan fine here (after ebassi pushed an equivalent commit to what I had locally)

Oh sorry, I see now...

Review of attachment 169268 [details] [review]:

::: giscanner/gdumpparser.py
@@ +130,3 @@
                 get_type_func = self._namespace.get(name)
+                if get_type_func:
+                    to_remove.append(get_type_func)

This shouldn't be necessary if we've got the namespacing parsing working right, right?

::: giscanner/transformer.py
@@ +287,3 @@
+                        name = ident[len(prefix):]
+                        if name == "":
+                            matches.append((ns, prefix, 0))

This part seems ok; we should add a comment here or link to this bug.

@@ +289,3 @@
+                            matches.append((ns, prefix, 0))
+                        elif name == "Class": # XXX
+                            matches.append((ns, prefix+"Class", 0))

Yuck.  I'd like to try harder to figure out a less hacky fix.  We could special-case GdkPixbuf in the scanner (I did this already for Atk).

@@ +309,3 @@
                 if not prefix.endswith('_'):
                     prefix = prefix + '_'
+                if ns.name in ns.ctypes:

We can't look in the namespace *contents* in this function, that would introduce ordering issues and make the function effectively nondeterministic.

Created attachment 173253 [details] [review]
updated patch

I kind of wanted to emit a warning in _split_type_and_symbol_prefix, since it's currently a fatal error, but that would cause the test to fail.

(In reply to comment #7)
> Created an attachment (id=173253) [details] [review]
> updated patch

I'm still not sure we want this.  The example given of GdkPixbuf was just a missing --identifier-prefix=Gdk.  

So we do want GdkPixbuf.Pixbuf, not GdkPixbuf.GdkPixbuf.  Are there other libraries which would be fixed with this patch?

I'm working on introspection support for rhythmbox, which uses RhythmDB as both a namespace and a class name.  I could throw everything into one namespace and end up with slightly ugly things like RB.RhythmDB, but I'd prefer this solution.

This looks a bit cleaner than what I did.

Colin and I talked it over in #introspection on IRC
back when I submitted the original patch:

  * For now, it was too messy a patch, and the use case
    for back could be fixed very easily.

  * When introspecting failed (GdkPixbuf.GdkPixbuf),
    there was a very misleading assert or IndexError
    or something else like that. It looks that someone
    finally added an error for that...


The big dirty part in my patch way back when was when I
was checking the namespace contents. I don't see that here:
if the test works, that's wonderful (still not sure if it
was me being stupid or subsequent fixes in the repo).

The small dirty parts are checking for "Class", "Iface"
and "Interface", but I'd rather special-case GObject
than special-case Rhythmbox or ATK.

(In reply to comment #10)
> I'm working on introspection support for rhythmbox, which uses RhythmDB as both
> a namespace and a class name.  I could throw everything into one namespace and
> end up with slightly ugly things like RB.RhythmDB, but I'd prefer this
> solution.

You do want it to be RhythmDB.RhythmDB?  All the other cases where this came up we thought the --strip-prefix option was better.

It feels inconsistent, since everything else will be stripped, i.e. RhythmDB.Entry but RhythmDB.RhythmDB?  Supporting this in the code is ugly too as you found out.

Are there any obvious things to rename it to?  RhythmDBContext?  RhythmDBMaster?

If you feel strongly about it we can add this patch, but I just want to explore options more.

I don't really have a strong opinion on what name it gets, I just want it to work somehow, and renaming the class seemed like more work than doing this.  I tried out the approach of putting everything in the RB namespace, and that actually seems OK, so I'm not going to push for this any further.

Initially I wasn't getting the 'this class ends up with no name' error message, but instead I was getting a .gir with 'name=""' in various places, so I guess I should at least fix that.

*** Bug 675985 has been marked as a duplicate of this bug. ***

Note that bug 675985 had the idea to use a rename annotation to fix this (also had a patch which seemed to break other things though).

I've also added a patch to bug 706898 (stripping _t on struct names) which takes yet another approach. Adding a new g-ir-scanner flag (--identifier-filter-cmd). This would allow the problem described here to be fixed externally to the library  with minimal code changes to g-ir-scanner.

Hit that with gnome-pocket/GnomePocket as well (in https://git.gnome.org/browse/gnome-pocket/)

Could we at least add a warning explaining the problem and pointing to this bug? Took me a good hour to find what the problem was...

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]

Re-titling for clarity.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/34.