Improve and unify __repr__ format for PyGObject, PyGBoxed and PyGIStruct

Includes the module, Python type name, pointer and wrapper pointer
and GType name. e.g.
"<Gtk.Border object at 0x7f26bee9e4c8 (GtkBorder at 0x1c72e30)>"

In case of PyGObjects created from GType set the module to __gi__
instead of guessing the old static binding name. Also
remove the code that tries to add them to the guessed static bindings
module while at it. The __repr__ will now return
"<__gi__.X11Keymap ..." instead of "<gtk.gdk.X11Keymap ..."

https://bugzilla.gnome.org/show_bug.cgi?id=657915

4 files changed, 113 insertions, 45 deletions

diff --git a/gi/pygboxed.c b/gi/pygboxed.c
index 9faa6523..30ab423a 100644
--- a/gi/pygboxed.c
+++ b/gi/pygboxed.c
@@ -68,13 +68,34 @@ pyg_boxed_hash(PyGBoxed *self)
 }
 
 static PyObject *
-pyg_boxed_repr(PyGBoxed *self)
+pyg_boxed_repr(PyGBoxed *boxed)
 {
-    gchar buf[128];
+    PyObject *module, *repr, *self = (PyObject *)boxed;
+    gchar *module_str, *namespace;
 
-    g_snprintf(buf, sizeof(buf), "<%s at 0x%lx>", g_type_name(self->gtype),
-	       (long)pyg_boxed_get_ptr (self));
-    return PYGLIB_PyUnicode_FromString(buf);
+    module = PyObject_GetAttrString (self, "__module__");
+    if (module == NULL)
+        return NULL;
+
+    if (!PYGLIB_PyUnicode_Check (module)) {
+        Py_DECREF (module);
+        return NULL;
+    }
+
+    module_str = PYGLIB_PyUnicode_AsString (module);
+    namespace = g_strrstr (module_str, ".");
+    if (namespace == NULL) {
+        namespace = module_str;
+    } else {
+        namespace += 1;
+    }
+
+    repr = PYGLIB_PyUnicode_FromFormat ("<%s.%s object at %p (%s at %p)>",
+                                        namespace, Py_TYPE (self)->tp_name,
+                                        self, g_type_name (boxed->gtype),
+                                        pyg_boxed_get_ptr (boxed));
+    Py_DECREF (module);
+    return repr;
 }
 
 static int
diff --git a/gi/pygi-struct.c b/gi/pygi-struct.c
index adf8e72e..d84eed5a 100644
--- a/gi/pygi-struct.c
+++ b/gi/pygi-struct.c
@@ -159,6 +159,28 @@ _pygi_struct_new (PyTypeObject *type,
     return (PyObject *) self;
 }
 
+static PyObject *
+_struct_repr(PyGIStruct *self)
+{
+    PyObject* repr;
+    GIBaseInfo *info;
+    PyGPointer *pointer = (PyGPointer *)self;
+
+    info = _struct_get_info ((PyObject *)self);
+    if (info == NULL)
+        return NULL;
+
+    repr = PYGLIB_PyUnicode_FromFormat ("<%s.%s object at %p (%s at %p)>",
+                                        g_base_info_get_namespace (info),
+                                        g_base_info_get_name (info),
+                                        self, g_type_name (pointer->gtype),
+                                        pointer->pointer);
+
+    g_base_info_unref (info);
+
+    return repr;
+}
+
 void
 _pygi_struct_register_types (PyObject *m)
 {
@@ -168,6 +190,7 @@ _pygi_struct_register_types (PyObject *m)
     PyGIStruct_Type.tp_init = (initproc) _struct_init;
     PyGIStruct_Type.tp_dealloc = (destructor) _struct_dealloc;
     PyGIStruct_Type.tp_flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE);
+    PyGIStruct_Type.tp_repr = (reprfunc)_struct_repr;
 
     if (PyType_Ready (&PyGIStruct_Type))
         return;
diff --git a/gi/pygobject.c b/gi/pygobject.c
index 9839f493..a09ae367 100644
--- a/gi/pygobject.c
+++ b/gi/pygobject.c
@@ -722,8 +722,6 @@ pygobject_new_with_interfaces(GType gtype)
     PyObject *dict;
     PyTypeObject *py_parent_type;
     PyObject *bases;
-    PyObject *modules, *module;
-    gchar *type_name, *mod_name, *gtype_name;
 
     state = pyglib_gil_state_ensure();
 
@@ -739,32 +737,14 @@ pygobject_new_with_interfaces(GType gtype)
     /* set up __doc__ descriptor on type */
     PyDict_SetItemString(dict, "__doc__", pyg_object_descr_doc_get());
 
