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 664436 - Port gnome-shell to the new gjs class framework
Port gnome-shell to the new gjs class framework
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on: 664437
Blocks:
 
 
Reported: 2011-11-20 20:08 UTC by Giovanni Campagna
Modified: 2011-11-24 08:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port PopupMenu to new Lang.Class framework (21.86 KB, patch)
2011-11-20 20:11 UTC, Giovanni Campagna
committed Details | Review
Port PanelMenu to new class framework (13.77 KB, patch)
2011-11-20 20:11 UTC, Giovanni Campagna
committed Details | Review
Port message tray sources and notifications to class framework (27.58 KB, patch)
2011-11-20 20:11 UTC, Giovanni Campagna
committed Details | Review
Port ModalDialog to the class framework (9.20 KB, patch)
2011-11-20 20:11 UTC, Giovanni Campagna
committed Details | Review
Port all classes with inheritance to class framework (30.71 KB, patch)
2011-11-20 20:12 UTC, Giovanni Campagna
committed Details | Review
Port GDM and Caribou to GDBus (7.29 KB, patch)
2011-11-20 20:12 UTC, Giovanni Campagna
committed Details | Review
Port everything to class framework (61.26 KB, patch)
2011-11-20 20:12 UTC, Giovanni Campagna
committed Details | Review

Description Giovanni Campagna 2011-11-20 20:08:36 UTC
gjs has recently gained a new framework to ease writing classes in JS, so I thought I would be interesting to see how the shell would look with it.
Turns out it's pretty clean and saves some lines of code, but see for yourself.
Comment 1 Giovanni Campagna 2011-11-20 20:11:30 UTC
Created attachment 201759 [details] [review]
Port PopupMenu to new Lang.Class framework

The Lang module in gjs has recently gained a small yet powerful
Class framework, that should help improve the readability of code
when using complex inheritance.
This commit starts porting shell code, by rewriting all classes in
popupMenu.js (and all derived classes) to Lang.Class.
Comment 2 Giovanni Campagna 2011-11-20 20:11:39 UTC
Created attachment 201760 [details] [review]
Port PanelMenu to new class framework

Second patch in the class framework, now it's the turn of
PanelMenu (buttons, menus and status indicators).
Comment 3 Giovanni Campagna 2011-11-20 20:11:47 UTC
Created attachment 201761 [details] [review]
Port message tray sources and notifications to class framework

Third step in the class framework port, now it's the turn of
MessageTray.Source and MessageTray.Notification, as well as
the various implementations around the shell.
Comment 4 Giovanni Campagna 2011-11-20 20:11:55 UTC
Created attachment 201762 [details] [review]
Port ModalDialog to the class framework

Similar to the previous commits, time to port shell modal dialogs
to the class framework.
Comment 5 Giovanni Campagna 2011-11-20 20:12:03 UTC
Created attachment 201763 [details] [review]
Port all classes with inheritance to class framework

All classes that have at least one other derived class (and thus
benefit from the framework) have been now ported. These includes
NMDevice, SearchProvider, AltTab.SwitcherList, and some other
stuff around.
Comment 6 Giovanni Campagna 2011-11-20 20:12:10 UTC
Created attachment 201764 [details] [review]
Port GDM and Caribou to GDBus

During the mass port to GDBus, this classes were left out (probably
because they didn't exist at the time). Now it's time to update
them.
Comment 7 Giovanni Campagna 2011-11-20 20:12:17 UTC
Created attachment 201765 [details] [review]
Port everything to class framework

The last patch in the sequence. Every place that was previously
setting prototype has been ported to Lang.Class, to make code more
concise and allow for better toString().
Comment 8 Jasper St. Pierre (not reading bugmail) 2011-11-21 02:22:52 UTC
Review of attachment 201764 [details] [review]:

Can you re-file this in a different bug?
Comment 9 Jasper St. Pierre (not reading bugmail) 2011-11-21 02:24:32 UTC
Review of attachment 201761 [details] [review]:

::: js/ui/notificationDaemon.js
@@ +478,3 @@
 
+const Source = new Lang.Class({
+    Name: 'NotificationDaemonSource',

Names do not match.

::: js/ui/status/bluetooth.js
@@ +336,3 @@
 
+const Source = new Lang.Class({
+    Name: 'BluetoothSource',

Names do not match.

::: js/ui/windowAttentionHandler.js
@@ +47,3 @@
 
+const Source = new Lang.Class({
+    Name: 'WindowAttentionSource',

Names do not match.
Comment 10 Giovanni Campagna 2011-11-21 13:07:23 UTC
(In reply to comment #9)

This is actually intended: Class.Name is meant for debug and it's shown out for context in toString. Having all those as [object Source] would not be a great improvement over [object Object]; OTOH, having to refer to them as Bluetooth.BluetoothSource or NotificationDaemon.NotificationDaemonSource would be unnecessary repetition.
I expect that when we start subclassing GObject this will become common, as GTypes need proper namespacing, while JS names don't.
Comment 11 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:01:16 UTC
Review of attachment 201759 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 12 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:01:55 UTC
Review of attachment 201760 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 13 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:02:08 UTC
Review of attachment 201761 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 14 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:04:58 UTC
Review of attachment 201762 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 15 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:05:06 UTC
Review of attachment 201762 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 16 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:07:13 UTC
Review of attachment 201765 [details] [review]:

I didn't look too closely. I assume that you've got the code running with these patches and everything works. Feel free to commit after Owen has made a release.
Comment 17 Jasper St. Pierre (not reading bugmail) 2011-11-21 21:07:50 UTC
Review of attachment 201763 [details] [review]:

Feel free to commit after Owen has made a release.
Comment 18 Jasper St. Pierre (not reading bugmail) 2011-11-23 19:13:33 UTC
3.3.2 is released, so feel free to commit.
Comment 19 Giovanni Campagna 2011-11-24 08:51:19 UTC
Attachment 201759 [details] pushed as 2b57603 - Port PopupMenu to new Lang.Class framework
Attachment 201760 [details] pushed as 566bdb5 - Port PanelMenu to new class framework
Attachment 201761 [details] pushed as b356aa8 - Port message tray sources and notifications to class framework
Attachment 201762 [details] pushed as 987099e - Port ModalDialog to the class framework
Attachment 201763 [details] pushed as d6b6f81 - Port all classes with inheritance to class framework
Attachment 201764 [details] pushed as 0996174 - Port GDM and Caribou to GDBus
Attachment 201765 [details] pushed as 17c46c2 - Port everything to class framework
Comment 20 Giovanni Campagna 2011-11-24 08:54:12 UTC
(In reply to comment #8)
> Review of attachment 201764 [details] [review]:
> 
> Can you re-file this in a different bug?

By mistake i pushed this togheter with the rest. Shall I revert, or is it ok?
(next time I'll check, I promise)