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 698283 - this.parent clashes with GtkWidget.parent
this.parent clashes with GtkWidget.parent
Status: RESOLVED WONTFIX
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal major
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2013-04-18 10:37 UTC by lamefun
Modified: 2014-02-24 16:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Rename this.parent to this.super for API version 2 (1.58 KB, patch)
2013-04-19 13:13 UTC, lamefun
rejected Details | Review
Allow using GObject.signal_* functions in place of Object methods (2.90 KB, patch)
2013-04-19 16:55 UTC, Giovanni Campagna
rejected Details | Review
Implement GObject.prototype.disconnect in JS (3.12 KB, patch)
2013-04-19 16:56 UTC, Giovanni Campagna
committed Details | Review

Description lamefun 2013-04-18 10:37:26 UTC
For example:

const MyWidget = new Lang.Class({
    Name: 'MyWidget',
    Extends: Gtk.Grid
});

let grid = new Gtk.Grid();
let widget = new MyWidget();

print(widget.parent);

Prints:

function _parent() {
    if (!this.__caller__) {
        throw new TypeError("The method 'parent' cannot be called");
    }
    var caller = this.__caller__;
    var name = caller._name;
    var parent = caller._owner.__super__;
    var previous = parent ? parent.prototype[name] : undefined;
    if (!previous) {
        throw new TypeError("The method '" + name + "' is not on the superclass");
    }
    return previous.apply(this, arguments);
}
Comment 1 Giovanni Campagna 2013-04-18 14:56:48 UTC
Yes, this is expected, because if you subclass your widget you want to be able to chain-up, and having parent resolve differently based on the scope you're in is not really feasible.
So just use widget.get_parent().
Comment 2 lamefun 2013-04-18 17:34:07 UTC
Maybe just rename this.parent to this.Parent (with big P) or this.Super, so it won't clash with property naming conventions at all?
Comment 3 Giovanni Campagna 2013-04-18 19:24:39 UTC
I don't think we can do that, at this point: there are too many uses of .parent() meaning the superclass in existing gjs code.
Comment 4 lamefun 2013-04-19 07:41:40 UTC
s/this.parent/this.Super/g could help. I think most if not all applications using Gjs still are developed by GNOME developers (because of lack of documentation), maybe there is still a chance to break compatibility?

Or maybe make a copy of Lang.Class (Oop.Class?) with fixed API for use in new applications?
Comment 5 lamefun 2013-04-19 13:13:02 UTC
Created attachment 241900 [details] [review]
Rename this.parent to this.super for API version 2

Patch that renames this.parent to this.Super for API version 2.

Depends on API version support in
https://bugzilla.gnome.org/show_bug.cgi?id=698360
Comment 6 Giovanni Campagna 2013-04-19 15:22:24 UTC
Really, I don't understand all the noise on this bug. Yes, Gtk.Widget.parent cannot be used, and you need Gtk.Widget.get_parent()
So?

It's definitely not worth a new major revision of the gjs API (or worse, a copy of the whole lang module). Also, it would cause sed pain on every shell extension out there, like we didn't break API enough for them...

If it were for me, this would be wontfix.
Comment 7 lamefun 2013-04-19 16:07:15 UTC
I didn't see that in practice, but what if a function is named 'parent'? There's no another alias for it, so it becomes completely inaccessible. Also there are GObject.connect, GObject.disconnect, GObject.connect_after, GObject.emit. They too clash with valid method names (GSocket.connect?) (these deserve another bug). There are lots of other issues like these, I even made a list of them:

https://live.gnome.org/NikitaChuraev/GlaringGjsApiIssues

I understand that many of the issues listed there are small, insignificant and can be easily worked around (eg. rename GSocket.connect to GSocket._connect or something like this), but combined they contribute to the feeling of clumsiness of Gjs platform. It just doesn't feel right. These are really basic thing: loading from file, adding text to GtkTextView, connecting via a socket.

Is that really not possible to fix these issues now? I've heard that Gjs documentation is coming, IDE, lowering the bar significantly for new developers, and then it'll be even harder to fix.

Suggestions for both issues: maybe move Lang.Class into Oop.Class or another place and make Lang.Class a simple compatibility shim? It'll break MyClass instanceof Lang.Class, but I don't think anyone does that at this point.

function Class(params) {
    let newClass = new Oop.Class(params);
    if (newClass.prototype.parent === undefined)
        newClass.prototype.parent = Oop._super;
    return newClass;
}

