Merge lp:~macslow/notify-osd/gravity into lp:notify-osd/trunk

[Mirco Müller]

[Mirco Müller (macslow)]

Added gconf-key "/apps/notify-osd/gravity" to allow selecting between positioning bubbles at NorthEast (top-right, 1) or East (vertically centered at right of screen, 2) gravity.

To test it use these commands once notify-osd is started:

gconftool-2 --set /apps/notify-osd/gravity --type int 1

or

gconftool-2 --set /apps/notify-osd/gravity --type int 2

If no gconf-key is found or an out-of-bounds value is set it'll default to NorthEast gravity. If you changed the gravity gconf-key the change will only apply to the next bubble coming showing up. It will not update the position of an already shown bubble.

There is no gconf-schema file coming along with this intentionally.

[David Barth (dbarth)]

Thanks for putting that together quickly.

Remarks
- line 57: the comment is wrong (mentions a font size)
- it looks like the error rollback code could be factored into a helper function; but that's fine for a quick fix
- unit tests, good

review: Approve

[lp:~macslow/notify-osd/gravity]

389. By Mirco Müller on 2009-09-01

fixed comment, added error-handling function to avoid redundant code

[Mirco Müller (macslow)]

> Thanks for putting that together quickly.
>
> Remarks
> - line 57: the comment is wrong (mentions a font size)
> - it looks like the error rollback code could be factored into a helper
> function; but that's fine for a quick fix
> - unit tests, good

Fixed comment and added error-handling helper-function in rev. 389

=== modified file 'src/defaults.c'
--- src/defaults.c	2009-08-26 11:17:55 +0000
+++ src/defaults.c	2009-09-01 08:46:33 +0000
@@ -84,12 +84,14 @@
 	PROP_TEXT_BODY_SIZE,
 	PROP_PIXELS_PER_EM,
 	PROP_SYSTEM_FONT_SIZE,
-	PROP_SCREEN_DPI
+	PROP_SCREEN_DPI,
+	PROP_GRAVITY
 };
 
 enum
 {
 	VALUE_CHANGED,
+	GRAVITY_CHANGED,
 	LAST_SIGNAL
 };
 
@@ -138,6 +140,7 @@
 #define DEFAULT_PIXELS_PER_EM        10.0f
 #define DEFAULT_SYSTEM_FONT_SIZE     10.0f
 #define DEFAULT_SCREEN_DPI           96.0f
+#define DEFAULT_GRAVITY              GRAVITY_NORTH_EAST
 
 /* these values are interpreted as milliseconds-measurements and do comply to
  * the visual guide for jaunty-notifications */
@@ -155,9 +158,11 @@
 /* GConf-trees to watch */
 #define GCONF_UI_TREE             "/desktop/gnome/interface"
 #define GCONF_FONT_TREE           "/desktop/gnome/font_rendering"
+#define GCONF_NOSD_TREE           "/apps/notify-osd"
 
 /* notify-osd settings */
 #define GCONF_MULTIHEAD_MODE "/apps/notify-osd/multihead_mode"
+#define GCONF_GRAVITY        "/apps/notify-osd/gravity"
 
 static guint g_defaults_signals[LAST_SIGNAL] = { 0 };
 
@@ -272,6 +277,37 @@
 }
 
 static void
+_get_gravity (Defaults* self)
+{
+	GError* error = NULL;
+	Gravity gravity;
+
+	if (!IS_DEFAULTS (self))
+		return;
+
+	// grab current gravity-setting for notify-osd from gconf
+	error = NULL;
+	gravity = gconf_client_get_int (self->context, GCONF_GRAVITY, &error);
+	if (error)
+	{
+		// if something went wrong, assume "Sans 10" and continue
+		gravity = DEFAULT_GRAVITY;
+
+		g_warning ("%s(): Got error \"%s\"\n",
+		           G_STRFUNC,
+		           error->message);
+		g_error_free (error);
+	}
+
+	// protect against out-of-bounds values for gravity
+	if (gravity != GRAVITY_EAST && gravity != GRAVITY_NORTH_EAST)
+		gravity = DEFAULT_GRAVITY;
+
+	// update stored DPI-value
+	g_object_set (self, "gravity", gravity, NULL);
+}
+
+static void
 _font_changed (GConfClient* client,
 	       guint        cnxn_id,
 	       GConfEntry*  entry,
@@ -373,6 +409,27 @@
 	g_signal_emit (defaults, g_defaults_signals[VALUE_CHANGED], 0);
 }
 
