Merge lp:~gandalfn/gala/add-dbus-accelerator-and-input-sources into lp:gala

[gandalfn]

this a merge of https://code.launchpad.net/~gandalfn/gala/add-dbus-accelerator-interface and lp:~gandalfn/gala/add-input-sources-management branches

this branch provides:

- Add check for gnome-settings-daemon >= 3.14, this needs a new dependency under gnome-settings-daemon-dev for deb packaging

- Add multimedia key grab management under gala:

Multimedia keys has been managed by gnome-settings-deamon, which forward
accelerators trought org.gnome.shell grab/ungrab interface.

Add fake dbus accelerator interface which add registered multimedia keys
in gnome-settings-deamon to gala.

See in pantheon debian port

This patch is perhaps debian specific and perhaps needs a define to enable
or not on which distribution gala is been compiled

- Add keyboard layout management in gala:

Under debian since gnome 3.14 keyboard layout management has been managed by mutter
and gnome-shell instead of gnome-settings-daemon.

This patch provide the management of keyboard layout configuration in
gala, to allow the keyboard layout configuration under pantheon desktop.
But this patch still incomplete since it is not manage ibus input sources.

[lp:~gandalfn/gala/add-dbus-accelerator-and-input-sources]

489. By gandalfn on 2015-10-18

Enable keyboard switch with manager only if we have mutter >= 3.14 and
gsd >= 3.14

[Rico Tzschichholz (ricotz)]

JFYI, putting these merges together won't make the review easier.

* Please make sure to follow gala's codestyle.
* Watch out for nested conditionals. e.g. "HAS_MUTTER_310" inside of "HAS_GSD314"
* Did you intent to ignored support for GSD 3.10/3.12?
* Do not make things public if not needed
* Use a more lightweight structure for KeyboardLayout*?
* Don't ignore existing code which could be replaced or incorporated with these changes.

There are a bunch of things to think about and I might make some adjustments myself.

E.g. Can this really be null? "unowned KeyboardManager? manager = KeyboardManager.get_instance ();

review: Needs Fixing

[gandalfn (gandalfn)]

Sorry for merging, it was easier for me and debian packaging.

for unowned has nullable it was a bad habit I have, for me an unowned always can be nullable since it does not transfer its ownership, not really accurate in fact

I'll watch all you remarks, but I think at term pantheon-desktop needs a gnome-settings-daemon/gnome-session programs which manage this kind of stuff

[Rico Tzschichholz (ricotz)]

> for unowned has nullable it was a bad habit I have, for me an unowned always
> can be nullable since it does not transfer its ownership, not really accurate
> in fact

Let me rephrase that to be more clear "Can KeyboardManager.get_instance() really be null?"

[gandalfn (gandalfn)]

> > for unowned has nullable it was a bad habit I have, for me an unowned always
> > can be nullable since it does not transfer its ownership, not really
> accurate
> > in fact
>
> Let me rephrase that to be more clear "Can KeyboardManager.get_instance()
> really be null?"

No it can not be null :)

[Corentin Noël (tintou)]

I added a few comments to your merge request

Unmerged revisions

489. By gandalfn on 2015-10-18

Enable keyboard switch with manager only if we have mutter >= 3.14 and
gsd >= 3.14

488. By gandalfn on 2015-10-18

Enable keyboard manager only if we have mutter >= 3.14 and gsd >= 3.14

487. By gandalfn on 2015-10-18

Since gnome 3.14 keyboard layout management has been managed by mutter
and gnome-shell instead of gnome-settings-daemon.

This patch provide the management of keyboard layout configuration in
gala, to allow the keyboard layout configuration under pantheon desktop.
But this patch still incomplete since it is not manage ibus input sources.

486. By gandalfn on 2015-10-18

Multimedia keys has been managed by gnome-settings-deamon, which forward
accelerators trought org.gnome.shell grab/ungrab interface.
Add fake dbus accelerator interface which add registered multimedia keys
in gnome-settings-deamon to gala.

485. By gandalfn on 2015-10-18

Add check for gnome-settings-daemon >= 3.14

