[gnome-control-center] keyboard: Refactoring of CcKeyboardItem handling of multiple combos

[Robert Ancell <rancell src gnome org>]

commit 67cb50880288c72f79c2367666d5a138f1f9dee1
Author: Ian Douglas Scott <idscott system76 com>
Date:   Mon Jul 6 10:22:26 2020 -0700

    keyboard: Refactoring of CcKeyboardItem handling of multiple combos
    
    This commit includes all the changes that seem to be necessary for
    CcKeyboardItem to be used for dealing with multiple keybindings, without
    (yet) changing the user interface to expose this.
    
    The `primary_combo` and `binding` fields of `CcKeyboardItem` are
    removed, in favor of the existing `key_combos`. No combination is
    "primary", since all of them can now be seen and changed equally.
    
    We treat `CcKeyboardItem.key_combos` as a set, that a combo can be added
    to or removed from. Though it continues to be represented as a `GList`,
    instead of a `GHashTable`, to preserve ordering.
    
    A lot of the keyboard panel code relied on the assumption that only one
    combo can be set for each setting, so this required a variety of
    miscellaneous changes.

 panels/keyboard/cc-keyboard-item.c            | 414 +++++++++++++-------------
 panels/keyboard/cc-keyboard-item.h            |  11 +-
 panels/keyboard/cc-keyboard-manager.c         |  18 +-
 panels/keyboard/cc-keyboard-manager.h         |   3 -
 panels/keyboard/cc-keyboard-panel.c           |  18 +-
 panels/keyboard/cc-keyboard-shortcut-editor.c |  34 +--
 panels/keyboard/keyboard-shortcuts.c          |   8 +-
 panels/keyboard/keyboard-shortcuts.h          |   8 +-
 8 files changed, 246 insertions(+), 268 deletions(-)
---
diff --git a/panels/keyboard/cc-keyboard-item.c b/panels/keyboard/cc-keyboard-item.c
index 853c5986b..2f7cdf753 100644
--- a/panels/keyboard/cc-keyboard-item.c
+++ b/panels/keyboard/cc-keyboard-item.c
@@ -34,20 +34,18 @@ struct _CcKeyboardItem
 {
   GObject parent_instance;
 
-  char *binding;
-
   CcKeyboardItem *reverse_item;
   gboolean is_reversed;
   gboolean hidden;
 
   CcKeyboardItemType type;
 
-  CcKeyCombo *primary_combo;
   BindingGroupType group;
   char *description;
   gboolean editable;
   GList *key_combos;
   GList *default_combos;
+  gboolean can_set_multiple;
 
   /* GSettings path */
   char *gsettings_path;
@@ -65,11 +63,11 @@ enum
 {
   PROP_0,
   PROP_DESCRIPTION,
-  PROP_BINDING,
   PROP_EDITABLE,
   PROP_TYPE,
   PROP_IS_VALUE_DEFAULT,
-  PROP_COMMAND
+  PROP_COMMAND,
+  PROP_KEY_COMBOS
 };
 
 static void     cc_keyboard_item_class_init     (CcKeyboardItemClass *klass);
@@ -78,21 +76,46 @@ static void     cc_keyboard_item_finalize       (GObject               *object);
 
 G_DEFINE_TYPE (CcKeyboardItem, cc_keyboard_item, G_TYPE_OBJECT)
 
-static const gchar *
-get_binding_from_variant (GVariant *variant)
+static const CcKeyCombo EMPTY_COMBO = { 0, 0, 0 };
+
+static gboolean
+combo_equal (CcKeyCombo *a, CcKeyCombo *b)
 {
-  const char *str, **strv;
+  return (a->keyval == b->keyval
+       && a->keycode == b->keycode
+       && a->mask == b->mask);
+}
 
-  if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING))
-    return g_variant_get_string (variant, NULL);
-  else if (!g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY))
-    return "";
+static gboolean
+combos_contains (GList *combos, CcKeyCombo *needle)
+{
+  for (GList *l = combos; l != NULL; l = l->next)
+    {
+      if (combo_equal (l->data, needle))
+        return TRUE;
+    }
+
+  return FALSE;
+}
 
-  strv = g_variant_get_strv (variant, NULL);
-  str = strv[0];
-  g_free (strv);
+static gboolean
+combos_equal (GList *a, GList *b)
+{
+  // Should be efficient enough for any sane number of bindings
+
+  for (GList *l = a; l != NULL; l = l->next)
+    {
+      if (!combos_contains (b, l->data))
+        return FALSE;
+    }
 
-  return str;
+  for (GList *l = b; l != NULL; l = l->next)
+    {
+      if (!combos_contains (a, l->data))
+        return FALSE;
+    }
+
+    return TRUE;
 }
 
 static gboolean