+static void
+_gravity_changed (GConfClient* client,
+                  guint        cnxn_id,
+                  GConfEntry*  entry,
+                  gpointer     data)
+{
+	Defaults* defaults;
+
+	if (!data)
+		return;
+
+	defaults = (Defaults*) data;
+	if (!IS_DEFAULTS (defaults))
+		return;
+
+    	// grab gravity setting for notify-osd from gconf
+	_get_gravity (defaults);
+
+	g_signal_emit (defaults, g_defaults_signals[GRAVITY_CHANGED], 0);
+}
+
 static gdouble
 _get_average_char_width (Defaults* self)
 {
@@ -590,8 +647,10 @@
 	gconf_client_notify_remove (defaults->context, defaults->notifier[2]);
 	gconf_client_notify_remove (defaults->context, defaults->notifier[3]);
 	gconf_client_notify_remove (defaults->context, defaults->notifier[4]);
+	gconf_client_notify_remove (defaults->context, defaults->notifier[5]);
 	gconf_client_remove_dir (defaults->context, GCONF_UI_TREE, NULL);
 	gconf_client_remove_dir (defaults->context, GCONF_FONT_TREE, NULL);
+	gconf_client_remove_dir (defaults->context, GCONF_NOSD_TREE, NULL);
 	g_object_unref (defaults->context);
 
 	if (defaults->bubble_shadow_color)
@@ -697,6 +756,23 @@
 		return;
 	}
 
+	/* register watching all settings for notify-osd */
+	error = NULL;
+	gconf_client_add_dir (self->context,
+			      GCONF_NOSD_TREE,
+			      GCONF_CLIENT_PRELOAD_NONE,
+			      &error);
+	if (error)
+	{
+		gconf_client_remove_dir (self->context, GCONF_UI_TREE, NULL);
+		gconf_client_remove_dir (self->context, GCONF_FONT_TREE, NULL);
+		g_object_unref (self->context);
+		g_warning ("defaults_init(): Got error \"%s\"\n",
+		           error->message);
+		g_error_free (error);
+		return;
+	}
+
 	/* hook up notifier for font-name/size changes */
 	error = NULL;
 	self->notifier[0] = gconf_client_notify_add (self->context,
@@ -801,6 +877,30 @@
 		g_error_free (error);
 		return;
 	}
+
+	// hook up notifiier for gravity changes
+	error = NULL;
+	self->notifier[5] = gconf_client_notify_add (self->context,
+						     GCONF_NOSD_TREE,
+						     _gravity_changed,
+						     (gpointer) self,
+						     NULL,
+						     &error);
+	if (error)
+	{
+		gconf_client_notify_remove (self->context, self->notifier[0]);
+		gconf_client_notify_remove (self->context, self->notifier[1]);
+		gconf_client_notify_remove (self->context, self->notifier[2]);
+		gconf_client_notify_remove (self->context, self->notifier[3]);
+		gconf_client_notify_remove (self->context, self->notifier[4]);
+		gconf_client_remove_dir (self->context, GCONF_UI_TREE, NULL);
+		gconf_client_remove_dir (self->context, GCONF_FONT_TREE, NULL);
+		g_object_unref (self->context);
+		g_warning ("defaults_init(): Got error \"%s\"\n",
+		           error->message);
+		g_error_free (error);
+		return;
+	}
 }
 
 static void
@@ -975,6 +1075,10 @@
 			g_value_set_double (value, defaults->screen_dpi);
 		break;
 