As for GObject.connect and others, mark them as deprecated without removing them and replace them with external methods (as in C):

GObject.connect
GObject.connect_after
GObject.disconnect
GObject.emit
Comment 8 Giovanni Campagna 2013-04-19 16:44:25 UTC
You can use My.Foo.prototype.parent.call(obj, args...); if you have an hypothetical method called "parent".
And you can use GObject.Object.prototype.connect.call(...) if .connect is Gio.Socket.prototype.connect.

I agree that
GObject.signal_connect() / GObject.signal_disconnect() / GObject.signal_emit() are useful, to avoid the long incantation, because .connect() as a method is more likely than .parent().
Easy to do in a backward compatible way with overrides, patch coming.
Comment 9 Giovanni Campagna 2013-04-19 16:55:59 UTC
Created attachment 241931 [details] [review]
Allow using GObject.signal_* functions in place of Object methods

GObject.Object.connect() and friends might be unavailable because
shadowed by another method in the prototype chain (such as
g_socket_connect()).
In that case, it is nice to use the C equivalents, which are
GObject.signal_connect(), GObject.signal_handler_disconnect() and
GObject.signal_emit_by_name().
Comment 10 Giovanni Campagna 2013-04-19 16:56:22 UTC
Created attachment 241932 [details] [review]
Implement GObject.prototype.disconnect in JS

Because GObject.signal_handler_disconnect() is introspectable now.
Comment 11 Giovanni Campagna 2013-04-19 16:57:28 UTC
Both my patches are depending on bug 698382 for g_signal_handler_disconnect().
Comment 12 Giovanni Campagna 2014-01-19 13:56:37 UTC
Uhm... this patches have been waiting for one year, maybe it's time for a review?
Comment 13 Jasper St. Pierre (not reading bugmail) 2014-01-19 14:53:55 UTC
Review of attachment 241931 [details] [review]:

I don't like this. I'd prefer to see GObject.Object.prototype.connect.call(obj, Lang.bind(...));
Comment 14 Jasper St. Pierre (not reading bugmail) 2014-01-19 14:54:23 UTC
Review of attachment 241932 [details] [review]:

OK. Is there a reason we can't do the same for connect? The custom value marshalling?
Comment 15 Jasper St. Pierre (not reading bugmail) 2014-01-19 14:54:36 UTC
Review of attachment 241900 [details] [review]:

No.
Comment 16 Giovanni Campagna 2014-01-19 14:57:35 UTC
(In reply to comment #13)
> Review of attachment 241931 [details] [review]:
> 
> I don't like this. I'd prefer to see GObject.Object.prototype.connect.call(obj,
> Lang.bind(...));

So we should bind automatically? And to what?

(In reply to comment #14)
> Review of attachment 241932 [details] [review]:
> 
> OK. Is there a reason we can't do the same for connect? The custom value
> marshalling?

Yes. Generic GClosure marshalling doesn't use GSignal type information (or GIRepository type information, which we should start using...), so it's less accurate.
Comment 17 Jasper St. Pierre (not reading bugmail) 2014-01-19 15:13:23 UTC
(In reply to comment #16)
> So we should bind automatically? And to what?

What? No, I meant users should call GObject.prototype.connect.call(obj, foo); themselves, just like they need to do for any other "conflicted" method.

> (In reply to comment #14)
> Yes. Generic GClosure marshalling doesn't use GSignal type information (or
> GIRepository type information, which we should start using...), so it's less
> accurate.

OK. It would be better if we could just do connect(GjsPrivate.create_signal_marshaller(obj, signalName, myFunction)); instead, but I won't budge on it.
Comment 18 Giovanni Campagna 2014-01-19 15:22:32 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > So we should bind automatically? And to what?
> 
> What? No, I meant users should call GObject.prototype.connect.call(obj, foo);
> themselves, just like they need to do for any other "conflicted" method.

Well, yes, but "connect" is a fairly common name, and g_signal_connect() is already part of the C API...
Comment 19 Giovanni Campagna 2014-02-24 16:02:01 UTC
I pushed the code cleanup for disconnect(), but I understand
we're not going to touch connect() and emit(). Prototypes exists,
and people can patch GObject in their apps if they want.

Attachment 241932 [details] pushed as 9a54db2 - Implement GObject.prototype.disconnect in JS