Merge lp:~attente/indicator-keyboard/ibus-panel into lp:indicator-keyboard

[William Hua]

Implement the IBus panel within the indicator.

Implement the IBus panel within the indicator.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:267
http://jenkins.qa.ubuntu.com/job/indicator-keyboard-ci/45/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-keyboard-trusty-amd64-ci/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-keyboard-trusty-armhf-ci/12
        deb: http://jenkins.qa.ubuntu.com/job/indicator-keyboard-trusty-armhf-ci/12/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Testing building/running the new version, it seems to work fine. I would welcome testing from somebody who use IMs regularly though since I only use layouts.

I'm going to code review tomorrow (looks fine from a first glance) and try to land that before ff (if somebody else wants to help and do a code review that would be welcome though)

review: Approve (user testing)

[Ted Gould (ted)]

Sad to see that there isn't any tests and not a single comment.

Looking through the code I don't see any specific issues.

I'm curious about the UI thought, I would have expected these options to be radio items: http://ubuntuone.com/21XhYlv015IJKciNuU0DTN

[Ted Gould (ted)]

Oh, looks like I lost "Japanese (Anthy)" as a label as well.

[lp:~attente/indicator-keyboard/ibus-panel]

268. By William Hua on 2014-02-20

Break reference cycle which caused some inconsistent action handling.

=== modified file 'lib/Makefile.am'
--- lib/Makefile.am	2013-11-22 16:44:45 +0000
+++ lib/Makefile.am	2014-02-20 04:00:27 +0000
@@ -2,12 +2,14 @@
 
 AM_CFLAGS = -w -DGNOME_DESKTOP_USE_UNSTABLE_API
 AM_LDFLAGS = -lm
-AM_VALAFLAGS = --metadatadir $(top_srcdir)/deps \
+AM_VALAFLAGS = --enable-experimental-non-null   \
+               --metadatadir $(top_srcdir)/deps \
                --vapidir $(top_srcdir)/deps
 
 indicator_keyboard_service_SOURCES = main.vala          \
                                      source.vala        \
                                      common.vala        \
+                                     ibus-menu.vala     \
                                      window-stack.vala  \
                                      unity-greeter.vala
 indicator_keyboard_service_VALAFLAGS = $(AM_VALAFLAGS)            \