=== modified file 'configure.ac'
--- configure.ac	2015-09-23 14:52:12 +0000
+++ configure.ac	2015-10-18 11:52:40 +0000
@@ -181,6 +181,13 @@
     MUTTER_API="3.18"
 fi
 
+# 3.14
+PKG_CHECK_MODULES(GSD314, [gnome-settings-daemon >= 3.14], [have_gsd314=yes], [have_gsd314=no])
+if test "x$have_gsd314" = "xyes" ; then
+    VALAFLAGS="$VALAFLAGS --define HAS_GSD314"
+fi
+AM_CONDITIONAL([HAVE_GSD314], [test "x$have_gsd314" = "xyes"])
+
 # -----------------------------------------------------------
 # Dependencies for Notifications plugin
 # -----------------------------------------------------------

=== added file 'src/DBusAccelerator.vala'
--- src/DBusAccelerator.vala	1970-01-01 00:00:00 +0000
+++ src/DBusAccelerator.vala	2015-10-18 11:52:40 +0000
@@ -0,0 +1,116 @@
+//
+//  This program is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  This program is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+namespace Gala
+{
+	[DBus (name="org.gnome.Shell")]
+	public class DBusAccelerator
+	{
+		static DBusAccelerator? instance;
+		static WindowManager wm;
+
+		HashTable<string, uint?> grabbed_accelerators;
+
+		public struct Accelerator
+		{
+			public string name;
+			public uint   flags;
+		}
+
+		[DBus (visibile = false)]
+		public static void init (WindowManager _wm)
+		{
+			wm = _wm;
+
+			Bus.own_name (BusType.SESSION, "org.gnome.Shell", BusNameOwnerFlags.NONE,
+				(connection) => {
+					if (instance == null)
+						instance = new DBusAccelerator ();
+
+					try {
+						connection.register_object ("/org/gnome/Shell", instance);
+					} catch (Error e) { warning (e.message); }
+				},
+				() => {},
+				() => warning ("Could not acquire name\n") );
+		}
+
+		public signal void AcceleratorActivated (uint action, uint device_id, uint timestamp);
+
+		private DBusAccelerator ()
+		{
+			grabbed_accelerators = new HashTable<string, uint> (str_hash, str_equal);
+
+			wm.get_screen ().get_display ().accelerator_activated.connect (on_accelerator_activated);
+		}
+
+#if HAS_MUTTER310
+		private void on_accelerator_activated (uint action, uint device_id, uint timestamp)
+#else
+		private void on_accelerator_activated (uint action, uint device_id)
+#endif
+		{
+			foreach (string accelerator in grabbed_accelerators.get_keys ()) {
+				if (grabbed_accelerators[accelerator] == action) {
+#if HAS_MUTTER310
+					AcceleratorActivated (action, device_id, timestamp);
+#else
+					AcceleratorActivated (action, device_id, 0);
+#endif
+				}
+			}
+		}
+
+		public uint GrabAccelerator (string accelerator, uint flags)
+		{
+			uint? action = grabbed_accelerators[accelerator];
+
+			if (action == null) {
+				action = wm.get_screen ().get_display ().grab_accelerator (accelerator);
+				if (action != 0) {
+					grabbed_accelerators[accelerator] = action;
+				}
+			}
+
+			return action;
+		}
+
+		public uint[] GrabAccelerators (Accelerator[] accelerators)
+		{
+			uint[] actions = {};
+
+			foreach (unowned Accelerator? accelerator in accelerators) {
+				actions += GrabAccelerator (accelerator.name, accelerator.flags);
+			}
+
+			return actions;
+		}
+
+		public bool UngrabAccelerator (uint action)
+		{
+			bool ret = false;
+
+			foreach (string accelerator in grabbed_accelerators.get_keys ()) {
+				if (grabbed_accelerators[accelerator] == action) {
+					ret = wm.get_screen ().get_display ().ungrab_accelerator (action);
+					grabbed_accelerators.remove (accelerator);
+					break;
+				}
+			}
+
+			return ret;
+		}
+	}
+}

