Merge pull request #3669 from orestisf1993/config

Allow checking for duplicate bindings with -C

include/configuration.h

@@ -400,28 +400,24 @@ struct tray_output_t {
     tray_outputs;
 };
 
-/**
+typedef enum {
- * Finds the configuration file to use (either the one specified by
+    C_VALIDATE,
- * override_configpath), the user’s one or the system default) and calls
+    C_LOAD,
- * parse_file().
+    C_RELOAD,
- *
+} config_load_t;
- * If you specify override_configpath, only this path is used to look for a
- * configuration file.
- *
- * If use_nagbar is false, don't try to start i3-nagbar but log the errors to
- * stdout/stderr instead.
- *
- */
-bool parse_configuration(const char *override_configpath, bool use_nagbar);
 
 /**
- * Reads the configuration from ~/.i3/config or /etc/i3/config if not found.
+ * (Re-)loads the configuration file (sets useful defaults before).
  *
  * If you specify override_configpath, only this path is used to look for a
  * configuration file.
  *
+ * load_type specifies the type of loading: C_VALIDATE is used to only verify
+ * the correctness of the config file (used with the flag -C). C_LOAD will load
+ * the config for normal use and display errors in the nagbar. C_RELOAD will
+ * also clear the previous config.
  */
-void load_configuration(xcb_connection_t *conn, const char *override_configfile, bool reload);
+bool load_configuration(const char *override_configfile, config_load_t load_type);
 
 /**
  * Ungrabs all keys, to be called before re-grabbing the keys because of a
@@ -435,14 +431,3 @@ void ungrab_all_keys(xcb_connection_t *conn);
  *
  */
 void update_barconfig(void);
-
-/**
- * Kills the configerror i3-nagbar process, if any.
- *
- * Called when reloading/restarting.
- *
- * If wait_for_it is set (restarting), this function will waitpid(), otherwise,
- * ev is assumed to handle it (reloading).
- *
- */
-void kill_configerror_nagbar(bool wait_for_it);

libi3/font.c

