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 703054 - Simplify signal handlers tracking
Simplify signal handlers tracking
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-06-25 13:01 UTC by Emanuele Aina
Modified: 2013-06-27 14:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
main-toolbar: Simplify signal handlers tracking (5.92 KB, patch)
2013-06-25 13:01 UTC, Emanuele Aina
committed Details | Review
embed: Simplify signal handlers tracking (4.36 KB, patch)
2013-06-25 13:01 UTC, Emanuele Aina
committed Details | Review
view-container: Simplify signal handlers tracking (3.89 KB, patch)
2013-06-25 13:01 UTC, Emanuele Aina
committed Details | Review
organize-collection-model: Simplify signal handlers tracking (2.96 KB, patch)
2013-06-25 13:01 UTC, Emanuele Aina
committed Details | Review
load-more-button: Simplify signal handlers tracking (2.12 KB, patch)
2013-06-25 13:01 UTC, Emanuele Aina
committed Details | Review

Description Emanuele Aina 2013-06-25 13:01:13 UTC
We currently keep track of a lot of signals by id, but by using
g_signal_connect_object() and g_signal_handlers_disconnect_by_func() we can
reduce considerably the needed bookeeping.
Comment 1 Emanuele Aina 2013-06-25 13:01:15 UTC
Created attachment 247725 [details] [review]
main-toolbar: Simplify signal handlers tracking

Use g_signal_connect_object() to make sure that signals are disconnected
when the toolbar is destroyed and use g_signal_handlers_disconnect_by_func()
otherwise.

This allows us to get rid of all the signal id member fields.
Comment 2 Emanuele Aina 2013-06-25 13:01:20 UTC
Created attachment 247726 [details] [review]
embed: Simplify signal handlers tracking

Use g_signal_connect_object() to make sure that signals are disconnected
when the toolbar is destroyed and use g_signal_handlers_disconnect_by_func()
otherwise.

This allows us to get rid of all the signal id member fields.
Comment 3 Emanuele Aina 2013-06-25 13:01:25 UTC
Created attachment 247727 [details] [review]
view-container: Simplify signal handlers tracking

Use g_signal_connect_object() to make sure that signals are disconnected
when the toolbar is destroyed and use g_signal_handlers_disconnect_by_func()
otherwise.

This allows us to get rid of all the signal id member fields.
Comment 4 Emanuele Aina 2013-06-25 13:01:29 UTC
Created attachment 247728 [details] [review]
organize-collection-model: Simplify signal handlers tracking

Use g_signal_connect_object() to make sure that signals are disconnected
when the toolbar is destroyed and use g_signal_handlers_disconnect_by_func()
otherwise.

This allows us to get rid of all the signal id member fields.
Comment 5 Emanuele Aina 2013-06-25 13:01:33 UTC
Created attachment 247729 [details] [review]
load-more-button: Simplify signal handlers tracking

Use g_signal_connect_object() to make sure that signals are disconnected
when the toolbar is destroyed and use g_signal_handlers_disconnect_by_func()
otherwise.

This allows us to get rid of all the signal id member fields.
Comment 6 Debarshi Ray 2013-06-27 13:12:41 UTC
Review of attachment 247725 [details] [review]:

Thanks for the patch. Committed after making the following style changes.

::: src/photos-main-toolbar.c
@@ +217,3 @@
+  g_signal_connect_object (priv->col_mngr, "active-changed",
+                           G_CALLBACK (photos_main_toolbar_active_changed),
+                           self, 0);

Better to put each argument on its own line.

@@ +390,3 @@
+  g_signal_connect_object (priv->sel_cntrlr, "selection-changed",
+                           G_CALLBACK (photos_main_toolbar_set_toolbar_title),
+                           self, G_CONNECT_SWAPPED);

Ditto.

@@ +487,3 @@
+  g_signal_connect_object (priv->src_mngr, "active-changed",
+                           G_CALLBACK (photos_main_toolbar_set_toolbar_title),
+                           self, G_CONNECT_SWAPPED);

Ditto.

@@ +493,3 @@
+  g_signal_connect_object (priv->mode_cntrlr, "window-mode-changed",
+                           G_CALLBACK (photos_main_toolbar_window_mode_changed),
+                           self, G_CONNECT_SWAPPED);

Ditto.

@@ +498,3 @@
+  g_signal_connect_object (priv->sel_cntrlr, "selection-mode-changed",
+                           G_CALLBACK (photos_main_toolbar_reset_toolbar_mode),
+                           self, G_CONNECT_SWAPPED);

Ditto.
Comment 7 Debarshi Ray 2013-06-27 13:37:38 UTC
Review of attachment 247726 [details] [review]:

Thanks for the patch. Committed after making the following minor changes.

::: src/photos-embed.c
@@ -200,3 @@
-      g_signal_handler_disconnect (priv->monitor, priv->no_results_change_id);
-      priv->no_results_change_id = 0;
-    }

Can remove this extra new line too.

@@ +224,3 @@
+      g_signal_connect_object (priv->monitor, "changes-pending",
+                               G_CALLBACK (photos_embed_changes_pending),
+                               self, G_CONNECT_SWAPPED);

Better to have each argument on its own line.
Comment 8 Debarshi Ray 2013-06-27 13:51:29 UTC
Review of attachment 247727 [details] [review]:

Thanks for the patch. Committed after making the following minor style changes.

::: src/photos-view-container.c
@@ +112,3 @@
+                           "value-changed",
+                           G_CALLBACK (photos_view_container_view_changed),
+                           self, G_CONNECT_SWAPPED);

Better to put each argument on its own line.

@@ +118,3 @@
+                           "notify::visible",
+                           G_CALLBACK (photos_view_container_view_changed),
+                           self, G_CONNECT_SWAPPED);

Ditto.
Comment 9 Debarshi Ray 2013-06-27 13:56:40 UTC
Review of attachment 247728 [details] [review]:

Thanks for the patch. Committed after making the following minor changes.

::: src/photos-organize-collection-model.c
@@ +213,3 @@
+  g_signal_connect_object (priv->manager, "object-removed",
+                           G_CALLBACK (photos_organize_collection_model_object_removed),
+                           self, 0);

Better to put each argument on its own line.
Comment 10 Debarshi Ray 2013-06-27 14:00:41 UTC
Review of attachment 247729 [details] [review]:

Thanks for the patch. Committed after making the following style change.

::: src/photos-load-more-button.c
@@ +121,3 @@
+  g_signal_connect_object (priv->offset_cntrlr, "count-changed",
+                           G_CALLBACK (photos_load_more_button_count_changed),
+                           self, 0);

Better to put each argument on its own line.
Comment 11 Debarshi Ray 2013-06-27 14:01:21 UTC
Thanks a lot for cleaning these up.