@@ -100,7 +123,7 @@ binding_from_string (const char *str,
                      CcKeyCombo *combo)
 {
   g_return_val_if_fail (combo != NULL, FALSE);
-  guint *keycodes;
+  g_autofree guint *keycodes = NULL;
 
   if (str == NULL || strcmp (str, "disabled") == 0)
     {
@@ -111,7 +134,6 @@ binding_from_string (const char *str,
   gtk_accelerator_parse_with_keycode (str, &combo->keyval, &keycodes, &combo->mask);
 
   combo->keycode = (keycodes ? keycodes[0] : 0);
-  g_free (keycodes);
 
   if (combo->keyval == 0)
     return FALSE;
@@ -143,89 +165,6 @@ cc_keyboard_item_get_desc_editable (CcKeyboardItem *item)
   return item->desc_editable;
 }
 
-/* wrapper around g_settings_set_str[ing|v] */
-static void
-settings_set_binding (GSettings  *settings,
-                      const char *key,
-                     const char *value)
-{
-  GVariant *variant;
-
-  variant = g_settings_get_value (settings, key);
-
-  if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING))
-    g_settings_set_string (settings, key, value ? value : "");
-  else if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY))
-    {
-      if (value == NULL || *value == '\0')
-        g_settings_set_strv (settings, key, NULL);
-      else
-        {
-          char **str_array = g_new0 (char *, 2);
-
-          /* clear any additional bindings by only setting the first one */
-          *str_array = g_strdup (value);
-
-          g_settings_set_strv (settings, key, (const char * const *)str_array);
-          g_strfreev (str_array);
-        }
-    }
-
-  g_variant_unref (variant);
-}
-
-
-static void
-_set_binding (CcKeyboardItem *item,
-              const char     *value,
-             gboolean        set_backend)
-{
-  CcKeyboardItem *reverse;
-  gboolean enabled;
-
-  reverse = item->reverse_item;
-  enabled = value && strlen (value) > 0;
-
-  g_clear_pointer (&item->binding, g_free);
-  item->binding = enabled ? g_strdup (value) : g_strdup ("");
-
-  binding_from_string (item->binding, item->primary_combo);
-
-  /*
-   * Always treat the pair (item, reverse) as a unit: setting one also
-   * disables the other, disabling one up also sets the other.
-   */
-  if (reverse)
-    {
-      GdkModifierType reverse_mask;
-
-      reverse_mask = enabled ? item->primary_combo->mask ^ GDK_SHIFT_MASK
-                             : item->primary_combo->mask;
-
-      g_clear_pointer (&reverse->binding, g_free);
-      if (enabled)
-        reverse->binding = gtk_accelerator_name_with_keycode (NULL,
-                                                                    item->primary_combo->keyval,
-                                                                    item->primary_combo->keycode,
-                                                                    reverse_mask);
-
-      binding_from_string (reverse->binding, reverse->primary_combo);
-    }
-
-  if (set_backend == FALSE)
-    return;
-
-  settings_set_binding (item->settings, item->key, item->binding);
-
-  g_object_notify (G_OBJECT (item), "is-value-default");
-
-  if (reverse)
-    {
-      settings_set_binding (reverse->settings, reverse->key, reverse->binding);
-      g_object_notify (G_OBJECT (reverse), "is-value-default");
-    }
-}
-
 static void
 _set_type (CcKeyboardItem *item,
            gint            value)
@@ -271,9 +210,6 @@ cc_keyboard_item_set_property (GObject      *object,
   case PROP_DESCRIPTION:
     _set_description (self, g_value_get_string (value));
     break;
-  case PROP_BINDING:
-    _set_binding (self, g_value_get_string (value), TRUE);
-    break;
   case PROP_COMMAND:
     _set_command (self, g_value_get_string (value));
     break;
@@ -300,9 +236,6 @@ cc_keyboard_item_get_property (GObject    *object,
   case PROP_DESCRIPTION:
     g_value_set_string (value, self->description);
     break;
-  case PROP_BINDING:
-    g_value_set_string (value, self->binding);
-    break;
   case PROP_EDITABLE:
     g_value_set_boolean (value, self->editable);
     break;
@@ -312,6 +245,9 @@ cc_keyboard_item_get_property (GObject    *object,
   case PROP_IS_VALUE_DEFAULT:
     g_value_set_boolean (value, cc_keyboard_item_is_value_default (self));
     break;
+  case PROP_KEY_COMBOS:
+    g_value_set_pointer (value, self->key_combos);
+    break;
   default:
     G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
     break;
@@ -335,14 +271,6 @@ cc_keyboard_item_class_init (CcKeyboardItemClass *klass)
                                                         NULL,
                                                         G_PARAM_READWRITE));
 
-  g_object_class_install_property (object_class,
-                                   PROP_BINDING,
-                                   g_param_spec_string ("binding",
-                                                        "binding",
-                                                        "binding",
-                                                        NULL,
-                                                        G_PARAM_READWRITE));
-
   g_object_class_install_property (object_class,
                                    PROP_EDITABLE,
                                    g_param_spec_boolean ("editable",
@@ -376,12 +304,18 @@ cc_keyboard_item_class_init (CcKeyboardItemClass *klass)
                                                          "is value default",
                                                          TRUE,
                                                          G_PARAM_READABLE));
+
+  g_object_class_install_property (object_class,
+                                   PROP_KEY_COMBOS,
+                                   g_param_spec_pointer ("key-combos",
+                                                         "key combos",
+                                                         "key combos",
+                                                         G_PARAM_READABLE));
 }
 
 static void
 cc_keyboard_item_init (CcKeyboardItem *item)
 {
-  item->primary_combo = g_new0 (CcKeyCombo, 1);
 }
 
 static void