=== added directory 'src/Keyboard'
=== added file 'src/Keyboard/KeyboardManager.vala'
--- src/Keyboard/KeyboardManager.vala	1970-01-01 00:00:00 +0000
+++ src/Keyboard/KeyboardManager.vala	2015-10-18 11:52:40 +0000
@@ -0,0 +1,46 @@
+//
+//  This program is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  This program is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+namespace Gala
+{
+	public class KeyboardManager : Object
+	{
+		public KeyboardLayoutSettings settings { get; private set; }
+
+		// singleton pattern
+		static KeyboardManager? instance;
+		public static unowned KeyboardManager get_instance () {
+			if (instance == null) {
+				instance = new KeyboardManager ();
+			}
+			return instance;
+		}
+
+		private void update_keymap () {
+			unowned KeyboardLayoutXKB? layout = settings.layouts[settings.layouts.active] as KeyboardLayoutXKB;
+			if (layout != null)
+			{
+				Meta.Backend.get_backend ().set_keymap (layout.layout, layout.variant ?? "", settings.xkb_options);
+			}
+		}
+
+		private KeyboardManager () {
+			settings = new KeyboardLayoutSettings ();
+			update_keymap ();
+
+			settings.layouts.notify["active"].connect (update_keymap);
+		}
+	}
+}

