From ab353591f81b23b50bce22897b529150bcfb79e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Tue, 27 Aug 2013 01:57:41 +0200
Subject: [PATCH] keyboard: Refactor finding of conflicting items

When comparing keys for uniqueness, we currently apply various tests to
check whether shortcuts are different, until we decide that we found a
conflict if none of the tests passed. That approach is a bit weird for
shortcuts that have a reverse item - when comparing a binding to two
different shortcuts, it should always be different from at least one of
them, so there should never be a conflict for any reversible shortcuts.
The reason it does work anyway is that reverse items usually only differ
in modifiers, which is_shortcut_different() currently doesn't consider at
all. We are about to change that however, so refactor the code to set the
conflicting item as soon as we find a match rather than as fall-through.

https://bugzilla.gnome.org/show_bug.cgi?id=673078
---
 panels/keyboard/cc-keyboard-manager.c | 34 ++++++++++++---------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/panels/keyboard/cc-keyboard-manager.c b/panels/keyboard/cc-keyboard-manager.c
index bea97191c..aff1908bd 100644
--- a/panels/keyboard/cc-keyboard-manager.c
+++ b/panels/keyboard/cc-keyboard-manager.c
@@ -102,25 +102,24 @@ get_binding_from_variant (GVariant *variant)
 }
 
 static gboolean
-is_shortcut_different (CcUniquenessData *data,
-                       CcKeyboardItem   *item)
+find_conflict (CcUniquenessData *data,
+               CcKeyboardItem   *item)
 {
   CcKeyCombo *combo = item->primary_combo;
+  gboolean is_conflict = FALSE;
 
   if (data->orig_item && cc_keyboard_item_equal (data->orig_item, item))
     return FALSE;
 
   if (data->new_keyval != 0)
-    {
-      if (data->new_keyval != combo->keyval)
-        return TRUE;
-    }
-  else if (combo->keyval != 0 || data->new_keycode != combo->keycode)
-    {
-      return TRUE;
-    }
+    is_conflict = data->new_keyval == combo->keyval;
+  else
+    is_conflict = combo->keyval == 0 && data->new_keycode == combo->keycode;
 
-  return FALSE;
+  if (is_conflict)
+    data->conflict_item = item;
+
+  return is_conflict;
 }
 
 static gboolean
@@ -143,17 +142,14 @@ compare_keys_for_uniqueness (CcKeyboardItem   *current_item,
   if (reverse_item && cc_keyboard_item_is_hidden (current_item))
     return FALSE;
 
-  if (is_shortcut_different (data, current_item))
-    return FALSE;
+  if (find_conflict (data, current_item))
+    return TRUE;
 
   /* Also check for the reverse item if any */
-  if (reverse_item && is_shortcut_different (data, reverse_item))
-    return FALSE;
+  if (reverse_item && find_conflict (data, reverse_item))
+    return TRUE;
 
-  /* No tests failed and we found a conflict */
-  data->conflict_item = current_item;
-
-  return TRUE;
+  return FALSE;
 }
 
 static gboolean