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
067fe9d047 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

@@ -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.
  *

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);
+			char **argv = split_args(exec, &argc);
-			if (strcmp(argv[0], "exec") != 0) {
+			if (!argv) {
-				int i;
+				if (results) {
-				for (i = 1; i < argc; ++i) {
+					free_cmd_results(results);
-					if (*argv[i] == '\"' || *argv[i] == '\'') {
-						strip_quotes(argv[i]);
-					}
 				}
+				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");
+				results = cmd_results_new(CMD_INVALID, cmd,
-				free_argv(argc, argv);
+						"Unknown/invalid command");
 				goto cleanup;
 			}
 
-			// Var replacement, for all but first argument of set
+			// Process command arguments
-			for (int i = handler->handle == cmd_set ? 2 : 1; i < argc; ++i) {
+			argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1;
-				argv[i] = do_var_replacement(argv[i]);
-				unescape_string(argv[i]);
-			}
 
 			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
+	argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1;
-	// 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]);
-	}
 	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
+	argc = preprocess_arg(handler->handle, argv + 1, argc - 1) + 1;
-	if (argc > 1 && (*argv[1] == '\"' || *argv[1] == '\'')) {
+
-		strip_quotes(argv[1]);
-	}
 	if (handler->handle) {
 		return handler->handle(argc - 1, argv + 1);
 	}

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 "
+			"Invalid %s command (expected exactly one non-option "
-			"(expected at least 2 non-option arguments, got %d)", bindtype, argc);
+			"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) {

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);

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);