=== added file 'src/Keyboard/KeyboardSettings.vala'
--- src/Keyboard/KeyboardSettings.vala	1970-01-01 00:00:00 +0000
+++ src/Keyboard/KeyboardSettings.vala	2015-10-18 11:52:40 +0000
@@ -0,0 +1,284 @@
+//
+//  This program is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  This program is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+//
+
+namespace Gala
+{
+	public abstract class KeyboardLayout : Object {
+		public string name { get; set; }
+
+		public KeyboardLayout (string name) {
+			Object (name: name);
+		}
+
+		public virtual bool equal (KeyboardLayout other) {
+			return this.name == other.name;
+		}
+
+		public static KeyboardLayout from_variant (GLib.Variant variant) {
+			KeyboardLayout? ret = null;
+			if (variant.is_of_type (new VariantType ("(ss)"))) {
+				unowned string type;
+				unowned string name;
+
+				variant.get ("(&s&s)", out type, out name);
+
+				if (type == "xkb") {
+					ret = new KeyboardLayoutXKB (name);
+				} else if (type == "ibus") {
+					ret = new KeyboardLayoutIBUS (name);
+				} else {
+					warning ("Unkown type %s", type);
+				}
+
+			} else {
+				warning ("Variant has invalid type");
+			}
+
+			return ret;
+		}
+	}
+
+	public class KeyboardLayoutIBUS : KeyboardLayout {
+		public KeyboardLayoutIBUS (string name) {
+			base (name);
+		}
+
+		public override bool equal (KeyboardLayout other) {
+			return (other is KeyboardLayoutIBUS) && base.equal (other);
+		}
+	}
+
+	public class KeyboardLayoutXKB : KeyboardLayout {
+		public string? layout {
+			owned get {
+				string[]? split = name.split("+", 2);
+				if (split != null)
+					return split[0];
+				return null;
+			}
+		}
+
+		public string? variant {
+			owned get {
+				string[]? split = name.split("+", 2);
+				if (split != null && split.length >= 2)
+					return split[1];
+				return null;
+			}
+		}
+
+		public KeyboardLayoutXKB (string layout, string? variant = null) {
+			string name = layout;
+			if (variant != null && variant != "")
+				name += "+" + variant;
+
+			base (name);
+		}
+
+		public override bool equal (KeyboardLayout other) {
+			return (other is KeyboardLayoutXKB) && base.equal (other);
+		}
+	}
+
+	public class KeyboardLayoutList : Object {
+		GLib.List<KeyboardLayout> layouts = new GLib.List<KeyboardLayout> ();
+
+		public uint length {
+			get {
+				return layouts.length ();
+			}
+		}
+
+		uint _active;
+		public uint active {
+			get {
+				return _active;
+			}
+			set {
+				if (length == 0)
+					return;
+
+				if (_active == value)
+					return;
+
+				_active = value;
+				if (_active >= length)
+					_active = length - 1;
+			}
+
+		}
+
+		public bool contains (KeyboardLayout given_layout) {
+			return get_layout_index (given_layout) != -1;
+		}
+
+		public int get_layout_index (KeyboardLayout given_layout) {
+			int i = 0;
+			foreach (KeyboardLayout l in layouts) {
+				if (l.equal (given_layout))
+					return i;
+				i++;
+			}
+			return -1;
+		}
+
+		public new unowned KeyboardLayout? @get (uint index) {
+			if (index >= length)
+				return null;
+
+			return layouts.nth_data (index);
+		}
+
+		public void add (KeyboardLayout layout) {
+			if (!(layout in this))
+				layouts.append (layout);
+		}
+
+		public void clear () {
+			layouts = new GLib.List<KeyboardLayout> ();
+		}
+	}
+
+	public class KeyboardLayoutSettings : Object {
+		GLib.Settings settings;
+
+		public KeyboardLayoutList layouts { get; private set; }
+		public bool per_window { get; private set; }
+		public string xkb_options {
+			owned get {
+				string ret = "";
+				GLib.Variant sources = settings.get_value ("sources");
+				if (sources.is_of_type (VariantType.ARRAY)) {
+					for (size_t i = 0; i < sources.n_children (); i++) {
+						GLib.Variant child = sources.get_child_value (i);
+						if (child.is_of_type (new VariantType ("s"))) {
+							unowned string opt;
+							child.get ("&s", out opt);
+							if (ret == "")
+								ret += opt;
+							else
+								ret += ","+opt;
+						}
+					}
+				}
+				return ret;
+			}
+		}
+
+		void update_list_from_gsettings () {
+			layouts.clear ();
+			GLib.Variant sources = settings.get_value ("sources");
+			if (sources.is_of_type (VariantType.ARRAY)) {
+				for (size_t i = 0; i < sources.n_children (); i++) {
+					GLib.Variant child = sources.get_child_value (i);
+					KeyboardLayout? layout = KeyboardLayout.from_variant (child);
+					if (layout != null)
+						layouts.add (layout);
+				}
+			} else {
+				warning ("Unkown type");
+			}
+		}
+
+		void update_active_from_gsettings () {
+			layouts.active = settings.get_uint ("current");
+		}
+
+		void parse_default () {
+			var file = File.new_for_path ("/etc/default/keyboard");
+
+			if (!file.query_exists ()) {
+				warning ("File '%s' doesn't exist.\n", file.get_path ());
+				return;
+			}
+
+			string xkb_layout  = "";
+			string xkb_variant = "";
+
+			try {
+				var dis = new DataInputStream (file.read ());
+
+				string line;
+
+				while ((line = dis.read_line (null)) != null)
+				{
+					if (line.contains ("XKBLAYOUT="))
+					{
+						xkb_layout = line.replace ("XKBLAYOUT=", "").replace ("\"", "");
+
+						while ((line = dis.read_line (null)) != null) {
+							if (line.contains ("XKBVARIANT=")) {
+								xkb_variant = line.replace ("XKBVARIANT=", "").replace ("\"", "");
+							}
+						}
+
+						break;
+					}
+				}
+			}
+			catch (Error e) {
+				warning ("%s", e.message);
+				return;
+			}
+
+			var variants = xkb_variant.split (",");
+			var xkb_layouts  = xkb_layout.split (",");
+
+			for (int i = 0; i < layouts.length; i++) {
+				if (variants[i] != null && variants[i] != "")
+					layouts.add (new KeyboardLayoutXKB (xkb_layouts[i] + "+" + variants[i]));
+				else
+					layouts.add (new KeyboardLayoutXKB (xkb_layouts[i]));
+			}
+
+			layouts.active = 0;
+		}
+
+		public KeyboardLayoutSettings ()
+		{
+			settings = new Settings ("org.gnome.desktop.input-sources");
+			layouts = new KeyboardLayoutList ();
+
+			per_window = settings.get_boolean ("per-window");
+			settings.changed["per-window"].connect (() => {
+				per_window = settings.get_boolean ("per-window");
+			});
+
+			update_list_from_gsettings ();
+			update_active_from_gsettings ();
+
+			settings.changed["sources"].connect (() => {
+				update_list_from_gsettings ();
+				update_active_from_gsettings ();
+			});
+
+			settings.changed["xkb-options"].connect (() => {
+				update_active_from_gsettings ();
+			});
+
+			settings.changed["current"].connect (update_active_from_gsettings);
+
+			layouts.notify["active"].connect (() => {
+				settings.changed["current"].disconnect (update_active_from_gsettings);
+				settings.set_uint ("current", layouts.active);
+				settings.changed["current"].connect (update_active_from_gsettings);
+			});
+
+			if (layouts.length == 0)
+				parse_default ();
+		}
+	}
+
+}

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2015-03-04 11:29:53 +0000
+++ src/Makefile.am	2015-10-18 11:52:40 +0000
@@ -69,6 +69,11 @@
 	Background/SystemBackground.vala \
 	$(NULL)
 