-    /* generate the pygtk module name and extract the base type name */
-    gtype_name = (gchar*)g_type_name(gtype);
-    if (g_str_has_prefix(gtype_name, "Gtk")) {
-	mod_name = "gtk";
-	gtype_name += 3;
-	type_name = g_strconcat(mod_name, ".", gtype_name, NULL);
-    } else if (g_str_has_prefix(gtype_name, "Gdk")) {
-	mod_name = "gtk.gdk";
-	gtype_name += 3;
-	type_name = g_strconcat(mod_name, ".", gtype_name, NULL);
-    } else if (g_str_has_prefix(gtype_name, "Atk")) {
-	mod_name = "atk";
-	gtype_name += 3;
-	type_name = g_strconcat(mod_name, ".", gtype_name, NULL);
-    } else if (g_str_has_prefix(gtype_name, "Pango")) {
-	mod_name = "pango";
-	gtype_name += 5;
-	type_name = g_strconcat(mod_name, ".", gtype_name, NULL);
-    } else {
-	mod_name = "__main__";
-	type_name = g_strconcat(mod_name, ".", gtype_name, NULL);
-    }
+    /* Something special to point out that it's not accessible through
+     * gi.repository */
+    o = PYGLIB_PyUnicode_FromString ("__gi__");
+    PyDict_SetItemString (dict, "__module__", o);
+    Py_DECREF (o);
 
     type = (PyTypeObject*)PyObject_CallFunction((PyObject *) Py_TYPE(py_parent_type),
-                                                "sNN", type_name, bases, dict);
-    g_free(type_name);
+                                                "sNN", g_type_name (gtype), bases, dict);
 
     if (type == NULL) {
 	PyErr_Print();
@@ -796,12 +776,6 @@ pygobject_new_with_interfaces(GType gtype)
         pyglib_gil_state_release(state);
 	return NULL;
     }
-    /* insert type name in module dict */
-    modules = PyImport_GetModuleDict();
-    if ((module = PyDict_GetItemString(modules, mod_name)) != NULL) {
-        if (PyObject_SetAttrString(module, gtype_name, (PyObject *)type) < 0)
-            PyErr_Clear();
-    }
 
     /* stash a pointer to the python class with the GType */
     Py_INCREF(type);
@@ -1126,15 +1100,32 @@ pygobject_hash(PyGObject *self)
 static PyObject *
 pygobject_repr(PyGObject *self)
 {
-    gchar buf[256];
+    PyObject *module, *repr;
+    gchar *module_str, *namespace;
+
+    module = PyObject_GetAttrString ((PyObject *)self, "__module__");
+    if (module == NULL)
+        return NULL;
+
+    if (!PYGLIB_PyUnicode_Check (module)) {
+        Py_DECREF (module);
+        return NULL;
+    }
+
+    module_str = PYGLIB_PyUnicode_AsString (module);
+    namespace = g_strrstr (module_str, ".");
+    if (namespace == NULL) {
+        namespace = module_str;
+    } else {
+        namespace += 1;
+    }
 
-    g_snprintf(buf, sizeof(buf),
-	       "<%s object at 0x%lx (%s at 0x%lx)>",
-	       Py_TYPE(self)->tp_name,
-	       (long)self,
-	       self->obj ? G_OBJECT_TYPE_NAME(self->obj) : "uninitialized",
-               (long)self->obj);
-    return PYGLIB_PyUnicode_FromString(buf);
+    repr = PYGLIB_PyUnicode_FromFormat ("<%s.%s object at %p (%s at %p)>",
+                                        namespace, Py_TYPE (self)->tp_name, self,
+                                        self->obj ? G_OBJECT_TYPE_NAME (self->obj) : "uninitialized",
+                                        self->obj);
+    Py_DECREF (module);
+    return repr;
 }
 
 
diff --git a/tests/test_gi.py b/tests/test_gi.py
index 4cdd1a42..abd2466c 100644
--- a/tests/test_gi.py
+++ b/tests/test_gi.py
@@ -1930,6 +1930,27 @@ class TestStructure(unittest.TestCase):
 
         self.assertRaises(TypeError, GIMarshallingTests.Union.method)
 
+    def test_repr(self):
+        self.assertRegexpMatches(
+            repr(GIMarshallingTests.PointerStruct()),
+            "<GIMarshallingTests.PointerStruct object at 0x[^\s]+ "
+            "\(void at 0x[^\s]+\)>")
+
+        self.assertRegexpMatches(
+            repr(GIMarshallingTests.SimpleStruct()),
+            "<GIMarshallingTests.SimpleStruct object at 0x[^\s]+ "
+            "\(void at 0x[^\s]+\)>")
+
+        self.assertRegexpMatches(
+            repr(GIMarshallingTests.Union()),
+            "<GIMarshallingTests.Union object at 0x[^\s]+ "
+            "\(GIMarshallingTestsUnion at 0x[^\s]+\)>")
+
+        self.assertRegexpMatches(
+            repr(GIMarshallingTests.BoxedStruct()),
+            "<GIMarshallingTests.BoxedStruct object at 0x[^\s]+ "
+            "\(GIMarshallingTestsBoxedStruct at 0x[^\s]+\)>")
+
 
 class TestGObject(unittest.TestCase):
 
@@ -2060,6 +2081,18 @@ class TestGObject(unittest.TestCase):
         self.assertEqual(object_.__grefcount__, 1)
         self.assertEqual(new_object.__grefcount__, 1)
 
+    def test_repr(self):
+        self.assertRegexpMatches(
+            repr(GIMarshallingTests.Object(int=42)),
+            "<GIMarshallingTests.Object object at 0x[^\s]+ "
+            "\(GIMarshallingTestsObject at 0x[^\s]+\)>")
+
+    def test_nongir_repr(self):
+        self.assertRegexpMatches(
+            repr(Gio.File.new_for_path("")),
+            "<__gi__.GLocalFile object at 0x[^\s]+ "
+            "\(GLocalFile at 0x[^\s]+\)>")
+
 # FIXME: Doesn't actually return the same object.
 #    def test_object_inout_same(self):
 #        object_ = GIMarshallingTests.Object()