+		case PROP_GRAVITY:
+			g_value_set_int (value, defaults->gravity);
+		break;
+
 		default :
 			G_OBJECT_WARN_INVALID_PROPERTY_ID (gobject, prop, spec);
 		break;
@@ -1191,6 +1295,10 @@
 			defaults->screen_dpi = g_value_get_double (value);
 		break;
 
+		case PROP_GRAVITY:
+			defaults->gravity = g_value_get_int (value);
+		break;
+
 		default :
 			G_OBJECT_WARN_INVALID_PROPERTY_ID (gobject, prop, spec);
 		break;
@@ -1241,6 +1349,7 @@
 	GParamSpec*   property_pixels_per_em;
 	GParamSpec*   property_system_font_size;
 	GParamSpec*   property_screen_dpi;
+	GParamSpec*   property_gravity;
 
 	gobject_class->constructed  = defaults_constructed;
 	gobject_class->dispose      = defaults_dispose;
@@ -1259,6 +1368,17 @@
 		G_TYPE_NONE,
 		0);
 
+	g_defaults_signals[GRAVITY_CHANGED] = g_signal_new (
+		"gravity-changed",
+		G_OBJECT_CLASS_TYPE (gobject_class),
+		G_SIGNAL_RUN_LAST,
+		G_STRUCT_OFFSET (DefaultsClass, gravity_changed),
+		NULL,
+		NULL,
+		g_cclosure_marshal_VOID__VOID,
+		G_TYPE_NONE,
+		0);
+
 	property_desktop_width = g_param_spec_int (
 				"desktop-width",
 				"desktop-width",
@@ -1737,6 +1857,18 @@
 					 PROP_SCREEN_DPI,
 					 property_screen_dpi);
 
+	property_gravity = g_param_spec_int (
+				"gravity",
+				"gravity",
+				"Positional hint for placing bubbles",
+				0,
+				2,
+				DEFAULT_GRAVITY,
+				G_PARAM_CONSTRUCT |
+				G_PARAM_READWRITE);
+	g_object_class_install_property (gobject_class,
+					 PROP_GRAVITY,
+					 property_gravity);
 }
 
 /*-- public API --------------------------------------------------------------*/
@@ -2500,3 +2632,16 @@
 
 	g_debug ("top corner at: %d, %d", *x, *y);
 }
+
+Gravity
+defaults_get_gravity (Defaults* self)
+{
+	if (!self || !IS_DEFAULTS (self))
+		return GRAVITY_NONE;
+
+	Gravity gravity;
+
+	g_object_get (self, "gravity", &gravity, NULL);
+
+	return gravity;
+}

=== modified file 'src/defaults.h'
--- src/defaults.h	2009-07-31 14:19:48 +0000
+++ src/defaults.h	2009-09-01 08:46:33 +0000
@@ -54,6 +54,13 @@
 typedef struct _Defaults      Defaults;
 typedef struct _DefaultsClass DefaultsClass;
 
+typedef enum
+{
+        GRAVITY_NONE = 0,
+        GRAVITY_NORTH_EAST, // top-right of screen
+        GRAVITY_EAST        // vertically centered at right of screen
+} Gravity;
+
 /* instance structure */
 struct _Defaults
 {
@@ -61,7 +68,7 @@
 
 	/* private */
 	GConfClient* context;
-	guint        notifier[5];
+	guint        notifier[6];
 	gint         desktop_width;
 	gint         desktop_height;
 	gint         desktop_top;
@@ -100,6 +107,7 @@
 	gdouble      pixels_per_em;
 	gdouble      system_font_size;
 	gdouble      screen_dpi;
+	Gravity      gravity;
 };
 
 /* class structure */
@@ -111,6 +119,9 @@
 	void (*value_changed) (Defaults* defaults); /* used to "inform" bubble
 						    ** about any changes in
 						    ** rendering and position */
+	void (*gravity_changed) (Defaults* defaults); // used to "inform" about
+						      // gravity/position change
+						      // for bubbles
 };
 
 GType defaults_get_type (void);
