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 765210 - Simplify / modernize GObject boilerplate
Simplify / modernize GObject boilerplate
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-18 13:12 UTC by Debarshi Ray
Modified: 2016-04-18 18:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pty: Use G_ADD_PRIVATE instead of g_type_class_add_private (1.46 KB, patch)
2016-04-18 13:13 UTC, Debarshi Ray
committed Details | Review
pty: Simplify the code by removing the separate private struct (6.80 KB, patch)
2016-04-18 13:13 UTC, Debarshi Ray
rejected Details | Review
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE (1.14 KB, patch)
2016-04-18 14:12 UTC, Debarshi Ray
none Details | Review
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE (1.17 KB, patch)
2016-04-18 16:26 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2016-04-18 13:12:33 UTC
VtePty is a final class, and it was like that even before the most recent ABI bump. Since the definitions of the instance and class structs are not exported, there is no reason to use a separate private structure.

In case it was a deliberate decision to use a private struct, we should at least use the modern G_ADD_PRIVATE / my_object_get_instance_private combination instead of g_type_class_add_private / G_TYPE_INSTANCE_GET_PRIVATE.
Comment 1 Debarshi Ray 2016-04-18 13:13:36 UTC
Created attachment 326251 [details] [review]
pty: Use G_ADD_PRIVATE instead of g_type_class_add_private
Comment 2 Debarshi Ray 2016-04-18 13:13:58 UTC
Created attachment 326252 [details] [review]
pty: Simplify the code by removing the separate private struct
Comment 3 Debarshi Ray 2016-04-18 14:12:03 UTC
Created attachment 326260 [details] [review]
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE
Comment 4 Christian Persch 2016-04-18 16:05:43 UTC
Comment on attachment 326251 [details] [review]
pty: Use G_ADD_PRIVATE instead of g_type_class_add_private

Ok (master only)
Comment 5 Christian Persch 2016-04-18 16:07:35 UTC
Comment on attachment 326252 [details] [review]
pty: Simplify the code by removing the separate private struct

No, this isn't where I want to go. These structs need to remain reparate (in fact the goal is to make all the public vte API just call functions on private C++ classes, like VteTerminalPrivate already does. Just haven't got to VtePty yet.)
Comment 6 Christian Persch 2016-04-18 16:08:37 UTC
Comment on attachment 326260 [details] [review]
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE

Ok to change this to the faster accessor, but this is the one place where I don't want to use IMPL; just spell it out.
Comment 7 Debarshi Ray 2016-04-18 16:14:57 UTC
Comment on attachment 326251 [details] [review]
pty: Use G_ADD_PRIVATE instead of g_type_class_add_private

Pushed to master.
Comment 8 Debarshi Ray 2016-04-18 16:26:30 UTC
Created attachment 326277 [details] [review]
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE
Comment 9 Debarshi Ray 2016-04-18 18:00:17 UTC
Comment on attachment 326277 [details] [review]
Use my_object_get_instance_private, not G_TYPE_INSTANCE_GET_PRIVATE

Thanks for the review, Christian.