=== added file 'lib/ibus-menu.vala'
--- lib/ibus-menu.vala	1970-01-01 00:00:00 +0000
+++ lib/ibus-menu.vala	2014-02-20 04:00:27 +0000
@@ -0,0 +1,318 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * 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, version 3 of the License.
+ *
+ * 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/>.
+ *
+ * Authors: William Hua <[e-mail]>
+ */
+
+public class Indicator.Keyboard.IBusMenu : MenuModel {
+
+	private static uint radio_counter = 0;
+
+	private IBus.PropList? properties;
+
+	private Menu menu;
+	private SimpleActionGroup? action_group;
+
+	private string? radio_name;
+	private SimpleAction? radio_action;
+	private Gee.HashMap<string, IBus.Property> radio_properties;
+
+	// A list of the action names this menu registers
+	private Gee.LinkedList<string> names;
+
+	public IBusMenu (SimpleActionGroup? action_group = null, IBus.PropList? properties = null) {
+		menu = new Menu ();
+
+		menu.items_changed.connect ((position, removed, added) => {
+			items_changed (position, removed, added);
+		});
+
+		names = new Gee.LinkedList<string> ();
+		set_action_group (action_group);
+		set_properties (properties);
+	}
+
+	~IBusMenu () {
+		remove_actions ();
+	}
+
+	public signal void activate (IBus.Property property, IBus.PropState state);
+
+	private string get_action_name (string key) {
+		string name;
+
+		if (!action_name_is_valid (key)) {
+			var builder = new StringBuilder.sized (key.length + 1);
+
+			unichar letter = 0;
+			int index = 0;
+
+			while (key.get_next_char (ref index, out letter)) {
+				if (letter == '-' || letter == '.' || letter.isalnum ()) {
+					builder.append_unichar (letter);
+				} else {
+					builder.append_c ('-');
+				}
+			}
+
+			name = @"ibus-$(builder.str)";
+		} else {
+			name = @"ibus-$key";
+		}
+
+		// Find an unused action name using a counter
+		if (action_group != null && (Action?) ((!) action_group).lookup_action (name) != null) {
+			var i = 0;
+			var unique_name = @"$name-$i";
+
+			while ((Action?) ((!) action_group).lookup_action (unique_name) != null) {
+				i++;
+				unique_name = @"$name-$i";
+			}
+
+			name = unique_name;
+		}
+
+		return name;
+	}
+
+	private string? get_label (IBus.Property property) {
+		string? label = null;
+
+		if ((IBus.Text?) property.label != null) {
+			label = property.label.text;
+		}
+
+		if (label == null && (IBus.Text?) property.symbol != null) {
+			label = property.symbol.text;
+		}
+
+		return label;
+	}
+
+	private void append_normal_property (IBus.Property property) {
+		if (property.prop_type == IBus.PropType.NORMAL) {
+			if ((string?) property.key != null) {
+				var name = get_action_name (property.key);
+
+				if (action_group != null) {
+					var action = new SimpleAction (name, null);
+					action.activate.connect ((parameter) => { activate (property, property.state); });
+					((!) action_group).add_action (action);
+					names.add (name);
+				}
+
+				menu.append (get_label (property), property.sensitive ? @"indicator.$name" : "-private-disabled");
+			}
+		}
+	}
+
+	private void append_toggle_property (IBus.Property property) {
+		if (property.prop_type == IBus.PropType.TOGGLE) {
+			if ((string?) property.key != null) {
+				var name = get_action_name (property.key);
+
+				if (action_group != null) {
+					var state = new Variant.boolean (property.state == IBus.PropState.CHECKED);
+					var action = new SimpleAction.stateful (name, null, state);
+
+					action.activate.connect ((parameter) => {
+						action.change_state (new Variant.boolean (!action.get_state ().get_boolean ()));
+					});
+
+					action.change_state.connect ((value) => {
+						if (value != null) {
+							action.set_state ((!) value);
+							activate (property, ((!) value).get_boolean () ? IBus.PropState.CHECKED : IBus.PropState.UNCHECKED);
+						}
+					});
+
+					((!) action_group).add_action (action);
+					names.add (name);
+				}
+
+				menu.append (get_label (property), property.sensitive ? @"indicator.$name" : "-private-disabled");
+			}
+		}
+	}
+
+	private void append_radio_property (IBus.Property property) {
+		if (property.prop_type == IBus.PropType.RADIO) {
+			if ((string?) property.key != null) {
+				// Create a single action for all radio properties.
+				if (action_group != null && radio_name == null) {
+					radio_counter++;
+					radio_name = @"-private-radio-$radio_counter";
+					radio_action = new SimpleAction.stateful ((!) radio_name, VariantType.STRING, new Variant.string (""));
+
+					((!) radio_action).activate.connect ((parameter) => {
+						((!) radio_action).change_state (parameter);
+					});
+
+					((!) radio_action).change_state.connect ((value) => {
+						if (value != null) {
+							var key = ((!) value).get_string ();
+
+							if (radio_properties.has_key (key)) {
+								((!) radio_action).set_state ((!) value);
+								activate (radio_properties[key], IBus.PropState.CHECKED);
+							}
+						}
+					});
+
+					((!) action_group).add_action ((!) radio_action);
+					names.add ((!) radio_name);
+				}
+
+				radio_properties[property.key] = property;
+
+				if (property.state == IBus.PropState.CHECKED) {
+					((!) radio_action).change_state (new Variant.string (property.key));
+				}
+
+				var item = new MenuItem (get_label (property), "-private-disabled");
+
+				if (property.sensitive) {
+					item.set_action_and_target_value (@"indicator.$((!) radio_name)", new Variant.string (property.key));
+				}
+
+				menu.append_item (item);
+			}
+		}
+	}
+
+	private void append_menu_property (IBus.Property property) {
+		if (property.prop_type == IBus.PropType.MENU) {
+			var submenu = new IBusMenu (action_group, ((!) property).sub_props);
+			submenu.activate.connect ((property, state) => { activate (property, state); });
+			menu.append_submenu (get_label (property), submenu);
+		}
+	}
+
+	private void append_property (IBus.Property? property) {
+		if (property != null && ((!) property).visible) {
+			switch (((!) property).prop_type) {
+			case IBus.PropType.NORMAL:
+				append_normal_property ((!) property);
+				break;
+
+			case IBus.PropType.TOGGLE:
+				append_toggle_property ((!) property);
+				break;
+
+			case IBus.PropType.RADIO:
+				append_radio_property ((!) property);
+				break;
+
+			case IBus.PropType.MENU:
+				append_menu_property ((!) property);
+				break;
+
+			case IBus.PropType.SEPARATOR:
+				break;
+			}
+		}
+	}
+
+	private void update_menu () {
+		// There's a reference cycle between the action group and the submenus.
+		// We need to break it here so that those submenus aren't hanging around.
+		for (var i = 0; i < menu.get_n_items (); i++) {
+			var submenu = menu.get_item_link (i, Menu.LINK_SUBMENU) as IBusMenu;
+
+			if (submenu != null) {
+				((!) submenu).remove_actions ();
+			}
+		}
+
+		menu.remove_all ();
+
+		if (properties != null) {
+			for (var i = 0; i < ((!) properties).properties.length; i++) {
+				append_property (((!) properties).get (i));
+			}
+		}
+	}
+
+	private void remove_actions () {
+		radio_action = null;
+		radio_name = null;
+
+		if (action_group != null) {
+			foreach (var name in names) {
+				((!) action_group).remove_action (name);
+			}
+		}
+
+		names.clear ();
+	}
+
+	public void set_action_group (SimpleActionGroup? action_group) {
+		if (action_group != this.action_group) {
+			remove_actions ();
+			this.action_group = action_group;
+			update_menu ();
+		}
+	}
+
+	public void set_properties (IBus.PropList? properties) {
+		if (properties != this.properties) {
+			remove_actions ();
+			radio_properties = new Gee.HashMap<string, IBus.Property> ();
+			this.properties = properties;
+			update_menu ();
+		}
+	}
+
+	public void update_property (IBus.Property property) {
+		remove_actions ();
+		radio_properties = new Gee.HashMap<string, IBus.Property> ();
+		update_menu ();
+	}
+
+	// Forward all menu model calls to our internal menu
+
+	public override Variant get_item_attribute_value (int item_index, string attribute, VariantType? expected_type) {
+		return menu.get_item_attribute_value (item_index, attribute, expected_type);
+	}
+
+	public override void get_item_attributes (int item_index, out HashTable<string, Variant>? attributes) {
+		menu.get_item_attributes (item_index, out attributes);
+	}
+
+	public override MenuModel get_item_link (int item_index, string link) {
+		return menu.get_item_link (item_index, link);
+	}
+
+	public override void get_item_links (int item_index, out HashTable<string, MenuModel>? links) {
+		menu.get_item_links (item_index, out links);
+	}
+
+	public override int get_n_items () {
+		return menu.get_n_items ();
+	}
+
+	public override bool is_mutable () {
+		return menu.is_mutable ();
+	}
+
+	public override MenuAttributeIter iterate_item_attributes (int item_index) {
+		return menu.iterate_item_attributes (item_index);
+	}
+
+	public override MenuLinkIter iterate_item_links (int item_index) {
+		return menu.iterate_item_links (item_index);
+	}
+}