@@ -238,6 +249,9 @@
 void
 defaults_get_top_corner (Defaults *self, gint *x, gint *y);
 
+Gravity
+defaults_get_gravity (Defaults *self);
+
 G_END_DECLS
 
 #endif /* __DEFAULTS_H */

=== modified file 'src/display.c'
--- src/display.c	2009-08-26 09:35:02 +0000
+++ src/display.c	2009-08-31 13:56:28 +0000
@@ -85,9 +85,9 @@
 	// TODO: with multi-head, in focus follow mode, there may be enough
 	// space left on the top monitor
 
-	switch (bubble_get_placement (bubble))
+	switch (defaults_get_gravity (d))
 	{
-		case PLACEMENT_OLD:
+		case GRAVITY_NORTH_EAST:
 			async = stack_find_bubble_on_display (self);
 			if (async != NULL)
 			{
@@ -98,7 +98,7 @@
 			}
 		break;
 
-		case PLACEMENT_NEW:
+		case GRAVITY_EAST:
 			y += defaults_get_desktop_height (d) / 2 -
 			     EM2PIXELS (defaults_get_bubble_vert_gap (d) / 2.0f, d) -
 			     bubble_get_height (bubble) +
@@ -106,7 +106,7 @@
 		break;
 
 		default:
-			g_warning ("Unhandled bubble placement!\n");
+			g_warning ("Unhandled gravity!\n");
 		break;
 	}
 
@@ -241,9 +241,9 @@
 
 	d = self->defaults;
 
-	switch (bubble_get_placement (bubble))
+	switch (defaults_get_gravity (d))
 	{
-		case PLACEMENT_OLD:
+		case GRAVITY_NORTH_EAST:
 			if (sync_bubble != NULL && bubble_is_visible (sync_bubble))
 			{
 				d = self->defaults;
@@ -258,7 +258,7 @@
 			}
 		break;
 
-		case PLACEMENT_NEW:
+		case GRAVITY_EAST:
 			if (sync_bubble != NULL && bubble_is_visible (sync_bubble))
 			{
 				y += defaults_get_desktop_height (d) / 2 -
@@ -275,7 +275,7 @@
 		break;
 
 		default:
-			g_warning ("Unhandled bubble placement!\n");
+			g_warning ("Unhandled gravity!\n");
 		break;
 	}
 

=== modified file 'src/stack.h'
--- src/stack.h	2009-08-21 10:43:50 +0000
+++ src/stack.h	2009-08-31 13:56:28 +0000
@@ -61,7 +61,6 @@
 	Observer* observer;
 	GList*    list;
 	guint     next_id;
-
 };
 
 /* class structure */

=== modified file 'tests/test-defaults.c'
--- tests/test-defaults.c	2009-08-04 17:34:48 +0000
+++ tests/test-defaults.c	2009-08-31 13:56:28 +0000
@@ -147,6 +147,23 @@
 	g_object_unref (defaults);
 }
 
+static void
+test_defaults_get_gravity ()
+{
+        Defaults* defaults = defaults_new ();
+
+        // upon creation the gravity should not be unset
+        g_assert_cmpint (defaults_get_gravity (defaults), !=, GRAVITY_NONE);
+
+        // currently the default value should be NE (top-right) gravity
+        g_assert_cmpint (defaults_get_gravity (defaults), ==, GRAVITY_NORTH_EAST);
+
+        // check if we can pass "crap" to the call without causing a crash
+        g_assert_cmpint (defaults_get_gravity (NULL), ==, GRAVITY_NONE);
+
+        g_object_unref (G_OBJECT (defaults));
+}
+
 GTestSuite *
 test_defaults_create_test_suite (void)
 {
@@ -166,6 +183,7 @@
 	g_test_suite_add(ts, TC(test_defaults_get_desktop_right));
 	g_test_suite_add(ts, TC(test_defaults_get_stack_height));
 	g_test_suite_add(ts, TC(test_defaults_get_bubble_width));
+	g_test_suite_add(ts, TC(test_defaults_get_gravity));
 
 	return ts;
 }