Merge lp:~larsu/indicator-power/use-gsettings-actions into lp:indicator-power/14.04

[Lars Karlitski]

Use GSettingsActions instead of g_settings_bind

This also fixes a bug: the actions were created stateless, but assigned a state later.

[Sebastien Bacher (seb128)]

looks fine to me, works/fixes the issue with glib 2.39 as well

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:217
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/72/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-armhf-ci/9/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/72/rebuild

review: Approve (continuous-integration)

[Allison Karlitskaya (desrt)]

Looks pretty good -- but the fact that you're keeping the action around is suspicious. Can you look into that? Either it's being used in a way that would be valid for a GSimpleAction but not a GSettingsAction or it's not being used at all... Maybe there is more code that can be cut...

[lp:~larsu/indicator-power/use-gsettings-actions]

218. By Lars Karlitski on 2013-11-29

Don't keep GSettingsActions around

They were only kept around so that a notify handler could be disconnected.
Since the GSettings object is the canonical source of that data, it makes more
sense to listen to changes there instead of on the action objects.

That was already done for the one remaining key in the schema. This patch
changes that to rebuild the header when any key changes.

[Allison Karlitskaya (desrt)]

+ g_signal_connect_swapped (p->settings, "changed", G_CALLBACK(rebuild_header_now), self);

Strictly speaking, you should be disconnecting this...

[Allison Karlitskaya (desrt)]

...and you already are. Sorry -- misread the patch and didn't see that you were simply modifying an existing connection to the same object.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:218
http://jenkins.qa.ubuntu.com/job/indicator-power-ci/73/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-trusty-armhf-ci/10/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-ci/73/rebuild

review: Approve (continuous-integration)

=== modified file 'src/service.c'
--- src/service.c	2013-11-08 17:23:15 +0000
+++ src/service.c	2013-11-29 15:40:05 +0000
@@ -113,8 +113,6 @@
 
   GSimpleActionGroup * actions;
   GSimpleAction * header_action;
-  GSimpleAction * show_time_action;
-  GSimpleAction * show_percentage_action;
   GSimpleAction * battery_level_action;
   GSimpleAction * brightness_action;
 
@@ -715,39 +713,12 @@
 ****
 ***/
 
-/* toggles the state */
-static void
-on_toggle_action_activated (GSimpleAction * simple,
-                            GVariant      * parameter G_GNUC_UNUSED,
-                            gpointer        unused    G_GNUC_UNUSED)
-{
-  GVariant * v = g_action_get_state (G_ACTION (simple));
-  gboolean flag = g_variant_get_boolean (v);
-  g_simple_action_set_state (simple, g_variant_new_boolean (!flag));
-  g_variant_unref (v);
-}
-
-static gboolean
-settings_to_action_state (GValue    * value,
-                          GVariant  * variant,
-                          gpointer    user_data  G_GNUC_UNUSED)
-{
-  g_value_set_variant (value, variant);
-  return TRUE;
-}
-
-static GVariant *
-action_state_to_settings (const GValue       * value,
-                          const GVariantType * expected_type G_GNUC_UNUSED,
-                          gpointer             user_data     G_GNUC_UNUSED)
-{
-  return g_value_dup_variant (value);
-}
-
 static void
 init_gactions (IndicatorPowerService * self)
 {
   GSimpleAction * a;
+  GAction * show_time_action;
+  GAction * show_percentage_action;
   priv_t * p = self->priv;
 
   GActionEntry entries[] = {
@@ -781,32 +752,17 @@
   p->brightness_action = a;
 
   /* add the show-time action */
-  a = g_simple_action_new ("show-time", NULL);
-  g_settings_bind_with_mapping (p->settings, SETTINGS_SHOW_TIME_S,
-                                a, "state",
-                                G_SETTINGS_BIND_DEFAULT,
-                                settings_to_action_state,
-                                action_state_to_settings,
-                                NULL, NULL);
-  g_signal_connect (a, "activate", G_CALLBACK(on_toggle_action_activated), self);
-  g_signal_connect_swapped (a, "notify", G_CALLBACK(rebuild_header_now), self);
-  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
-  p->show_time_action = a;
+  show_time_action = g_settings_create_action (p->settings, "show-time");
+  g_action_map_add_action (G_ACTION_MAP(p->actions), show_time_action);
 
   /* add the show-percentage action */
-  a = g_simple_action_new ("show-percentage", NULL);
-  g_settings_bind_with_mapping (p->settings, SETTINGS_SHOW_PERCENTAGE_S,
-                                a, "state",
-                                G_SETTINGS_BIND_DEFAULT,
-                                settings_to_action_state,
-                                action_state_to_settings,
-                                NULL, NULL);
-  g_signal_connect (a, "activate", G_CALLBACK(on_toggle_action_activated), self);
-  g_signal_connect_swapped (a, "notify", G_CALLBACK(rebuild_header_now), self);
-  g_action_map_add_action (G_ACTION_MAP(p->actions), G_ACTION(a));
-  p->show_percentage_action = a;
+  show_percentage_action = g_settings_create_action (p->settings, "show-percentage");
+  g_action_map_add_action (G_ACTION_MAP(p->actions), show_percentage_action);
 
   rebuild_header_now (self);
+
+  g_object_unref (show_time_action);
+  g_object_unref (show_percentage_action);
 }
 
 /***
@@ -1009,20 +965,6 @@
       g_clear_object (&p->settings);
     }
 
-  if (p->show_time_action != NULL)
-    {
-      g_signal_handlers_disconnect_by_data (p->show_time_action, self);
-
-      g_clear_object (&p->show_time_action);
-    }
-
-  if (p->show_percentage_action != NULL)
-    {
-      g_signal_handlers_disconnect_by_data (p->show_percentage_action, self);
-
-      g_clear_object (&p->show_percentage_action);
-    }
-
   g_clear_object (&p->brightness_action);
   g_clear_object (&p->battery_level_action);
   g_clear_object (&p->header_action);
@@ -1057,8 +999,7 @@
 
   init_gactions (self);
 
-  g_signal_connect_swapped (p->settings, "changed::" SETTINGS_ICON_POLICY_S,
-                            G_CALLBACK(rebuild_header_now), self);
+  g_signal_connect_swapped (p->settings, "changed", G_CALLBACK(rebuild_header_now), self);
 
   p->own_id = g_bus_own_name (G_BUS_TYPE_SESSION,
                               BUS_NAME,