From 08cdc3a6ae56623c7604af690c3fd2ef238d9ffa Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 29 Mar 2019 02:36:24 +0200 Subject: [PATCH] Allow checking for duplicate bindings with -C - Having both parse_configuration and parse_file is excessive now - We detect if we are parsing only by checking if conn is NULL, not with use_nagbar - font.pattern needs to be set to NULL because it is freed in free_font() Fixes #3660 --- include/configuration.h | 24 +++++------ libi3/font.c | 1 + src/commands.c | 2 +- src/config.c | 64 +++++++++++------------------ src/main.c | 4 +- testcases/t/235-check-config-no-x.t | 29 +++++++++++++ 6 files changed, 68 insertions(+), 56 deletions(-) diff --git a/include/configuration.h b/include/configuration.h index 1971146b..872f11c8 100644 --- a/include/configuration.h +++ b/include/configuration.h @@ -400,19 +400,11 @@ struct tray_output_t { tray_outputs; }; -/** - * Finds the configuration file to use (either the one specified by - * override_configpath), the user’s one or the system default) and calls - * parse_file(). - * - * 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); +typedef enum { + C_VALIDATE, + C_LOAD, + C_RELOAD, +} config_load_t; /** * (Re-)loads the configuration file (sets useful defaults before). @@ -420,8 +412,12 @@ bool parse_configuration(const char *override_configpath, bool use_nagbar); * 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(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 diff --git a/libi3/font.c b/libi3/font.c index c06bae00..e16ce85e 100644 --- a/libi3/font.c +++ b/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. */ diff --git a/src/commands.c b/src/commands.c index 69015d92..624c27db 100644 --- a/src/commands.c +++ b/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(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); diff --git a/src/config.c b/src/config.c index e0eb5378..81919566 100644 --- a/src/config.c +++ b/src/config.c @@ -159,42 +159,20 @@ static void free_configuration(void) { free(config.fake_outputs); } -/* - * Finds the configuration file to use (either the one specified by - * override_configpath), the user’s one or the system default) and calls - * parse_file(). - * - */ -bool parse_configuration(const char *override_configpath, bool use_nagbar) { - char *path = get_config_path(override_configpath, true); - if (path == NULL) { - die("Unable to find the configuration file (looked at " - "$XDG_CONFIG_HOME/i3/config, ~/.i3/config, $XDG_CONFIG_DIRS/i3/config " - "and " SYSCONFDIR "/i3/config)"); - } - - LOG("Parsing configfile %s\n", path); - FREE(current_configpath); - current_configpath = path; - - /* initialize default bindings if we're just validating the config file */ - if (!use_nagbar && bindings == NULL) { - bindings = scalloc(1, sizeof(struct bindings_head)); - TAILQ_INIT(bindings); - } - - return parse_file(path, use_nagbar); -} - /* * (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(const char *override_configpath, bool reload) { - if (reload) { +bool load_configuration(const char *override_configpath, config_load_t load_type) { + if (load_type == C_RELOAD) { free_configuration(); } @@ -250,24 +228,32 @@ void load_configuration(const char *override_configpath, bool reload) { config.focus_wrapping = FOCUS_WRAPPING_ON; - parse_configuration(override_configpath, true); - - if (reload) { - translate_keysyms(); - grab_all_keys(conn); - regrab_all_buttons(conn); + FREE(current_configpath); + current_configpath = get_config_path(override_configpath, true); + if (current_configpath == NULL) { + die("Unable to find the configuration file (looked at " + "$XDG_CONFIG_HOME/i3/config, ~/.i3/config, $XDG_CONFIG_DIRS/i3/config " + "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 - * the possibly new drawing parameters changed. */ - if (reload) { + if (load_type == C_RELOAD) { + translate_keysyms(); + 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; } diff --git a/src/main.c b/src/main.c index d7d7530a..3ebdbf0c 100644 --- a/src/main.c +++ b/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(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 */ diff --git a/testcases/t/235-check-config-no-x.t b/testcases/t/235-check-config-no-x.t index ef621142..dce70894 100644 --- a/testcases/t/235-check-config-no-x.t +++ b/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 = <