From 4e50c3c37564b0e68c86d1e7547d5e1a9a08c887 Mon Sep 17 00:00:00 2001 From: Georges Basile Stavracas Neto Date: Wed, 13 Jul 2016 20:09:17 -0300 Subject: [PATCH] keyboard: bring back uniqueness check The collision detection code was removed in commit a0a155884ef66ca as the cleanup was happening because the previous code was closely tied to the user interface components. Because that code wasn't appliable to the new listbox UI, it was temporarily removed. This patch re-adds this feature to work with the new code orgazination. https://bugzilla.gnome.org/show_bug.cgi?id=769063 --- panels/keyboard/cc-keyboard-manager.c | 122 ++++++++++++++++++ panels/keyboard/cc-keyboard-manager.h | 9 ++ panels/keyboard/cc-keyboard-shortcut-editor.c | 93 +++++++++++++ panels/keyboard/shortcut-editor.ui | 13 ++ 4 files changed, 237 insertions(+) diff --git a/panels/keyboard/cc-keyboard-manager.c b/panels/keyboard/cc-keyboard-manager.c index a98baeb8f..a9809a91c 100644 --- a/panels/keyboard/cc-keyboard-manager.c +++ b/panels/keyboard/cc-keyboard-manager.c @@ -84,6 +84,62 @@ free_key_array (GPtrArray *keys) } } +static gboolean +compare_keys_for_uniqueness (CcKeyboardItem *current_item, + CcUniquenessData *data) +{ + CcKeyboardItem *item; + + item = data->orig_item; + + /* No conflict for: blanks, different modifiers or ourselves */ + if (!current_item || + item == current_item || + data->new_mask != current_item->mask) + { + return FALSE; + } + + if (item && cc_keyboard_item_equal (item, current_item)) + goto out; + + if (data->new_keyval != 0) + { + if (data->new_keyval != current_item->keyval) + return FALSE; + } + else if (current_item->keyval != 0 || data->new_keycode != current_item->keycode) + { + return FALSE; + } + +out: + data->conflict_item = current_item; + + return TRUE; +} + +static gboolean +check_for_uniqueness (gpointer key, + GPtrArray *keys_array, + CcUniquenessData *data) +{ + guint i; + + for (i = 0; i < keys_array->len; i++) + { + CcKeyboardItem *item; + + item = keys_array->pdata[i]; + + if (compare_keys_for_uniqueness (item, data)) + return TRUE; + } + + return FALSE; +} + + static GHashTable* get_hash_for_group (CcKeyboardManager *self, BindingGroupType group) @@ -815,3 +871,69 @@ cc_keyboard_manager_remove_custom_shortcut (CcKeyboardManager *self, g_signal_emit (self, signals[SHORTCUT_REMOVED], 0, item); } + +/** + * cc_keyboard_manager_get_collision: + * @self: a #CcKeyboardManager + * @item: (nullable): a keyboard shortcut + * @keyval: the key value + * @mask: a mask for the key sequence + * @keycode: the code of the key. + * + * Retrieves the collision item for the given shortcut. + * + * Returns: (transfer none)(nullable): the collisioned shortcut + */ +CcKeyboardItem* +cc_keyboard_manager_get_collision (CcKeyboardManager *self, + CcKeyboardItem *item, + gint keyval, + GdkModifierType mask, + gint keycode) +{ + CcUniquenessData data; + + g_return_val_if_fail (CC_IS_KEYBOARD_MANAGER (self), NULL); + + data.orig_item = item; + data.new_keyval = keyval; + data.new_mask = mask; + data.new_keycode = keycode; + data.conflict_item = NULL; + + /* Any number of shortcuts can be disabled */ + if (keyval != 0 || keycode != 0) + { + BindingGroupType i; + + for (i = BINDING_GROUP_SYSTEM; i <= BINDING_GROUP_USER && !data.conflict_item; i++) + { + GHashTable *table; + + table = get_hash_for_group (self, i); + + if (!table) + continue; + + g_hash_table_find (table, (GHRFunc) check_for_uniqueness, &data); + } + } + + 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); +} diff --git a/panels/keyboard/cc-keyboard-manager.h b/panels/keyboard/cc-keyboard-manager.h index ab14d4a4a..b63ffa582 100644 --- a/panels/keyboard/cc-keyboard-manager.h +++ b/panels/keyboard/cc-keyboard-manager.h @@ -45,6 +45,15 @@ void cc_keyboard_manager_add_custom_shortcut (CcKeyboardMana void cc_keyboard_manager_remove_custom_shortcut (CcKeyboardManager *self, CcKeyboardItem *item); +CcKeyboardItem* cc_keyboard_manager_get_collision (CcKeyboardManager *self, + CcKeyboardItem *item, + gint keyval, + GdkModifierType mask, + gint keycode); + +void cc_keyboard_manager_disable_shortcut (CcKeyboardManager *self, + CcKeyboardItem *item); + G_END_DECLS #endif /* CC_KEYBOARD_MANAGER_H */ diff --git a/panels/keyboard/cc-keyboard-shortcut-editor.c b/panels/keyboard/cc-keyboard-shortcut-editor.c index eef7a2872..962eb8681 100644 --- a/panels/keyboard/cc-keyboard-shortcut-editor.c +++ b/panels/keyboard/cc-keyboard-shortcut-editor.c @@ -40,9 +40,11 @@ struct _CcKeyboardShortcutEditor GtkWidget *edit_button; GtkWidget *headerbar; GtkWidget *name_entry; + GtkWidget *new_shortcut_conflict_label; GtkWidget *remove_button; GtkWidget *replace_button; GtkWidget *shortcut_accel_label; + GtkWidget *shortcut_conflict_label; GtkWidget *stack; GtkWidget *top_info_label; @@ -53,6 +55,8 @@ struct _CcKeyboardShortcutEditor CcKeyboardManager *manager; CcKeyboardItem *item; + CcKeyboardItem *collision_item; + /* Custom shortcuts */ GdkDevice *grab_pointer; @@ -122,6 +126,8 @@ clear_custom_entries (CcKeyboardShortcutEditor *self) gtk_entry_set_text (GTK_ENTRY (self->command_entry), ""); gtk_shortcut_label_set_accelerator (GTK_SHORTCUT_LABEL (self->custom_shortcut_accel_label), ""); + gtk_label_set_label (GTK_LABEL (self->new_shortcut_conflict_label), ""); + gtk_label_set_label (GTK_LABEL (self->shortcut_conflict_label), ""); self->custom_keycode = 0; self->custom_keyval = 0; @@ -129,6 +135,8 @@ clear_custom_entries (CcKeyboardShortcutEditor *self) self->custom_is_modifier = TRUE; self->edited = FALSE; + self->collision_item = NULL; + g_signal_handlers_unblock_by_func (self->command_entry, command_entry_changed_cb, self); g_signal_handlers_unblock_by_func (self->name_entry, name_entry_changed_cb, self); } @@ -200,6 +208,10 @@ update_shortcut (CcKeyboardShortcutEditor *self) /* Setup the binding */ apply_custom_item_fields (self, self->item); + /* Eventually disable the conflict shortcut */ + if (self->collision_item) + cc_keyboard_manager_disable_shortcut (self->manager, self->collision_item); + /* Cleanup whatever was set before */ clear_custom_entries (self); @@ -215,10 +227,21 @@ get_current_shortcut_label (CcKeyboardShortcutEditor *self) return GTK_SHORTCUT_LABEL (self->shortcut_accel_label); } +static void +set_collision_headerbar (CcKeyboardShortcutEditor *self, + gboolean has_collision) +{ + gtk_header_bar_set_show_close_button (GTK_HEADER_BAR (self->headerbar), !has_collision); + + gtk_widget_set_visible (self->cancel_button, has_collision); + gtk_widget_set_visible (self->replace_button, has_collision); +} + static void setup_custom_shortcut (CcKeyboardShortcutEditor *self) { GtkShortcutLabel *shortcut_label; + CcKeyboardItem *collision_item; gboolean valid; gchar *accel; @@ -240,8 +263,16 @@ setup_custom_shortcut (CcKeyboardShortcutEditor *self) return; shortcut_label = get_current_shortcut_label (self); + + collision_item = cc_keyboard_manager_get_collision (self->manager, + self->item, + self->custom_keyval, + self->custom_mask, + self->custom_keycode); + accel = gtk_accelerator_name (self->custom_keyval, self->custom_mask); + /* Setup the accelerator label */ gtk_shortcut_label_set_accelerator (shortcut_label, accel); @@ -255,6 +286,48 @@ setup_custom_shortcut (CcKeyboardShortcutEditor *self) release_grab (self); + /* + * Oops! Looks like the accelerator is already being used, so we + * must warn the user and let it be very clear that adding this + * shortcut will disable the other. + */ + gtk_widget_set_visible (self->new_shortcut_conflict_label, collision_item != NULL); + + if (collision_item) + { + GtkWidget *label; + gchar *friendly_accelerator; + gchar *collision_text; + + friendly_accelerator = convert_keysym_state_to_string (self->custom_keyval, + self->custom_mask, + self->custom_keycode); + + collision_text = g_strdup_printf (_("%s is already being used for %s. If you " + "replace it, %s will be disabled"), + friendly_accelerator, + collision_item->description, + collision_item->description); + + label = is_custom_shortcut (self) ? self->new_shortcut_conflict_label : self->shortcut_conflict_label; + + gtk_label_set_markup (GTK_LABEL (label), collision_text); + + g_free (friendly_accelerator); + g_free (collision_text); + } + + /* + * When there is a collision between the current shortcut and another shortcut, + * and we're editing an existing shortcut (rather than creating a new one), setup + * the headerbar to display "Cancel" and "Replace". Otherwise, make sure to set + * only the close button again. + */ + if (self->mode == CC_SHORTCUT_EDITOR_EDIT) + set_collision_headerbar (self, collision_item != NULL); + + self->collision_item = collision_item; + g_free (accel); } @@ -268,6 +341,10 @@ add_button_clicked_cb (CcKeyboardShortcutEditor *self) /* Apply the custom shortcut setup at the new item */ apply_custom_item_fields (self, item); + /* Eventually disable the conflict shortcut */ + if (self->collision_item) + cc_keyboard_manager_disable_shortcut (self->manager, self->collision_item); + /* Cleanup everything once we're done */ clear_custom_entries (self); @@ -317,6 +394,14 @@ remove_button_clicked_cb (CcKeyboardShortcutEditor *self) cc_keyboard_manager_remove_custom_shortcut (self->manager, self->item); } +static void +replace_button_clicked_cb (CcKeyboardShortcutEditor *self) +{ + update_shortcut (self); + + gtk_widget_hide (GTK_WIDGET (self)); +} + static void setup_keyboard_item (CcKeyboardShortcutEditor *self, CcKeyboardItem *item) @@ -572,9 +657,11 @@ cc_keyboard_shortcut_editor_class_init (CcKeyboardShortcutEditorClass *klass) gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, edit_button); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, headerbar); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, name_entry); + gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, new_shortcut_conflict_label); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, remove_button); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, replace_button); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, shortcut_accel_label); + gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, shortcut_conflict_label); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, stack); gtk_widget_class_bind_template_child (widget_class, CcKeyboardShortcutEditor, top_info_label); @@ -584,6 +671,7 @@ cc_keyboard_shortcut_editor_class_init (CcKeyboardShortcutEditorClass *klass) gtk_widget_class_bind_template_callback (widget_class, edit_custom_shortcut_button_toggled_cb); gtk_widget_class_bind_template_callback (widget_class, name_entry_changed_cb); gtk_widget_class_bind_template_callback (widget_class, remove_button_clicked_cb); + gtk_widget_class_bind_template_callback (widget_class, replace_button_clicked_cb); } static void @@ -660,12 +748,17 @@ void cc_keyboard_shortcut_editor_set_mode (CcKeyboardShortcutEditor *self, CcShortcutEditorMode mode) { + gboolean is_create_mode; + g_return_if_fail (CC_IS_KEYBOARD_SHORTCUT_EDITOR (self)); if (self->mode == mode) return; self->mode = mode; + is_create_mode = mode == CC_SHORTCUT_EDITOR_CREATE; + + gtk_widget_set_visible (self->new_shortcut_conflict_label, is_create_mode); if (mode == CC_SHORTCUT_EDITOR_CREATE) { diff --git a/panels/keyboard/shortcut-editor.ui b/panels/keyboard/shortcut-editor.ui index 07b7cabac..65dc2e370 100644 --- a/panels/keyboard/shortcut-editor.ui +++ b/panels/keyboard/shortcut-editor.ui @@ -45,6 +45,18 @@ Disabled + + + True + False + True + True + word-char + 15 + 20 + 0 + + Reset @@ -242,6 +254,7 @@ True True True + end