From fc48a297ed0f5dd974b6177f8f4c643db3da3652 Mon Sep 17 00:00:00 2001
From: hwangcc23 <hwangcc@csie.nctu.edu.tw>
Date: Fri, 25 Sep 2015 22:20:28 +0800
Subject: [PATCH] Check duplicated bindings after translating keysym

1). See the issue #1926. For example, the second keybinding is not detected as a duplicate:
        bindcode Mod4+24 sticky toggle
        bindsym Mod4+q focus parent
2). To fix it, check duplicated bindings when translating the keysym to keycodes.
---
 include/config_parser.h |  5 ++++
 src/bindings.c          | 22 +++++++++++++++--
 src/config_parser.c     | 52 +++++++++++++++++++++++------------------
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/include/config_parser.h b/include/config_parser.h
index 28c28e48..9c23a11d 100644
--- a/include/config_parser.h
+++ b/include/config_parser.h
@@ -31,6 +31,11 @@ struct ConfigResultIR {
 
 struct ConfigResultIR *parse_config(const char *input, struct context *context);
 
+/**
+ * launch nagbar to indicate errors in the configuration file.
+ */
+void start_config_error_nagbar(const char *configpath, bool has_errors);
+
 /**
  * Parses the given file by first replacing the variables, then calling
  * parse_config and launching i3-nagbar if use_nagbar is true.
diff --git a/src/bindings.c b/src/bindings.c
index 32aac05a..086d230a 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -319,6 +319,7 @@ void translate_keysyms(void) {
         return;
     }
 
+    bool has_errors = false;
     Binding *bind;
     TAILQ_FOREACH(bind, bindings, bindings) {
         if (bind->input_type == B_MOUSE) {
@@ -389,6 +390,21 @@ void translate_keysyms(void) {
             sasprintf(&tmp, "%s %d", keycodes, bind->translated_to[n]);
             free(keycodes);
             keycodes = tmp;
+
+            /* check for duplicate bindings */
+            Binding *check;
+            TAILQ_FOREACH(check, bindings, bindings) {
+                if (check == bind)
+                    continue;
+                if (check->symbol != NULL)
+                    continue;
+                if (check->keycode != bind->translated_to[n] ||
+                    check->event_state_mask != bind->event_state_mask ||
+                    check->release != bind->release)
+                    continue;
+                has_errors = true;
+                ELOG("Duplicate keybinding in config file:\n  keysym = %s, keycode = %d, state_mask = 0x%x\n", bind->symbol, check->keycode, bind->event_state_mask);
+            }
         }
         DLOG("state=0x%x, cfg=\"%s\", sym=0x%x → keycodes%s (%d)\n",
              bind->event_state_mask, bind->symbol, keysym, keycodes, bind->number_keycodes);
@@ -396,6 +412,10 @@ void translate_keysyms(void) {
     }
 
     xkb_state_unref(dummy_state);
+
+    if (has_errors) {
+        start_config_error_nagbar(current_configpath, true);
+    }
 }
 
 /*
@@ -514,8 +534,6 @@ void check_for_duplicate_bindings(struct context *context) {
 
             /* Check if one is using keysym while the other is using bindsym.
              * If so, skip. */
-            /* XXX: It should be checked at a later place (when translating the
-             * keysym to keycodes) if there are any duplicates */
             if ((bind->symbol == NULL && current->symbol != NULL) ||
                 (bind->symbol != NULL && current->symbol == NULL))
                 continue;
diff --git a/src/config_parser.c b/src/config_parser.c
index ea00412d..2b4572b0 100644
--- a/src/config_parser.c
+++ b/src/config_parser.c
@@ -830,6 +830,34 @@ static char *migrate_config(char *input, off_t size) {
     return converted;
 }
 
+/**
+ * Launch nagbar to indicate errors in the configuration file.
+ */
+void start_config_error_nagbar(const char *configpath, bool has_errors) {
+    char *editaction, *pageraction;
+    sasprintf(&editaction, "i3-sensible-editor \"%s\" && i3-msg reload\n", configpath);
+    sasprintf(&pageraction, "i3-sensible-pager \"%s\"\n", errorfilename);
+    char *argv[] = {
+        NULL, /* will be replaced by the executable path */
+        "-f",
+        (config.font.pattern ? config.font.pattern : "fixed"),
+        "-t",
+        (has_errors ? "error" : "warning"),
+        "-m",
+        (has_errors ? "You have an error in your i3 config file!" : "Your config is outdated. Please fix the warnings to make sure everything works."),
+        "-b",
+        "edit config",
+        editaction,
+        (errorfilename ? "-b" : NULL),
+        (has_errors ? "show errors" : "show warnings"),
+        pageraction,
+        NULL};
+
+    start_nagbar(&config_error_nagbar_pid, argv);
+    free(editaction);
+    free(pageraction);
+}
+
 /*
  * Parses the given file by first replacing the variables, then calling
  * parse_config and possibly launching i3-nagbar.
@@ -1005,29 +1033,7 @@ bool parse_file(const char *f, bool use_nagbar) {
         if (version == 3)
             ELOG("Please convert your configfile first, then fix any remaining errors (see above).\n");
 
-        char *editaction,
-            *pageraction;
-        sasprintf(&editaction, "i3-sensible-editor \"%s\" && i3-msg reload\n", f);
-        sasprintf(&pageraction, "i3-sensible-pager \"%s\"\n", errorfilename);
-        char *argv[] = {
-            NULL, /* will be replaced by the executable path */
-            "-f",
-            (config.font.pattern ? config.font.pattern : "fixed"),
-            "-t",
-            (context->has_errors ? "error" : "warning"),
-            "-m",
-            (context->has_errors ? "You have an error in your i3 config file!" : "Your config is outdated. Please fix the warnings to make sure everything works."),
-            "-b",
-            "edit config",
-            editaction,
-            (errorfilename ? "-b" : NULL),
-            (context->has_errors ? "show errors" : "show warnings"),
-            pageraction,
-            NULL};
-
-        start_nagbar(&config_error_nagbar_pid, argv);
-        free(editaction);
-        free(pageraction);
+        start_config_error_nagbar(f, context->has_errors);
     }
 
     bool has_errors = context->has_errors;