@@ -398,8 +332,6 @@ cc_keyboard_item_finalize (GObject *object)
     g_object_unref (item->settings);
 
   /* Free memory */
-  g_free (item->binding);
-  g_free (item->primary_combo);
   g_free (item->gsettings_path);
   g_free (item->description);
   g_free (item->command);
@@ -514,33 +446,18 @@ translate_above_tab (char **original_bindings,
   return TRUE;
 }
 
-static char *
-translate_binding_string (const char *str)
-{
-  g_autofree guint *above_tab_keysyms = NULL;
-  g_autofree char *symname = NULL;
-  g_auto (GStrv) split_str = NULL;
-
-  if (str == NULL || strstr (str, "Above_Tab") == NULL)
-    return g_strdup (str);
-
-  above_tab_keysyms = get_above_tab_keysyms ();
-  symname = gtk_accelerator_name (above_tab_keysyms[0], 0);
-
-  split_str = g_strsplit (str, "Above_Tab", -1);
-  return g_strjoinv (symname, split_str);
-}
-
 static GList *
 variant_get_key_combos (GVariant *variant)
 {
   GList *combos = NULL;
-  char **bindings = NULL, **translated_bindings, **str;
+  char **translated_bindings, **str;
+  g_auto(GStrv) bindings = NULL;
 
   if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING))
     {
       bindings = g_malloc0_n (2, sizeof(char *));
-      bindings[0] = g_variant_dup_string (variant, NULL);
+      if (g_strcmp0 (g_variant_get_string (variant, NULL), "") != 0)
+        bindings[0] = g_variant_dup_string (variant, NULL);
     }
   else if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY))
     {
@@ -558,9 +475,10 @@ variant_get_key_combos (GVariant *variant)
       CcKeyCombo *combo = g_new (CcKeyCombo, 1);
 
       binding_from_string (*str, combo);
-      combos = g_list_prepend (combos, combo);
+
+      if (combo->keyval != 0 || combo->keycode != 0 || combo->mask != 0)
+        combos = g_list_prepend (combos, combo);
     }
-  g_strfreev (bindings);
 
   return g_list_reverse (combos);
 }
@@ -571,56 +489,27 @@ settings_get_key_combos (GSettings  *settings,
                          gboolean    use_default)
 {
   GList *key_combos;
-  GVariant *variant;
+  g_autoptr(GVariant) variant = NULL;
 
   if (use_default)
     variant = g_settings_get_default_value (settings, key);
   else
     variant = g_settings_get_value (settings, key);
   key_combos = variant_get_key_combos (variant);
-  g_variant_unref (variant);
 
   return key_combos;
 }
 
-/* wrapper around g_settings_get_str[ing|v] */
-static char *
-settings_get_binding (GSettings  *settings,
-                      const char *key)
-{
-  GVariant *variant;
-  char *value = NULL;
-
-  variant = g_settings_get_value (settings, key);
-  if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING))
-    value = translate_binding_string (g_variant_get_string (variant, NULL));
-  else if (g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY))
-    {
-      const char **str_array;
-
-      str_array = g_variant_get_strv (variant, NULL);
-      value = translate_binding_string (str_array[0]);
-      g_free (str_array);
-    }
-  g_variant_unref (variant);
-
-  return value;
-}
-
 static void
 binding_changed (CcKeyboardItem *item,
                 const char *key)
 {
-  char *value;
-
   g_list_free_full (item->key_combos, g_free);
   item->key_combos = settings_get_key_combos (item->settings, item->key, FALSE);
 
-  value = settings_get_binding (item->settings, item->key);
   item->editable = g_settings_is_writable (item->settings, item->key);
-  _set_binding (item, value, FALSE);
-  g_free (value);
-  g_object_notify (G_OBJECT (item), "binding");
+
+  g_object_notify (G_OBJECT (item), "key-combos");
 }
 
 gboolean