@@ -163,6 +163,7 @@ i3Font load_font(const char *pattern, const bool fallback) {
 
     i3Font font;
     font.type = FONT_TYPE_NONE;
+    font.pattern = NULL;
 
     /* No XCB connction, return early because we're just validating the
      * configuration file. */

src/commands.c

@@ -1573,7 +1573,7 @@ void cmd_reload(I3_CMD) {
     LOG("reloading\n");
     kill_nagbar(&config_error_nagbar_pid, false);
     kill_nagbar(&command_error_nagbar_pid, false);
-    load_configuration(conn, NULL, true);
+    load_configuration(NULL, C_RELOAD);
     x_set_i3_atoms();
     /* Send an IPC event just in case the ws names have changed */
     ipc_send_workspace_event("reload", NULL, NULL);

src/config.c

@@ -39,154 +39,141 @@ void update_barconfig(void) {
     }
 }
 
-/*
+static void free_configuration(void) {
- * Finds the configuration file to use (either the one specified by
+    assert(conn != NULL);
- * override_configpath), the user’s one or the system default) and calls
+
- * parse_file().
+    /* If we are currently in a binding mode, we first revert to the default
- *
+     * since we have no guarantee that the current mode will even still exist
- */
+     * after parsing the config again. See #2228. */
-bool parse_configuration(const char *override_configpath, bool use_nagbar) {
+    switch_mode("default");
-    char *path = get_config_path(override_configpath, true);
+
-    if (path == NULL) {
+    /* First ungrab the keys */
-        die("Unable to find the configuration file (looked at "
+    ungrab_all_keys(conn);
-            "$XDG_CONFIG_HOME/i3/config, ~/.i3/config, $XDG_CONFIG_DIRS/i3/config "
+
-            "and " SYSCONFDIR "/i3/config)");
+    struct Mode *mode;
+    while (!SLIST_EMPTY(&modes)) {
+        mode = SLIST_FIRST(&modes);
+        FREE(mode->name);
+
+        /* Clear the old binding list */
+        while (!TAILQ_EMPTY(mode->bindings)) {
+            Binding *bind = TAILQ_FIRST(mode->bindings);
+            TAILQ_REMOVE(mode->bindings, bind, bindings);
+            binding_free(bind);
+        }
+        FREE(mode->bindings);
+
+        SLIST_REMOVE(&modes, mode, Mode, modes);
+        FREE(mode);
     }
 
-    LOG("Parsing configfile %s\n", path);
+    while (!TAILQ_EMPTY(&assignments)) {
-    FREE(current_configpath);
+        struct Assignment *assign = TAILQ_FIRST(&assignments);
-    current_configpath = path;
+        if (assign->type == A_TO_WORKSPACE || assign->type == A_TO_WORKSPACE_NUMBER)
-
+            FREE(assign->dest.workspace);
-    /* initialize default bindings if we're just validating the config file */
+        else if (assign->type == A_COMMAND)
-    if (!use_nagbar && bindings == NULL) {
+            FREE(assign->dest.command);
-        bindings = scalloc(1, sizeof(struct bindings_head));
+        else if (assign->type == A_TO_OUTPUT)
-        TAILQ_INIT(bindings);
+            FREE(assign->dest.output);
+        match_free(&(assign->match));
+        TAILQ_REMOVE(&assignments, assign, assignments);
+        FREE(assign);
     }
 
-    return parse_file(path, use_nagbar);
+    while (!TAILQ_EMPTY(&ws_assignments)) {
+        struct Workspace_Assignment *assign = TAILQ_FIRST(&ws_assignments);
+        FREE(assign->name);
+        FREE(assign->output);
+        TAILQ_REMOVE(&ws_assignments, assign, ws_assignments);
+        FREE(assign);
+    }
+
+    /* Clear bar configs */
+    Barconfig *barconfig;
+    while (!TAILQ_EMPTY(&barconfigs)) {
+        barconfig = TAILQ_FIRST(&barconfigs);
+        FREE(barconfig->id);
+        for (int c = 0; c < barconfig->num_outputs; c++)
+            free(barconfig->outputs[c]);
+
+        while (!TAILQ_EMPTY(&(barconfig->bar_bindings))) {
+            struct Barbinding *binding = TAILQ_FIRST(&(barconfig->bar_bindings));
+            FREE(binding->command);
+            TAILQ_REMOVE(&(barconfig->bar_bindings), binding, bindings);
+            FREE(binding);
+        }
+
+        while (!TAILQ_EMPTY(&(barconfig->tray_outputs))) {
+            struct tray_output_t *tray_output = TAILQ_FIRST(&(barconfig->tray_outputs));
+            FREE(tray_output->output);
+            TAILQ_REMOVE(&(barconfig->tray_outputs), tray_output, tray_outputs);
+            FREE(tray_output);
+        }
+
+        FREE(barconfig->outputs);
+        FREE(barconfig->socket_path);
+        FREE(barconfig->status_command);
+        FREE(barconfig->i3bar_command);
+        FREE(barconfig->font);
+        FREE(barconfig->colors.background);
+        FREE(barconfig->colors.statusline);
+        FREE(barconfig->colors.separator);
+        FREE(barconfig->colors.focused_background);
+        FREE(barconfig->colors.focused_statusline);
+        FREE(barconfig->colors.focused_separator);
+        FREE(barconfig->colors.focused_workspace_border);
+        FREE(barconfig->colors.focused_workspace_bg);
+        FREE(barconfig->colors.focused_workspace_text);
+        FREE(barconfig->colors.active_workspace_border);
+        FREE(barconfig->colors.active_workspace_bg);
+        FREE(barconfig->colors.active_workspace_text);
+        FREE(barconfig->colors.inactive_workspace_border);
+        FREE(barconfig->colors.inactive_workspace_bg);
+        FREE(barconfig->colors.inactive_workspace_text);
+        FREE(barconfig->colors.urgent_workspace_border);
+        FREE(barconfig->colors.urgent_workspace_bg);
+        FREE(barconfig->colors.urgent_workspace_text);
+        FREE(barconfig->colors.binding_mode_border);
+        FREE(barconfig->colors.binding_mode_bg);
+        FREE(barconfig->colors.binding_mode_text);
+        TAILQ_REMOVE(&barconfigs, barconfig, configs);
+        FREE(barconfig);
+    }
+
+    Con *con;
+    TAILQ_FOREACH(con, &all_cons, all_cons) {
+        /* Assignments changed, previously ran assignments are invalid. */
+        if (con->window) {
+            con->window->nr_assignments = 0;
+            FREE(con->window->ran_assignments);
+        }
+        /* Invalidate pixmap caches in case font or colors changed. */
+        FREE(con->deco_render_params);
+    }
+
+    /* Get rid of the current font */
+    free_font();
+
+    free(config.ipc_socket_path);
+    free(config.restart_state_path);
+    free(config.fake_outputs);
 }
 
 /*
  * (Re-)loads the configuration file (sets useful defaults before).
  *
+ * If you specify override_configpath, only this path is used to look for a
+ * configuration file.
+ *
+ * load_type specifies the type of loading: C_VALIDATE is used to only verify
+ * the correctness of the config file (used with the flag -C). C_LOAD will load
+ * the config for normal use and display errors in the nagbar. C_RELOAD will
+ * also clear the previous config.
+ *
  */
-void load_configuration(xcb_connection_t *conn, const char *override_configpath, bool reload) {
+bool load_configuration(const char *override_configpath, config_load_t load_type) {
-    if (reload) {
+    if (load_type == C_RELOAD) {
-        /* If we are currently in a binding mode, we first revert to the
+        free_configuration();
-         * default since we have no guarantee that the current mode will even
-         * still exist after parsing the config again. See #2228. */
-        switch_mode("default");
-
-        /* First ungrab the keys */
-        ungrab_all_keys(conn);
-
-        struct Mode *mode;
-        while (!SLIST_EMPTY(&modes)) {
-            mode = SLIST_FIRST(&modes);
-            FREE(mode->name);
-
-            /* Clear the old binding list */
-            while (!TAILQ_EMPTY(mode->bindings)) {
-                Binding *bind = TAILQ_FIRST(mode->bindings);
-                TAILQ_REMOVE(mode->bindings, bind, bindings);
-                binding_free(bind);
-            }
-            FREE(mode->bindings);
-
-            SLIST_REMOVE(&modes, mode, Mode, modes);
-            FREE(mode);
-        }
-
-        while (!TAILQ_EMPTY(&assignments)) {
-            struct Assignment *assign = TAILQ_FIRST(&assignments);
-            if (assign->type == A_TO_WORKSPACE || assign->type == A_TO_WORKSPACE_NUMBER)
-                FREE(assign->dest.workspace);
-            else if (assign->type == A_COMMAND)
-                FREE(assign->dest.command);
-            else if (assign->type == A_TO_OUTPUT)
-                FREE(assign->dest.output);
-            match_free(&(assign->match));
-            TAILQ_REMOVE(&assignments, assign, assignments);
-            FREE(assign);
-        }
-
-        while (!TAILQ_EMPTY(&ws_assignments)) {
-            struct Workspace_Assignment *assign = TAILQ_FIRST(&ws_assignments);
-            FREE(assign->name);
-            FREE(assign->output);
-            TAILQ_REMOVE(&ws_assignments, assign, ws_assignments);
-            FREE(assign);
-        }
-
-        /* Clear bar configs */
-        Barconfig *barconfig;
-        while (!TAILQ_EMPTY(&barconfigs)) {
-            barconfig = TAILQ_FIRST(&barconfigs);
-            FREE(barconfig->id);
-            for (int c = 0; c < barconfig->num_outputs; c++)
-                free(barconfig->outputs[c]);
-
-            while (!TAILQ_EMPTY(&(barconfig->bar_bindings))) {
-                struct Barbinding *binding = TAILQ_FIRST(&(barconfig->bar_bindings));
-                FREE(binding->command);
-                TAILQ_REMOVE(&(barconfig->bar_bindings), binding, bindings);
-                FREE(binding);
-            }
-
-            while (!TAILQ_EMPTY(&(barconfig->tray_outputs))) {
-                struct tray_output_t *tray_output = TAILQ_FIRST(&(barconfig->tray_outputs));
-                FREE(tray_output->output);
-                TAILQ_REMOVE(&(barconfig->tray_outputs), tray_output, tray_outputs);
-                FREE(tray_output);
-            }
-
-            FREE(barconfig->outputs);
-            FREE(barconfig->socket_path);
-            FREE(barconfig->status_command);
-            FREE(barconfig->i3bar_command);
-            FREE(barconfig->font);
-            FREE(barconfig->colors.background);
-            FREE(barconfig->colors.statusline);
-            FREE(barconfig->colors.separator);
-            FREE(barconfig->colors.focused_background);
-            FREE(barconfig->colors.focused_statusline);
-            FREE(barconfig->colors.focused_separator);
-            FREE(barconfig->colors.focused_workspace_border);
-            FREE(barconfig->colors.focused_workspace_bg);
-            FREE(barconfig->colors.focused_workspace_text);
-            FREE(barconfig->colors.active_workspace_border);
-            FREE(barconfig->colors.active_workspace_bg);
-            FREE(barconfig->colors.active_workspace_text);
-            FREE(barconfig->colors.inactive_workspace_border);
-            FREE(barconfig->colors.inactive_workspace_bg);
-            FREE(barconfig->colors.inactive_workspace_text);
-            FREE(barconfig->colors.urgent_workspace_border);
-            FREE(barconfig->colors.urgent_workspace_bg);
-            FREE(barconfig->colors.urgent_workspace_text);
-            FREE(barconfig->colors.binding_mode_border);
-            FREE(barconfig->colors.binding_mode_bg);
-            FREE(barconfig->colors.binding_mode_text);
-            TAILQ_REMOVE(&barconfigs, barconfig, configs);
-            FREE(barconfig);
-        }
-
-        Con *con;
-        TAILQ_FOREACH(con, &all_cons, all_cons) {
-            /* Assignments changed, previously ran assignments are invalid. */
-            if (con->window) {
-                con->window->nr_assignments = 0;
-                FREE(con->window->ran_assignments);
-            }
-            /* Invalidate pixmap caches in case font or colors changed. */
-            FREE(con->deco_render_params);
-        }
-
-        /* Get rid of the current font */
-        free_font();
-
-        free(config.ipc_socket_path);
-        free(config.restart_state_path);
-        free(config.fake_outputs);
     }
 
     SLIST_INIT(&modes);
@@ -241,24 +228,32 @@ void load_configuration(xcb_connection_t *conn, const char *override_configpath,
 
     config.focus_wrapping = FOCUS_WRAPPING_ON;
 
-    parse_configuration(override_configpath, true);
+    FREE(current_configpath);
-
+    current_configpath = get_config_path(override_configpath, true);
-    if (reload) {
+    if (current_configpath == NULL) {
-        translate_keysyms();
+        die("Unable to find the configuration file (looked at "
-        grab_all_keys(conn);
+            "$XDG_CONFIG_HOME/i3/config, ~/.i3/config, $XDG_CONFIG_DIRS/i3/config "
-        regrab_all_buttons(conn);
+            "and " SYSCONFDIR "/i3/config)");
     }
+    LOG("Parsing configfile %s\n", current_configpath);
+    const bool result = parse_file(current_configpath, load_type != C_VALIDATE);
 
-    if (config.font.type == FONT_TYPE_NONE) {
+    if (config.font.type == FONT_TYPE_NONE && load_type != C_VALIDATE) {
         ELOG("You did not specify required configuration option \"font\"\n");
         config.font = load_font("fixed", true);
         set_font(&config.font);
     }
 
-    /* Redraw the currently visible decorations on reload, so that
+    if (load_type == C_RELOAD) {
-     * the possibly new drawing parameters changed. */
+        translate_keysyms();
-    if (reload) {
+        grab_all_keys(conn);
+        regrab_all_buttons(conn);
+
+        /* Redraw the currently visible decorations on reload, so that the
+         * possibly new drawing parameters changed. */
         x_deco_recurse(croot);
         xcb_flush(conn);
     }
+
+    return result;
 }

src/main.c

@@ -420,7 +420,7 @@ int main(int argc, char *argv[]) {
     }
 
     if (only_check_config) {
-        exit(parse_configuration(override_configpath, false) ? 0 : 1);
+        exit(load_configuration(override_configpath, C_VALIDATE) ? 0 : 1);
     }
 
     /* If the user passes more arguments, we act like i3-msg would: Just send
@@ -586,7 +586,7 @@ int main(int argc, char *argv[]) {
 #include "atoms.xmacro"
 #undef xmacro
 
-    load_configuration(conn, override_configpath, false);
+    load_configuration(override_configpath, C_LOAD);
 
     if (config.ipc_socket_path == NULL) {
         /* Fall back to a file name in /tmp/ based on the PID */

testcases/t/235-check-config-no-x.t

@@ -59,4 +59,33 @@ EOT
 is($ret, 0, "exit code == 0");
 is($out, "", 'valid config file');
 
+################################################################################
+# 3: test duplicate keybindings
+################################################################################
+
+$cfg = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+bindsym Shift+a nop 1
+bindsym Shift+a nop 2
+EOT
+
+($ret, $out) = check_config($cfg);
+is($ret, 1, "exit code == 1");
+like($out, qr/ERROR: *Duplicate keybinding in config file/, 'duplicate keybindings');
+
+################################################################################
+# 4: test no duplicate keybindings
+################################################################################
+
+$cfg = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+bindsym Shift+a nop 1
+EOT
+
+($ret, $out) = check_config($cfg);
+is($ret, 0, "exit code == 0");
+is($out, "", 'valid config file');
+
 done_testing;