=== modified file 'lib/main.vala'
--- lib/main.vala	2014-02-19 15:09:16 +0000
+++ lib/main.vala	2014-02-20 04:00:27 +0000
@@ -19,8 +19,9 @@
 [DBus (name = "com.canonical.indicator.keyboard")]
 public class Indicator.Keyboard.Service : Object {
 
+	private static const uint PROPERTIES_DELAY = 250;
+
 	private static Service service;
-	private static IBus.Bus? ibus;
 
 	private bool force;
 	private bool use_gtk;
@@ -35,12 +36,17 @@
 	private Gee.HashMap<uint, uint>? window_sources;
 	private uint focused_window_id;
 
+	private IBus.Bus? ibus;
+	private IBus.PanelService? panel_service;
+	private uint panel_timeout;
+
 	private Source[]? sources;
 
 	private SimpleActionGroup? action_group;
 	private SimpleAction? indicator_action;
 	private MenuModel? menu_model;
 	private Menu? sources_menu;
+	private IBusMenu? ibus_menu;
 
 	private UnityGreeter? unity_greeter;
 	private string? greeter_user;
@@ -96,7 +102,7 @@
 	}
 
 	[DBus (visible = false)]
-	private static IBus.Bus get_ibus () {
+	private IBus.Bus get_ibus () {
 		if (ibus == null) {
 			IBus.init ();
 			ibus = new IBus.Bus ();
@@ -210,7 +216,7 @@
 		if (source != null) {
 			var array = source_settings.get_value ("sources");
 
-			for (int i = 0; i < array.n_children (); i++) {
+			for (var i = 0; i < array.n_children (); i++) {
 				unowned string type;
 				unowned string name;
 
@@ -543,6 +549,14 @@
 						break;
 					}
 				}
+
+				if (panel_service == null && sources[i].is_ibus) {
+					if (get_ibus ().request_name (IBus.SERVICE_PANEL, IBus.BusNameFlag.REPLACE_EXISTING) > 0) {
+						panel_service = new IBus.PanelService (get_ibus ().get_connection ());
+						((!) panel_service).register_properties.connect (handle_registered_properties);
+						((!) panel_service).update_property.connect (handle_updated_property);
+					}
+				}
 			}
 		}
 
@@ -550,6 +564,25 @@
 	}
 
 	[DBus (visible = false)]
+	private void handle_registered_properties (IBus.PropList list) {
+		if (panel_timeout > 0) {
+			GLib.Source.remove (panel_timeout);
+			panel_timeout = 0;
+		}
+
+		panel_timeout = Timeout.add (PROPERTIES_DELAY, () => {
+			update_ibus_menu (list);
+			panel_timeout = 0;
+			return false;
+		});
+	}
+
+	[DBus (visible = false)]
+	private void handle_updated_property (IBus.Property property) {
+		get_ibus_menu ().update_property (property);
+	}
+
+	[DBus (visible = false)]
 	private void update_indicator_action () {
 		var visible = indicator_settings.get_boolean ("visible");
 		var current = source_settings.get_uint ("current");
@@ -600,7 +633,7 @@
 			var length = (int) sources.n_children ();
 
 			if (length > 0) {
-				var offset = parameter.get_int32 () % length;
+				var offset = ((!) parameter).get_int32 () % length;
 				source_settings.set_uint ("current", (current + (length - offset)) % length);
 			}
 		}
@@ -610,28 +643,28 @@
 	protected virtual SimpleActionGroup create_action_group (Action root_action) {
 		var group = new SimpleActionGroup ();
 
-		group.insert (root_action);
-		group.insert (source_settings.create_action ("current"));
+		group.add_action (root_action);
+		group.add_action (source_settings.create_action ("current"));
 
 		var action = new SimpleAction ("next", null);
 		action.activate.connect (handle_middle_click);
-		group.insert (action);
+		group.add_action (action);
 
 		action = new SimpleAction ("scroll", VariantType.INT32);
 		action.activate.connect (handle_scroll_wheel);
-		group.insert (action);
+		group.add_action (action);
 
 		action = new SimpleAction ("map", null);
 		action.activate.connect (handle_activate_map);
-		group.insert (action);
+		group.add_action (action);
 
 		action = new SimpleAction ("chart", null);
 		action.activate.connect (handle_activate_chart);
-		group.insert (action);
+		group.add_action (action);
 
 		action = new SimpleAction ("settings", null);
 		action.activate.connect (handle_activate_settings);
-		group.insert (action);
+		group.add_action (action);
 
 		return group;
 	}
@@ -649,12 +682,9 @@
 	private void update_sources_menu () {
 		if (sources_menu != null) {
 			var menu = get_sources_menu ();
-
-			while (menu.get_n_items () > 0)
-				menu.remove (0);
+			menu.remove_all ();
 
 			var sources = get_sources ();
-
 			for (var i = 0; i < sources.length; i++) {
 				var item = new MenuItem (sources[i].name, "indicator.current");
 				item.set_attribute (Menu.ATTRIBUTE_TARGET, "u", i);
@@ -682,12 +712,32 @@
 	}
 
 	[DBus (visible = false)]
-	protected virtual MenuModel create_menu_model (MenuModel section_menu) {
+	private void update_ibus_menu (IBus.PropList list) {
+		get_ibus_menu ().set_properties (list);
+	}
+
+	[DBus (visible = false)]
+	private IBusMenu get_ibus_menu () {
+		if (ibus_menu == null) {
+			ibus_menu = new IBusMenu (get_action_group ());
+			((!) ibus_menu).activate.connect ((property, state) => {
+				if (panel_service != null) {
+					((!) panel_service).property_activate (property.key, state);
+				}
+			});
+		}
+
+		return (!) ibus_menu;
+	}
+
+	[DBus (visible = false)]
+	protected virtual MenuModel create_menu_model (MenuModel sources_menu, MenuModel ibus_menu) {
 		var menu = new Menu ();
 
 		var submenu = new Menu ();
 
-		submenu.append_section (null, section_menu);
+		submenu.append_section (null, sources_menu);
+		submenu.append_section (null, ibus_menu);
 
 		if (!is_login_user ()) {
 			var section = new Menu ();
@@ -710,7 +760,7 @@
 	[DBus (visible = false)]
 	public MenuModel get_menu_model () {
 		if (menu_model == null) {
-			menu_model = create_menu_model (get_sources_menu ());
+			menu_model = create_menu_model (get_sources_menu (), get_ibus_menu ());
 		}
 
 		return (!) menu_model;

=== modified file 'lib/source.vala'
--- lib/source.vala	2013-08-07 19:11:32 +0000
+++ lib/source.vala	2014-02-20 04:00:27 +0000
@@ -69,6 +69,14 @@
 		construct set { _use_gtk = value; icon = null; }
 	}
 
+	public bool is_xkb {
+		get { return xkb != null; }
+	}
+
+	public bool is_ibus {
+		get { return ibus != null; }
+	}
+
 	public Source (Variant variant, bool use_gtk = false) {
 		Object (use_gtk: use_gtk);