@@ -628,6 +517,8 @@ cc_keyboard_item_load_from_gsettings_path (CcKeyboardItem *item,
                                            const char     *path,
                                            gboolean        reset)
 {
+  g_autoptr(GVariant) variant = NULL;
+
   item->schema = g_strdup (CUSTOM_KEYS_SCHEMA);
   item->gsettings_path = g_strdup (path);
   item->key = g_strdup ("binding");
@@ -636,6 +527,9 @@ cc_keyboard_item_load_from_gsettings_path (CcKeyboardItem *item,
   item->desc_editable = g_settings_is_writable (item->settings, "name");
   item->cmd_editable = g_settings_is_writable (item->settings, "command");
 
+  variant = g_settings_get_value (item->settings, item->key);
+  item->can_set_multiple = g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY);
+
   if (reset)
     {
       g_settings_reset (item->settings, "name");
@@ -651,9 +545,6 @@ cc_keyboard_item_load_from_gsettings_path (CcKeyboardItem *item,
   g_list_free_full (item->key_combos, g_free);
   item->key_combos = settings_get_key_combos (item->settings, item->key, FALSE);
 
-  g_free (item->binding);
-  item->binding = settings_get_binding (item->settings, item->key);
-  binding_from_string (item->binding, item->primary_combo);
   g_signal_connect_object (G_OBJECT (item->settings), "changed::binding",
                            G_CALLBACK (binding_changed), item, G_CONNECT_SWAPPED);
 
@@ -666,17 +557,15 @@ cc_keyboard_item_load_from_gsettings (CcKeyboardItem *item,
                                      const char *schema,
                                      const char *key)
 {
-  char *signal_name;
+  g_autofree char *signal_name = NULL;
+  g_autoptr(GVariant) variant = NULL;
 
   item->schema = g_strdup (schema);
   item->key = g_strdup (key);
   item->description = g_strdup (description);
 
   item->settings = g_settings_new (item->schema);
-  g_free (item->binding);
-  item->binding = settings_get_binding (item->settings, item->key);
   item->editable = g_settings_is_writable (item->settings, item->key);
-  binding_from_string (item->binding, item->primary_combo);
 
   g_list_free_full (item->key_combos, g_free);
   item->key_combos = settings_get_key_combos (item->settings, item->key, FALSE);
@@ -684,10 +573,12 @@ cc_keyboard_item_load_from_gsettings (CcKeyboardItem *item,
   g_list_free_full (item->default_combos, g_free);
   item->default_combos = settings_get_key_combos (item->settings, item->key, TRUE);
 
+  variant = g_settings_get_value (item->settings, item->key);
+  item->can_set_multiple = g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING_ARRAY);
+
   signal_name = g_strdup_printf ("changed::%s", item->key);
   g_signal_connect_object (G_OBJECT (item->settings), signal_name,
                            G_CALLBACK (binding_changed), item, G_CONNECT_SWAPPED);
-  g_free (signal_name);
 
   return TRUE;
 }
@@ -761,9 +652,6 @@ cc_keyboard_item_is_hidden (CcKeyboardItem *item)
 gboolean
 cc_keyboard_item_is_value_default (CcKeyboardItem *self)
 {
-  GVariant *user_value;
-  gboolean is_value_default;
-
   g_return_val_if_fail (CC_IS_KEYBOARD_ITEM (self), FALSE);
 
   /*
@@ -773,28 +661,7 @@ cc_keyboard_item_is_value_default (CcKeyboardItem *self)
   if (self->type == CC_KEYBOARD_ITEM_TYPE_GSETTINGS_PATH)
     return TRUE;
 
-  user_value = g_settings_get_user_value (self->settings, self->key);
-
-  is_value_default = TRUE;
-
-  if (user_value)
-    {
-      GVariant *default_value;
-      const gchar *default_binding, *user_binding;
-
-      default_value = g_settings_get_default_value (self->settings, self->key);
-
-      default_binding = get_binding_from_variant (default_value);
-      user_binding = get_binding_from_variant (user_value);
-
-      is_value_default = (g_strcmp0 (default_binding, user_binding) == 0);
-
-      g_clear_pointer (&default_value, g_variant_unref);
-    }
-
-  g_clear_pointer (&user_value, g_variant_unref);
-
-  return is_value_default;
+  return combos_equal (self->default_combos, self->key_combos);
 }
 
 /**
@@ -837,11 +704,11 @@ cc_keyboard_item_get_default_combos (CcKeyboardItem *item)
   return item->default_combos;
 }
 
-CcKeyCombo *
+CcKeyCombo
 cc_keyboard_item_get_primary_combo (CcKeyboardItem *item)
 {
-  g_return_val_if_fail (CC_IS_KEYBOARD_ITEM (item), NULL);
-  return item->primary_combo;
+  g_return_val_if_fail (CC_IS_KEYBOARD_ITEM (item), EMPTY_COMBO);
+  return (item->key_combos) ? *((CcKeyCombo*)item->key_combos->data) : EMPTY_COMBO;
 }
 
 const gchar *
@@ -872,3 +739,136 @@ cc_keyboard_item_get_settings (CcKeyboardItem *item)
   return item->settings;
 }
 
+gboolean
+cc_keyboard_item_can_set_multiple (CcKeyboardItem *item)
+{
+  return item->can_set_multiple;
+}
+
+static gchar*
+combo_get_accelerator (CcKeyCombo *combo)
+{
+  return gtk_accelerator_name_with_keycode (NULL,
+                                            combo->keyval,
+                                            combo->keycode,
+                                            combo->mask);
+}
+
+static void
+cc_keyboard_item_add_key_combo_inner (CcKeyboardItem *self,
+                                      CcKeyCombo *combo)
+{
+  g_auto(GStrv) strv = NULL;
+  int i;
+
+  if (!self->can_set_multiple)
+    {
+      g_settings_set_string (self->settings, self->key, combo_get_accelerator (combo));
+    }
+  else
+    {
+      strv = g_new0 (gchar*, g_list_length (self->key_combos) + 2);
+
+      i = 0;
+      for (GList *l = self->key_combos; l != NULL; l = l->next, i++)
+        {
+          if (combo_equal (l->data, combo))
+            // This combo is already in the list
+            return;
+          strv[i] = combo_get_accelerator (l->data);
+        }
+      strv[i] = combo_get_accelerator (combo);
+
+      g_settings_set_strv (self->settings, self->key, (const gchar **)strv);
+    }
+
+  binding_changed (self, self->key);
+}
+
+void
+cc_keyboard_item_add_key_combo (CcKeyboardItem *self,
+                                CcKeyCombo *combo)
+{
+  CcKeyCombo reverse_combo;
+
+  if (self->reverse_item)
+    {
+      reverse_combo.keyval = combo->keyval;
+      reverse_combo.keycode = combo->keycode;
+      reverse_combo.mask = combo->mask ^ GDK_SHIFT_MASK;
+      cc_keyboard_item_add_key_combo_inner (self->reverse_item, &reverse_combo);
+    }
+
+  cc_keyboard_item_add_key_combo_inner (self, combo);
+}
+
+static void
+cc_keyboard_item_remove_key_combo_inner (CcKeyboardItem *self,
+                                         CcKeyCombo *combo)
+{
+  g_auto(GStrv) strv = NULL;
+  gboolean found;
+  int i;
+
+  strv = g_new0 (gchar*, g_list_length (self->key_combos) + 1);
+
+  found = FALSE;
+  i = 0;
+  for (GList *l = self->key_combos; l != NULL; l = l->next, i++)
+    {
+      if (combo_equal (l->data, combo))
+        {
+          i--;
+          found = TRUE;
+        }
+      else
+        {
+          strv[i] = combo_get_accelerator (l->data);
+        }
+    }
+
+  if (found)
+    {
+      if (self->can_set_multiple)
+        g_settings_set_strv (self->settings, self->key, (const gchar **)strv);
+      else
+        g_settings_set_string (self->settings, self->key, "");
+    }
+
+  binding_changed (self, self->key);
+}
+
+void
+cc_keyboard_item_remove_key_combo (CcKeyboardItem *self,
+                                   CcKeyCombo *combo)
+{
+  CcKeyCombo reverse_combo;
+
+  if (self->reverse_item)
+    {
+      reverse_combo.keyval = combo->keyval;
+      reverse_combo.keycode = combo->keycode;
+      reverse_combo.mask = combo->mask ^ GDK_SHIFT_MASK;
+      cc_keyboard_item_remove_key_combo_inner (self->reverse_item, &reverse_combo);
+    }
+
+  cc_keyboard_item_remove_key_combo_inner (self, combo);
+}
+
+void cc_keyboard_item_disable (CcKeyboardItem *self)
+{
+  if (!self->can_set_multiple)
+    {
+      g_settings_set_string (self->settings, self->key, "");
+      if (self->reverse_item)
+        g_settings_set_string (self->reverse_item->settings, self->reverse_item->key, "");
+    }
+  else
+    {
+      g_settings_set_strv (self->settings, self->key, NULL);
+      if (self->reverse_item)
+        g_settings_set_strv (self->reverse_item->settings, self->reverse_item->key, NULL);
+    }
+
+  binding_changed (self, self->key);
+}
diff --git a/panels/keyboard/cc-keyboard-item.h b/panels/keyboard/cc-keyboard-item.h
index 2c344a773..2f4194dc0 100644
--- a/panels/keyboard/cc-keyboard-item.h
+++ b/panels/keyboard/cc-keyboard-item.h
@@ -90,7 +90,7 @@ GList*             cc_keyboard_item_get_key_combos           (CcKeyboardItem
 
 GList*             cc_keyboard_item_get_default_combos       (CcKeyboardItem     *self);
 
-CcKeyCombo*        cc_keyboard_item_get_primary_combo        (CcKeyboardItem     *self);
+CcKeyCombo         cc_keyboard_item_get_primary_combo        (CcKeyboardItem     *self);
 
 const gchar*       cc_keyboard_item_get_key                  (CcKeyboardItem     *self);
 
@@ -100,4 +100,13 @@ const gchar*       cc_keyboard_item_get_gsettings_path       (CcKeyboardItem
 
 GSettings*         cc_keyboard_item_get_settings             (CcKeyboardItem     *self);
 
+gboolean           cc_keyboard_item_can_set_multiple         (CcKeyboardItem     *self);
+
+void               cc_keyboard_item_add_key_combo            (CcKeyboardItem     *self,
+                                                              CcKeyCombo         *combo);
+
+void               cc_keyboard_item_remove_key_combo         (CcKeyboardItem     *self,
+                                                              CcKeyCombo         *combo);
+void               cc_keyboard_item_disable                  (CcKeyboardItem     *self);
+
 G_END_DECLS
diff --git a/panels/keyboard/cc-keyboard-manager.c b/panels/keyboard/cc-keyboard-manager.c
index 55ae3afb1..4d633c2a6 100644
--- a/panels/keyboard/cc-keyboard-manager.c
+++ b/panels/keyboard/cc-keyboard-manager.c
@@ -873,22 +873,6 @@ cc_keyboard_manager_get_collision (CcKeyboardManager *self,
   return data.conflict_item;
 }
 
-/**
- * cc_keyboard_manager_disable_shortcut:
- * @self: a #CcKeyboardManager
- * @item: a @CcKeyboardItem
- *
- * Disables the given keyboard shortcut.
- */
-void
-cc_keyboard_manager_disable_shortcut (CcKeyboardManager *self,
-                                      CcKeyboardItem    *item)
-{
-  g_return_if_fail (CC_IS_KEYBOARD_MANAGER (self));
-
-  g_object_set (item, "binding", NULL, NULL);
-}
-
 /**
  * cc_keyboard_manager_reset_shortcut:
  * @self: a #CcKeyboardManager
@@ -915,7 +899,7 @@ cc_keyboard_manager_reset_shortcut (CcKeyboardManager *self,
 
       collision = cc_keyboard_manager_get_collision (self, NULL, combo);
       if (collision)
-        cc_keyboard_manager_disable_shortcut (self, collision);
+        cc_keyboard_item_remove_key_combo (collision, combo);
     }
 
   /* Resets the current item */
diff --git a/panels/keyboard/cc-keyboard-manager.h b/panels/keyboard/cc-keyboard-manager.h
index 2b03752e8..5de19c0a5 100644
--- a/panels/keyboard/cc-keyboard-manager.h
+++ b/panels/keyboard/cc-keyboard-manager.h
@@ -47,9 +47,6 @@ CcKeyboardItem*      cc_keyboard_manager_get_collision           (CcKeyboardMana
                                                                   CcKeyboardItem     *item,
                                                                   CcKeyCombo         *combo);
 
-void                 cc_keyboard_manager_disable_shortcut        (CcKeyboardManager  *self,
-                                                                  CcKeyboardItem     *item);
-
 void                 cc_keyboard_manager_reset_shortcut          (CcKeyboardManager  *self,
                                                                   CcKeyboardItem     *item);
 
diff --git a/panels/keyboard/cc-keyboard-panel.c b/panels/keyboard/cc-keyboard-panel.c
index f47e34cb6..f4d8d7c2c 100644
--- a/panels/keyboard/cc-keyboard-panel.c
+++ b/panels/keyboard/cc-keyboard-panel.c
@@ -131,7 +131,7 @@ transform_binding_to_accel (GBinding     *binding,
                             gpointer      user_data)
 {
   CcKeyboardItem *item;
-  CcKeyCombo *combo;
+  CcKeyCombo combo;
   gchar *accelerator;
 
   item = CC_KEYBOARD_ITEM (g_binding_get_source (binding));
@@ -142,13 +142,13 @@ transform_binding_to_accel (GBinding     *binding,
     {
       g_autofree gchar *tmp = NULL;
 
-      tmp = convert_keysym_state_to_string (combo);
+      tmp = convert_keysym_state_to_string (&combo);
 
       accelerator = g_strdup_printf ("<b>%s</b>", tmp);
     }
   else
     {
-      accelerator = convert_keysym_state_to_string (combo);
+      accelerator = convert_keysym_state_to_string (&combo);
     }
 
   g_value_take_string (to_value, accelerator);
@@ -283,10 +283,10 @@ add_item (CcKeyboardPanel *self,
   gtk_size_group_add_widget (self->accelerator_sizegroup, label);
 
   g_object_bind_property_full (item,
-                               "binding",
+                               "key-combos",
                                label,
-                              "label",
-                               G_SETTINGS_BIND_GET | G_BINDING_SYNC_CREATE,
+                               "label",
+                              G_BINDING_SYNC_CREATE,
                                transform_binding_to_accel,
                                NULL, NULL, NULL);
 
@@ -417,18 +417,18 @@ static gboolean
 search_match_shortcut (CcKeyboardItem *item,
                        const gchar    *search)
 {
-  CcKeyCombo *combo = cc_keyboard_item_get_primary_combo (item);
+  CcKeyCombo combo = cc_keyboard_item_get_primary_combo (item);
   GStrv shortcut_tokens, search_tokens;
   g_autofree gchar *normalized_accel = NULL;
   g_autofree gchar *accel = NULL;
   gboolean match;
   guint i;
 
-  if (is_empty_binding (combo))
+  if (is_empty_binding (&combo))
     return FALSE;
 
   match = TRUE;
-  accel = convert_keysym_state_to_string (combo);
+  accel = convert_keysym_state_to_string (&combo);
   normalized_accel = cc_util_normalize_casefold_and_unaccent (accel);
 
   shortcut_tokens = g_strsplit_set (normalized_accel, SHORTCUT_DELIMITERS, -1);
diff --git a/panels/keyboard/cc-keyboard-shortcut-editor.c b/panels/keyboard/cc-keyboard-shortcut-editor.c
index 117f1e565..0a4b623ac 100644
--- a/panels/keyboard/cc-keyboard-shortcut-editor.c
+++ b/panels/keyboard/cc-keyboard-shortcut-editor.c
@@ -144,22 +144,12 @@ apply_custom_item_fields (CcKeyboardShortcutEditor *self,
   /* Only setup the binding when it was actually edited */
   if (self->edited)
     {
-      CcKeyCombo *combo = cc_keyboard_item_get_primary_combo (item);
-      g_autofree gchar *binding = NULL;
+      CcKeyCombo *combo = self->custom_combo;
 
-      combo->keycode = self->custom_combo->keycode;
-      combo->keyval = self->custom_combo->keyval;
-      combo->mask = self->custom_combo->mask;
+      cc_keyboard_item_disable (item);
 
-      if (combo->keycode == 0 && combo->keyval == 0 && combo->mask == 0)
-        binding = g_strdup ("");
-      else
-        binding = gtk_accelerator_name_with_keycode (NULL,
-                                                     combo->keyval,
-                                                     combo->keycode,
-                                                     combo->mask);
-
-      g_object_set (G_OBJECT (item), "binding", binding, NULL);
+      if (combo->keycode != 0 || combo->keyval != 0 || combo->mask != 0)
+        cc_keyboard_item_add_key_combo (item, combo);
     }
 
   /* Set the keyboard shortcut name and command for custom entries */
@@ -263,7 +253,7 @@ update_shortcut (CcKeyboardShortcutEditor *self)
 
   /* Eventually disable the conflict shortcut */
   if (self->collision_item)
-    cc_keyboard_manager_disable_shortcut (self->manager, self->collision_item);
+    cc_keyboard_item_disable (self->collision_item);
 
   /* Cleanup whatever was set before */
   clear_custom_entries (self);
@@ -447,7 +437,7 @@ add_button_clicked_cb (CcKeyboardShortcutEditor *self)
 
   /* Eventually disable the conflict shortcut */
   if (self->collision_item)
-    cc_keyboard_manager_disable_shortcut (self->manager, self->collision_item);
+    cc_keyboard_item_disable (self->collision_item);
 
   /* Cleanup everything once we're done */
   clear_custom_entries (self);
@@ -513,14 +503,14 @@ reset_custom_clicked_cb (CcKeyboardShortcutEditor *self)
 static void
 reset_item_clicked_cb (CcKeyboardShortcutEditor *self)
 {
-  CcKeyCombo *combo;
+  CcKeyCombo combo;
   gchar *accel;
 
   /* Reset first, then update the shortcut */
   cc_keyboard_manager_reset_shortcut (self->manager, self->item);
 
   combo = cc_keyboard_item_get_primary_combo (self->item);
-  accel = gtk_accelerator_name (combo->keyval, combo->mask);
+  accel = gtk_accelerator_name (combo.keyval, combo.mask);
   gtk_shortcut_label_set_accelerator (GTK_SHORTCUT_LABEL (self->shortcut_accel_label), accel);
 
   g_free (accel);
@@ -537,7 +527,7 @@ static void
 setup_keyboard_item (CcKeyboardShortcutEditor *self,
                      CcKeyboardItem           *item)
 {
-  CcKeyCombo *combo;
+  CcKeyCombo combo;
   gboolean is_custom;
   g_autofree gchar *accel = NULL;
   g_autofree gchar *description_text = NULL;
@@ -550,14 +540,12 @@ setup_keyboard_item (CcKeyboardShortcutEditor *self,
 
   combo = cc_keyboard_item_get_primary_combo (item);
   is_custom = cc_keyboard_item_get_item_type (item) == CC_KEYBOARD_ITEM_TYPE_GSETTINGS_PATH;
-  accel = gtk_accelerator_name (combo->keyval, combo->mask);
+  accel = gtk_accelerator_name (combo.keyval, combo.mask);
 
   /* To avoid accidentally thinking we unset the current keybinding, set the values
    * of the keyboard item that is being edited */
   self->custom_is_modifier = FALSE;
-  self->custom_combo->keycode = combo->keycode;
-  self->custom_combo->keyval = combo->keyval;
-  self->custom_combo->mask = combo->mask;
+  *self->custom_combo = combo;
 
   /* Headerbar */
   gtk_header_bar_set_title (self->headerbar,
diff --git a/panels/keyboard/keyboard-shortcuts.c b/panels/keyboard/keyboard-shortcuts.c
index ba748a630..ed04c57f6 100644
--- a/panels/keyboard/keyboard-shortcuts.c
+++ b/panels/keyboard/keyboard-shortcuts.c
@@ -243,7 +243,7 @@ keyval_is_forbidden (guint keyval)
 }
 
 gboolean
-is_valid_binding (CcKeyCombo *combo)
+is_valid_binding (const CcKeyCombo *combo)
 {
   if ((combo->mask == 0 || combo->mask == GDK_SHIFT_MASK) && combo->keycode != 0)
     {
@@ -268,7 +268,7 @@ is_valid_binding (CcKeyCombo *combo)
 }
 
 gboolean
-is_empty_binding (CcKeyCombo *combo)
+is_empty_binding (const CcKeyCombo *combo)
 {
   if (combo->keyval == 0 &&
       combo->mask == 0 &&
@@ -278,7 +278,7 @@ is_empty_binding (CcKeyCombo *combo)
 }
 
 gboolean
-is_valid_accel (CcKeyCombo *combo)
+is_valid_accel (const CcKeyCombo *combo)
 {
   /* Unlike gtk_accelerator_valid(), we want to allow Tab when combined
    * with some modifiers (Alt+Tab and friends)
@@ -356,7 +356,7 @@ parse_keylist_from_file (const gchar *path)
  * https://git.gnome.org/browse/gtk+/tree/gtk/gtkcellrendereraccel.c#n261
  */
 gchar*
-convert_keysym_state_to_string (CcKeyCombo *combo)
+convert_keysym_state_to_string (const CcKeyCombo *combo)
 {
   gchar *name;
 
diff --git a/panels/keyboard/keyboard-shortcuts.h b/panels/keyboard/keyboard-shortcuts.h
index e303486d4..e8e98cb32 100644
--- a/panels/keyboard/keyboard-shortcuts.h
+++ b/panels/keyboard/keyboard-shortcuts.h
@@ -67,12 +67,12 @@ enum
 
 gchar*   find_free_settings_path        (GSettings *settings);
 
-gboolean is_valid_binding               (CcKeyCombo *combo);
+gboolean is_valid_binding               (const CcKeyCombo *combo);
 
-gboolean is_empty_binding               (CcKeyCombo *combo);
+gboolean is_empty_binding               (const CcKeyCombo *combo);
 
-gboolean is_valid_accel                 (CcKeyCombo *combo);
+gboolean is_valid_accel                 (const CcKeyCombo *combo);
 
 KeyList* parse_keylist_from_file        (const gchar *path);
 
-gchar*   convert_keysym_state_to_string (CcKeyCombo *combo);
+gchar*   convert_keysym_state_to_string (const CcKeyCombo *combo);