+gala_keyboard = \
+	Keyboard/KeyboardSettings.vala \
+	Keyboard/KeyboardManager.vala \
+	$(NULL)
+
 gala_background312 = \
 	Background-3.12/Background.vala \
 	Background-3.12/BackgroundCache.vala \
@@ -79,10 +84,20 @@
 
 if HAVE_MUTTER314
 gala_VALASOURCES += $(gala_background)
+if HAVE_GSD314
+gala_VALASOURCES += $(gala_keyboard)
+endif
 else
 gala_VALASOURCES += $(gala_background312)
 endif
 
+gala_accelerator = \
+	DBusAccelerator.vala
+
+if HAVE_GSD314
+gala_VALASOURCES += $(gala_accelerator)
+endif
+
 nodist_gala_SOURCES = \
 	gala_vala.stamp \
 	$(gala_VALASOURCES:.vala=.c) \
@@ -104,6 +119,8 @@
 EXTRA_DIST = \
 	$(gala_background) \
 	$(gala_background312) \
+	$(gala_accelerator) \
+	$(gala_keyboard) \
 	$(gala_VALASOURCES) \
 	$(NULL)
 

=== modified file 'src/WindowManager.vala'
--- src/WindowManager.vala	2015-09-10 11:32:41 +0000
+++ src/WindowManager.vala	2015-10-18 11:52:40 +0000
@@ -93,11 +93,18 @@
 			var display = screen.get_display ();
 
 			DBus.init (this);
+#if HAS_GSD314
+			DBusAccelerator.init (this);
+#endif
 #if !HAS_MUTTER314
 			BackgroundCache.init (screen);
 #endif
 			WindowListener.init (screen);
 
+#if HAS_MUTTER314 && HAS_GSD314
+			KeyboardManager.get_instance ();
+#endif
+
 			// Due to a bug which enables access to the stage when using multiple monitors
 			// in the screensaver, we have to listen for changes and make sure the input area
 			// is set to NONE when we are in locked mode
@@ -325,6 +332,24 @@
 			X.Event event, Meta.KeyBinding binding)
 #endif
 		{
+#if HAS_MUTTER314 && HAS_GSD314
+			unowned KeyboardManager? manager = KeyboardManager.get_instance ();
+			if (manager != null) {
+				var n_sources = manager.settings.layouts.length;
+				if (n_sources < 2)
+					return;
+
+				var new_index = 0U;
+				var current_index = manager.settings.layouts.active;
+
+				if (binding.get_name () == "switch-input-source")
+					new_index = (current_index + 1) % n_sources;
+				else
+					new_index = (current_index - 1 + n_sources) % n_sources;
+
+				manager.settings.layouts.active = new_index;
+			}
+#else
 			var keyboard_input_settings = new GLib.Settings ("org.gnome.desktop.input-sources");
 
 			var n_sources = (uint) keyboard_input_settings.get_value ("sources").n_children ();
@@ -340,6 +365,7 @@
 				new_index = (current_index - 1 + n_sources) % n_sources;
 
 			keyboard_input_settings.set_uint ("current", new_index);
+#endif
 		}
 
 		[CCode (instance_pos = -1)]
@@ -746,7 +772,7 @@
 		{
 			unowned AnimationSettings animation_settings = AnimationSettings.get_default ();
 			var duration = animation_settings.minimize_duration;
-			
+
 			if (!animation_settings.enable_animations
 				|| duration == 0
 				|| actor.get_meta_window ().window_type != WindowType.NORMAL) {