From 01757979de2d0ddbcc040732b6f170e5876a08c0 Mon Sep 17 00:00:00 2001 From: mstoeckl Date: Sat, 25 Aug 2018 19:33:28 -0400 Subject: [PATCH] Simplify runtime variable substitution The operations bindsym, bindcode, and for_window extract a new (nested) command to be run from their last arguments. The argc/argv interface for commands hides a measure of preprocessing (removing quotes, unescaping characters, and performing variable substitution). Before this commit, this preprocessing was applied twice for the nested commands. To provide variable substitution for nested commands, commit 067fe9d0472ca24328a962 introduced a double-escaping method (simplifying $$x to $x in the first preprocessing pass, and $x to the appropriate value in the second. This commit ensures that the nested commands are only preprocessed once, so that the double-escape trick is no longer required. The preprocessing code in execute_command, config_command, and config_subcommand has been moved to a new function, preprocess_arg, which performs quote stripping, unescaping, and variable expansion as necessary, while preserving special cases for cmd_set and cmd_exec, and adding new special cases for cmd_for_window, cmd_bindsym, and cmd_bindcode, in which the trailing portion of their argument lists are passed on unmodified as the single final argument. --- include/sway/commands.h | 2 +- sway/commands.c | 148 +++++++++++++++++++++++++++---------- sway/commands/bind.c | 16 ++-- sway/commands/for_window.c | 12 ++- sway/config.c | 10 +-- 5 files changed, 131 insertions(+), 57 deletions(-) diff --git a/include/sway/commands.h b/include/sway/commands.h index 8e91c158f..be9dc276c 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -51,7 +51,7 @@ struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, /** * Parse and executes a command. */ -struct cmd_results *execute_command(char *command, struct sway_seat *seat); +struct cmd_results *execute_command(const char *command, struct sway_seat *seat); /** * Parse and handles a command during config file loading. * diff --git a/sway/commands.c b/sway/commands.c index d9c54adc4..6755b2448 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -209,15 +209,93 @@ struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, return res; } -struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { +/** + * A command is followed by its list of arguments. For instance, "floating" and + * ["toggle"]. This function is passed that list, in split_args format, and then + * strips quotes, unescapes the content, and expands variables. Some commands + * require slightly different preprocessing. (Variable expansion happens + * after unescaping so we don't double-unescape variable values.) + * + * The function returns the new (possibly smaller) value of argc, and if so + * zeros out the trailing values of argv. + */ +static int preprocess_arg(sway_cmd handle, char **argv, int argc) { + if (handle == cmd_exec) { + // Quotes are not stripped + for (int i = 0; i < argc; ++i) { + unescape_string(argv[i]); + argv[i] = do_var_replacement(argv[i]); + } + } else if (handle == cmd_for_window) { + // After first argument, merge remainder without preprocessing + if (*argv[0] == '\"' || *argv[0] == '\'') { + strip_quotes(argv[0]); + } + unescape_string(argv[0]); + argv[0] = do_var_replacement(argv[0]); + if (argc >= 2) { + // Compress the last arguments to one. + char *joined = join_args(argv + 1, argc - 1); + argv[1] = joined; + while (argc > 2) { + argc--; + free(argv[argc]); + argv[argc] = NULL; + } + } + } else if (handle == cmd_bindcode || handle == cmd_bindsym) { + // After the argument after the last '--X' option, merge + // remaining arguments without preprocessing. + int i = 0; + for (; i < argc; i++) { + if (*argv[i] == '\"' || *argv[i] == '\'') { + strip_quotes(argv[i]); + } + unescape_string(argv[i]); + argv[i] = do_var_replacement(argv[i]); + if (argv[i][0] != '-' || argv[i][1] != '-') { + break; + } + } + if (i < argc) { + if (*argv[i] == '\"' || *argv[i] == '\'') { + strip_quotes(argv[i]); + } + unescape_string(argv[i]); + argv[i] = do_var_replacement(argv[i]); + } + if (i + 1 < argc) { + char *joined = join_args(argv + i + 1, argc - i - 1); + argv[i + 1] = joined; + while (argc > i + 2) { + argc--; + free(argv[argc]); + argv[argc] = NULL; + } + } + } else { + // Default case: strip quotes, unescape, and expand variables + // When setting a variable, the first argument is ignored. + int start = handle == cmd_set ? 1 : 0; + for (int i = start; i < argc; ++i) { + if (*argv[i] == '\"' || *argv[i] == '\'') { + strip_quotes(argv[i]); + } + unescape_string(argv[i]); + argv[i] = do_var_replacement(argv[i]); + } + } + + return argc; +} + +struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { // Even though this function will process multiple commands we will only // return the last error, if any (for now). (Since we have access to an // error string we could e.g. concatenate all errors there.) struct cmd_results *results = NULL; char *exec = strdup(_exec); char *head = exec; - char *cmdlist; - char *cmd; list_t *views = NULL; if (seat == NULL) { @@ -251,43 +329,42 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { head += strspn(head, whitespace); } // Split command list - cmdlist = argsep(&head, ";"); + char *cmdlist = argsep(&head, ";"); cmdlist += strspn(cmdlist, whitespace); do { // Split commands - cmd = argsep(&cmdlist, ","); + char *cmd = argsep(&cmdlist, ","); cmd += strspn(cmd, whitespace); if (strcmp(cmd, "") == 0) { wlr_log(WLR_INFO, "Ignoring empty command."); continue; } wlr_log(WLR_INFO, "Handling command '%s'", cmd); - //TODO better handling of argv + + // Identify which command to execute. int argc; - char **argv = split_args(cmd, &argc); - if (strcmp(argv[0], "exec") != 0) { - int i; - for (i = 1; i < argc; ++i) { - if (*argv[i] == '\"' || *argv[i] == '\'') { - strip_quotes(argv[i]); - } + char **argv = split_args(exec, &argc); + if (!argv) { + if (results) { + free_cmd_results(results); } + results = cmd_results_new(CMD_INVALID, cmd, + "Allocation failure"); + goto cleanup; } + struct cmd_handler *handler = find_handler(argv[0], NULL, 0); if (!handler) { if (results) { free_cmd_results(results); } - results = cmd_results_new(CMD_INVALID, cmd, "Unknown/invalid command"); - free_argv(argc, argv); + results = cmd_results_new(CMD_INVALID, cmd, + "Unknown/invalid command"); goto cleanup; } - // Var replacement, for all but first argument of set - for (int i = handler->handle == cmd_set ? 2 : 1; i < argc; ++i) { - argv[i] = do_var_replacement(argv[i]); - unescape_string(argv[i]); - } + // Process command arguments + argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1; if (!config->handler_context.using_criteria) { // without criteria, the command acts upon the focused @@ -298,7 +375,7 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { "could not get focus-inactive for root container")) { return NULL; } - struct cmd_results *res = handler->handle(argc-1, argv+1); + struct cmd_results *res = handler->handle(argc - 1, argv + 1); if (res->status != CMD_SUCCESS) { free_argv(argc, argv); if (results) { @@ -312,7 +389,7 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { for (int i = 0; i < views->length; ++i) { struct sway_view *view = views->items[i]; config->handler_context.current_container = view->swayc; - struct cmd_results *res = handler->handle(argc-1, argv+1); + struct cmd_results *res = handler->handle(argc - 1, argv + 1); if (res->status != CMD_SUCCESS) { free_argv(argc, argv); if (results) { @@ -324,7 +401,9 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { free_cmd_results(res); } } - free_argv(argc, argv); + if (argc) { + free_argv(argc, argv); + } } while(cmdlist); } while(head); cleanup: @@ -372,18 +451,9 @@ struct cmd_results *config_command(char *exec) { results = cmd_results_new(CMD_INVALID, input, "Unknown/invalid command"); goto cleanup; } - int i; - // Var replacement, for all but first argument of set - // TODO commands - for (i = handler->handle == cmd_set ? 2 : 1; i < argc; ++i) { - argv[i] = do_var_replacement(argv[i]); - unescape_string(argv[i]); - } - // Strip quotes for first argument. - // TODO This part needs to be handled much better - if (argc>1 && (*argv[1] == '\"' || *argv[1] == '\'')) { - strip_quotes(argv[1]); - } + + argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1; + if (handler->handle) { results = handler->handle(argc-1, argv+1); } else { @@ -407,11 +477,9 @@ struct cmd_results *config_subcommand(char **argv, int argc, char *input = argv[0] ? argv[0] : "(empty)"; return cmd_results_new(CMD_INVALID, input, "Unknown/invalid command"); } - // Strip quotes for first argument. - // TODO This part needs to be handled much better - if (argc > 1 && (*argv[1] == '\"' || *argv[1] == '\'')) { - strip_quotes(argv[1]); - } + + argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1; + if (handler->handle) { return handler->handle(argc - 1, argv + 1); } diff --git a/sway/commands/bind.c b/sway/commands/bind.c index 8270b958d..e984e8c0f 100644 --- a/sway/commands/bind.c +++ b/sway/commands/bind.c @@ -206,14 +206,20 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, binding->type = BINDING_MOUSE; } - if (argc < 2) { + // the arguments to bindsym and bindcode are specially preprocessed, + // so that the command to be executed is provided as the final argument. + if (argc != 2) { free_sway_binding(binding); return cmd_results_new(CMD_FAILURE, bindtype, - "Invalid %s command " - "(expected at least 2 non-option arguments, got %d)", bindtype, argc); + "Invalid %s command (expected exactly one non-option " + "argument, followed by a command)", bindtype); + } + binding->command = strdup(argv[1]); + if (!binding->command) { + free_sway_binding(binding); + return cmd_results_new(CMD_FAILURE, bindtype, + "Unable to allocate a copy of the command"); } - - binding->command = join_args(argv + 1, argc - 1); list_t *split = split_string(argv[0], "+"); for (int i = 0; i < split->length; ++i) { diff --git a/sway/commands/for_window.c b/sway/commands/for_window.c index ac4d6563a..3f292587c 100644 --- a/sway/commands/for_window.c +++ b/sway/commands/for_window.c @@ -8,10 +8,18 @@ struct cmd_results *cmd_for_window(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "for_window", EXPECTED_AT_LEAST, 2))) { + if ((error = checkarg(argc, "for_window", EXPECTED_EQUAL_TO, 2))) { return error; } + // the arguments to cmd_for_window are specially preprocessed, + // so that the command to be executed is provided as the final argument. + char *cmdlist = strdup(argv[1]); + if (!cmdlist) { + return cmd_results_new(CMD_FAILURE, "for_window", + "Unable to allocate a copy of the command"); + } + char *err_str = NULL; struct criteria *criteria = criteria_parse(argv[0], &err_str); if (!criteria) { @@ -21,7 +29,7 @@ struct cmd_results *cmd_for_window(int argc, char **argv) { } criteria->type = CT_COMMAND; - criteria->cmdlist = join_args(argv + 1, argc - 1); + criteria->cmdlist = cmdlist; list_add(config->criteria, criteria); wlr_log(WLR_DEBUG, "for_window: '%s' -> '%s' added", criteria->raw, criteria->cmdlist); diff --git a/sway/config.c b/sway/config.c index 642abbac8..ad705ad8f 100644 --- a/sway/config.c +++ b/sway/config.c @@ -748,7 +748,6 @@ bool read_config(FILE *file, struct sway_config *config, } char *do_var_replacement(char *str) { - int i; char *find = str; while ((find = strchr(find, '$'))) { // Skip if escaped. @@ -758,15 +757,8 @@ char *do_var_replacement(char *str) { continue; } } - // Unescape double $ and move on - if (find[1] == '$') { - size_t length = strlen(find + 1); - memmove(find, find + 1, length); - find[length] = '\0'; - ++find; - continue; - } // Find matching variable + int i; for (i = 0; i < config->symbols->length; ++i) { struct sway_variable *var = config->symbols->items[i]; int vnlen = strlen(var->name);