Merge pull request #1540 from emersion/output-command-refactor

Refactor output command, add output enable

include/sway/config.h

@@ -397,7 +397,7 @@ struct seat_attachment_config *seat_config_get_attachment(
 void apply_seat_config(struct seat_config *seat);
 
 int output_name_cmp(const void *item, const void *data);
-struct output_config *new_output_config();
+struct output_config *new_output_config(const char *name);
 void merge_output_config(struct output_config *dst, struct output_config *src);
 void apply_output_config(struct output_config *oc, swayc_t *output);
 void free_output_config(struct output_config *oc);

sway/commands/output.c

@@ -20,143 +20,111 @@ static char *bg_options[] = {
 	"tile",
 };
 
-struct cmd_results *cmd_output(int argc, char **argv) {
+static struct cmd_results *cmd_output_mode(struct output_config *output,
-	struct cmd_results *error = NULL;
+		int *i, int argc, char **argv) {
-	if ((error = checkarg(argc, "output", EXPECTED_AT_LEAST, 1))) {
+	if (++*i >= argc) {
-		return error;
+		return cmd_results_new(CMD_INVALID, "output", "Missing mode argument.");
 	}
-	const char *name = argv[0];
-
-	struct output_config *output = new_output_config();
-	if (!output) {
-		sway_log(L_ERROR, "Failed to allocate output config");
-		return NULL;
-	}
-	output->name = strdup(name);
-
-	int i;
-	for (i = 1; i < argc; ++i) {
-		const char *command = argv[i];
-
-		if (strcasecmp(command, "disable") == 0) {
-			output->enabled = 0;
-		} else if (strcasecmp(command, "mode") == 0 ||
-				strcasecmp(command, "resolution") == 0 ||
-				strcasecmp(command, "res") == 0) {
-			if (++i >= argc) {
-				error = cmd_results_new(CMD_INVALID, "output",
-					"Missing mode argument.");
-				goto fail;
-			}
-
-			int width = -1, height = -1;
-			float refresh_rate = -1;
 
 	char *end;
-			width = strtol(argv[i], &end, 10);
+	output->width = strtol(argv[*i], &end, 10);
 	if (*end) {
 		// Format is 1234x4321
 		if (*end != 'x') {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid mode width.");
-					goto fail;
 		}
 		++end;
-				height = strtol(end, &end, 10);
+		output->height = strtol(end, &end, 10);
 		if (*end) {
 			if (*end != '@') {
-						error = cmd_results_new(CMD_INVALID, "output",
+				return cmd_results_new(CMD_INVALID, "output",
 					"Invalid mode height.");
-						goto fail;
 			}
 			++end;
-					refresh_rate = strtof(end, &end);
+			output->refresh_rate = strtof(end, &end);
 			if (strcasecmp("Hz", end) != 0) {
-						error = cmd_results_new(CMD_INVALID, "output",
+				return cmd_results_new(CMD_INVALID, "output",
 					"Invalid mode refresh rate.");
-						goto fail;
 			}
 		}
 	} else {
 		// Format is 1234 4321
-				if (++i >= argc) {
+		if (++*i >= argc) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Missing mode argument (height).");
-					goto fail;
 		}
-				height = strtol(argv[i], &end, 10);
+		output->height = strtol(argv[*i], &end, 10);
 		if (*end) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid mode height.");
-					goto fail;
 		}
 	}
-			output->width = width;
-			output->height = height;
-			output->refresh_rate = refresh_rate;
-		} else if (strcasecmp(command, "position") == 0 ||
-				strcasecmp(command, "pos") == 0) {
-			if (++i >= argc) {
-				error = cmd_results_new(CMD_INVALID, "output",
-					"Missing position argument.");
-				goto fail;
-			}
 
-			int x = -1, y = -1;
+	return NULL;
+}
+
+static struct cmd_results *cmd_output_position(struct output_config *output,
+		int *i, int argc, char **argv) {
+	if (++*i >= argc) {
+		return cmd_results_new(CMD_INVALID, "output",
+			"Missing position argument.");
+	}
 
 	char *end;
-			x = strtol(argv[i], &end, 10);
+	output->x = strtol(argv[*i], &end, 10);
 	if (*end) {
 		// Format is 1234,4321
 		if (*end != ',') {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid position x.");
-					goto fail;
 		}
 		++end;
-				y = strtol(end, &end, 10);
+		output->y = strtol(end, &end, 10);
 		if (*end) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid position y.");
-					goto fail;
 		}
 	} else {
 		// Format is 1234 4321 (legacy)
-				if (++i >= argc) {
+		if (++*i >= argc) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Missing position argument (y).");
-					goto fail;
 		}
-				y = strtol(argv[i], &end, 10);
+		output->y = strtol(argv[*i], &end, 10);
 		if (*end) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid position y.");
-					goto fail;
 		}
 	}
 
-			output->x = x;
+	return NULL;
-			output->y = y;
+}
-		} else if (strcasecmp(command, "scale") == 0) {
+
-			if (++i >= argc) {
+static struct cmd_results *cmd_output_scale(struct output_config *output,
-				error = cmd_results_new(CMD_INVALID, "output",
+		int *i, int argc, char **argv) {
-					"Missing scale parameter.");
+	if (++*i >= argc) {
-				goto fail;
+		return cmd_results_new(CMD_INVALID, "output",
+			"Missing scale argument.");
 	}
+
 	char *end;
-			output->scale = strtof(argv[i], &end);
+	output->scale = strtof(argv[*i], &end);
 	if (*end) {
-				error = cmd_results_new(CMD_INVALID, "output",
+		return cmd_results_new(CMD_INVALID, "output", "Invalid scale.");
-					"Invalid scale.");
-				goto fail;
 	}
-		} else if (strcasecmp(command, "transform") == 0) {
+
-			if (++i >= argc) {
+	return NULL;
-				error = cmd_results_new(CMD_INVALID, "output",
+}
-					"Missing transform parameter.");
+
-				goto fail;
+static struct cmd_results *cmd_output_transform(struct output_config *output,
+		int *i, int argc, char **argv) {
+	if (++*i >= argc) {
+		return cmd_results_new(CMD_INVALID, "output",
+			"Missing transform argument.");
 	}
-			char *value = argv[i];
+
+	char *value = argv[*i];
 	if (strcmp(value, "normal") == 0) {
 		output->transform = WL_OUTPUT_TRANSFORM_NORMAL;
 	} else if (strcmp(value, "90") == 0) {
@@ -174,33 +142,35 @@ struct cmd_results *cmd_output(int argc, char **argv) {
 	} else if (strcmp(value, "flipped-270") == 0) {
 		output->transform = WL_OUTPUT_TRANSFORM_FLIPPED_270;
 	} else {
-				error = cmd_results_new(CMD_INVALID, "output",
+		return cmd_results_new(CMD_INVALID, "output",
 			"Invalid output transform.");
-				goto fail;
 	}
-		} else if (strcasecmp(command, "background") == 0 ||
+
-				strcasecmp(command, "bg") == 0) {
+	return NULL;
-			wordexp_t p;
+}
-			if (++i >= argc) {
+
-				error = cmd_results_new(CMD_INVALID, "output",
+static struct cmd_results *cmd_output_background(struct output_config *output,
+		int *i, int argc, char **argv) {
+	if (++*i >= argc) {
+		return cmd_results_new(CMD_INVALID, "output",
 			"Missing background file or color specification.");
-				goto fail;
 	}
-			if (i + 1 >= argc) {
+	const char *background = argv[*i];
-				error = cmd_results_new(CMD_INVALID, "output",
+	if (*i + 1 >= argc) {
+		return cmd_results_new(CMD_INVALID, "output",
 			"Missing background scaling mode or `solid_color`.");
-				goto fail;
 	}
-			if (strcasecmp(argv[i + 1], "solid_color") == 0) {
+	const char *background_option = argv[*i];
-				output->background = strdup(argv[argc - 2]);
+
+	if (strcasecmp(background_option, "solid_color") == 0) {
+		output->background = strdup(background);
 		output->background_option = strdup("solid_color");
 	} else {
-				// argv[i+j]=bg_option
 		bool valid = false;
 		char *mode;
 		size_t j;
-				for (j = 0; j < (size_t) (argc - i); ++j) {
+		for (j = 0; j < (size_t)(argc - *i); ++j) {
-					mode = argv[i + j];
+			mode = argv[*i + j];
 			size_t n = sizeof(bg_options) / sizeof(char *);
 			for (size_t k = 0; k < n; ++k) {
 				if (strcasecmp(mode, bg_options[k]) == 0) {
@@ -213,16 +183,15 @@ struct cmd_results *cmd_output(int argc, char **argv) {
 			}
 		}
 		if (!valid) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Missing background scaling mode.");
-					goto fail;
 		}
 
-				char *src = join_args(argv + i, j);
+		wordexp_t p;
+		char *src = join_args(argv + *i - 1, j);
 		if (wordexp(src, &p, 0) != 0 || p.we_wordv[0] == NULL) {
-					error = cmd_results_new(CMD_INVALID, "output",
+			return cmd_results_new(CMD_INVALID, "output",
 				"Invalid syntax (%s).", src);
-					goto fail;
 		}
 		free(src);
 		src = p.we_wordv[0];
@@ -239,15 +208,13 @@ struct cmd_results *cmd_output(int argc, char **argv) {
 				}
 				free(conf);
 			} else {
-						sway_log(L_ERROR,
+				sway_log(L_ERROR, "Unable to allocate background source");
-							"Unable to allocate background source");
 			}
 		}
 		if (!src || access(src, F_OK) == -1) {
-					error = cmd_results_new(CMD_INVALID, "output",
-						"Background file unreadable (%s).", src);
 			wordfree(&p);
-					goto fail;
+			return cmd_results_new(CMD_INVALID, "output",
+				"Background file unreadable (%s).", src);
 		}
 
 		output->background = strdup(src);
@@ -257,16 +224,56 @@ struct cmd_results *cmd_output(int argc, char **argv) {
 		}
 		wordfree(&p);
 
-				i += j;
+		*i += j;
 	}
+
+	return NULL;
+}
+
+struct cmd_results *cmd_output(int argc, char **argv) {
+	struct cmd_results *error = NULL;
+	if ((error = checkarg(argc, "output", EXPECTED_AT_LEAST, 1))) {
+		return error;
+	}
+
+	struct output_config *output = new_output_config(argv[0]);
+	if (!output) {
+		sway_log(L_ERROR, "Failed to allocate output config");
+		return NULL;
+	}
+
+	for (int i = 1; i < argc; ++i) {
+		const char *command = argv[i];
+
+		if (strcasecmp(command, "enable") == 0) {
+			output->enabled = 1;
+		} else if (strcasecmp(command, "disable") == 0) {
+			output->enabled = 0;
+		} else if (strcasecmp(command, "mode") == 0 ||
+				strcasecmp(command, "resolution") == 0 ||
+				strcasecmp(command, "res") == 0) {
+			error = cmd_output_mode(output, &i, argc, argv);
+		} else if (strcasecmp(command, "position") == 0 ||
+				strcasecmp(command, "pos") == 0) {
+			error = cmd_output_position(output, &i, argc, argv);
+		} else if (strcasecmp(command, "scale") == 0) {
+			error = cmd_output_scale(output, &i, argc, argv);
+		} else if (strcasecmp(command, "transform") == 0) {
+			error = cmd_output_transform(output, &i, argc, argv);
+		} else if (strcasecmp(command, "background") == 0 ||
+				strcasecmp(command, "bg") == 0) {
+			error = cmd_output_background(output, &i, argc, argv);
 		} else {
 			error = cmd_results_new(CMD_INVALID, "output",
 				"Invalid output subcommand: %s.", command);
+		}
+
+		if (error != NULL) {
 			goto fail;
 		}
 	}
 
-	i = list_seq_find(config->output_configs, output_name_cmp, name);
+	int i = list_seq_find(config->output_configs, output_name_cmp, output->name);
 	if (i >= 0) {
 		// merge existing config
 		struct output_config *oc = config->output_configs->items[i];

sway/config/output.c

@@ -14,11 +14,16 @@ int output_name_cmp(const void *item, const void *data) {
 	return strcmp(output->name, name);
 }
 
-struct output_config *new_output_config() {
+struct output_config *new_output_config(const char *name) {
 	struct output_config *oc = calloc(1, sizeof(struct output_config));
 	if (oc == NULL) {
 		return NULL;
 	}
+	oc->name = strdup(name);
+	if (oc->name == NULL) {
+		free(oc);
+		return NULL;
+	}
 	oc->enabled = -1;
 	oc->width = oc->height = -1;
 	oc->refresh_rate = -1;