From a9fdd5dd2adf1f09ff9df555e7342828abf1a654 Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Tue, 23 Oct 2018 10:12:32 +0100 Subject: [PATCH 1/4] commands: print correct command on error for exec and opacity --- sway/commands/exec_always.c | 8 ++++---- sway/commands/opacity.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c index de78dd83..43b35dd7 100644 --- a/sway/commands/exec_always.c +++ b/sway/commands/exec_always.c @@ -16,7 +16,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { struct cmd_results *error = NULL; if (!config->active) return cmd_results_new(CMD_DEFER, NULL, NULL); - if ((error = checkarg(argc, "exec_always", EXPECTED_MORE_THAN, 0))) { + if ((error = checkarg(argc, argv[-1], EXPECTED_MORE_THAN, 0))) { return error; } @@ -24,7 +24,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { if (strcmp(argv[0], "--no-startup-id") == 0) { wlr_log(WLR_INFO, "exec switch '--no-startup-id' not supported, ignored."); --argc; ++argv; - if ((error = checkarg(argc, "exec_always", EXPECTED_MORE_THAN, 0))) { + if ((error = checkarg(argc, argv[-1], EXPECTED_MORE_THAN, 0))) { return error; } } @@ -71,7 +71,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { } else if (pid < 0) { close(fd[0]); close(fd[1]); - return cmd_results_new(CMD_FAILURE, "exec_always", "fork() failed"); + return cmd_results_new(CMD_FAILURE, argv[-1], "fork() failed"); } close(fd[1]); // close write ssize_t s = 0; @@ -85,7 +85,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { wlr_log(WLR_DEBUG, "Child process created with pid %d", child); root_record_workspace_pid(child); } else { - return cmd_results_new(CMD_FAILURE, "exec_always", + return cmd_results_new(CMD_FAILURE, argv[-1], "Second fork() failed"); } diff --git a/sway/commands/opacity.c b/sway/commands/opacity.c index 4e4fc994..8c45b528 100644 --- a/sway/commands/opacity.c +++ b/sway/commands/opacity.c @@ -15,7 +15,7 @@ static bool parse_opacity(const char *opacity, float *val) { struct cmd_results *cmd_opacity(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "layout", EXPECTED_EQUAL_TO, 1))) { + if ((error = checkarg(argc, "opacity", EXPECTED_EQUAL_TO, 1))) { return error; } From 5364255f265da2e324f65369037d5828688d08cd Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Tue, 23 Oct 2018 10:17:58 +0100 Subject: [PATCH 2/4] commands: remove EXPECTED_MORE_THAN Its uses have been replaced with EXPECTED_AT_LEAST. --- include/sway/commands.h | 1 - sway/commands.c | 8 -------- sway/commands/bar/bindsym.c | 2 +- sway/commands/bind.c | 2 +- sway/commands/exec_always.c | 4 ++-- sway/commands/layout.c | 2 +- 6 files changed, 5 insertions(+), 14 deletions(-) diff --git a/include/sway/commands.h b/include/sway/commands.h index 2f6d31b1..791b37dc 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -39,7 +39,6 @@ struct cmd_results { }; enum expected_args { - EXPECTED_MORE_THAN, EXPECTED_AT_LEAST, EXPECTED_LESS_THAN, EXPECTED_EQUAL_TO diff --git a/sway/commands.c b/sway/commands.c index 3f416afc..4c1b34d5 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -20,14 +20,6 @@ struct cmd_results *checkarg(int argc, const char *name, enum expected_args type, int val) { struct cmd_results *error = NULL; switch (type) { - case EXPECTED_MORE_THAN: - if (argc > val) { - return NULL; - } - error = cmd_results_new(CMD_INVALID, name, "Invalid %s command " - "(expected more than %d argument%s, got %d)", - name, val, (char*[2]){"s", ""}[argc==1], argc); - break; case EXPECTED_AT_LEAST: if (argc >= val) { return NULL; diff --git a/sway/commands/bar/bindsym.c b/sway/commands/bar/bindsym.c index 4eea3e6a..965c8903 100644 --- a/sway/commands/bar/bindsym.c +++ b/sway/commands/bar/bindsym.c @@ -10,7 +10,7 @@ struct cmd_results *bar_cmd_bindsym(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "bar bindsym", EXPECTED_MORE_THAN, 1))) { + if ((error = checkarg(argc, "bar bindsym", EXPECTED_AT_LEAST, 2))) { return error; } if (!config->current_bar) { diff --git a/sway/commands/bind.c b/sway/commands/bind.c index 5832d01e..a9de227f 100644 --- a/sway/commands/bind.c +++ b/sway/commands/bind.c @@ -145,7 +145,7 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, const char *bindtype = bindcode ? "bindcode" : "bindsym"; struct cmd_results *error = NULL; - if ((error = checkarg(argc, bindtype, EXPECTED_MORE_THAN, 1))) { + if ((error = checkarg(argc, bindtype, EXPECTED_AT_LEAST, 2))) { return error; } diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c index 43b35dd7..8bdeceeb 100644 --- a/sway/commands/exec_always.c +++ b/sway/commands/exec_always.c @@ -16,7 +16,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { struct cmd_results *error = NULL; if (!config->active) return cmd_results_new(CMD_DEFER, NULL, NULL); - if ((error = checkarg(argc, argv[-1], EXPECTED_MORE_THAN, 0))) { + if ((error = checkarg(argc, argv[-1], EXPECTED_AT_LEAST, 1))) { return error; } @@ -24,7 +24,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { if (strcmp(argv[0], "--no-startup-id") == 0) { wlr_log(WLR_INFO, "exec switch '--no-startup-id' not supported, ignored."); --argc; ++argv; - if ((error = checkarg(argc, argv[-1], EXPECTED_MORE_THAN, 0))) { + if ((error = checkarg(argc, argv[-1], EXPECTED_AT_LEAST, 1))) { return error; } } diff --git a/sway/commands/layout.c b/sway/commands/layout.c index c2ce2e78..65f67af8 100644 --- a/sway/commands/layout.c +++ b/sway/commands/layout.c @@ -96,7 +96,7 @@ static enum sway_container_layout get_layout(int argc, char **argv, struct cmd_results *cmd_layout(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "layout", EXPECTED_MORE_THAN, 0))) { + if ((error = checkarg(argc, "layout", EXPECTED_AT_LEAST, 1))) { return error; } struct sway_container *container = config->handler_context.container; From 000d96e52539fd6b8ecf6f6d0d399a1bedbbb9d9 Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Tue, 23 Oct 2018 10:32:05 +0100 Subject: [PATCH 3/4] commands: clean-up checkarg function Consolidates logic and fixes mistake that used argc instead of val for determining plural. --- sway/commands.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/sway/commands.c b/sway/commands.c index 4c1b34d5..6a5eb3dd 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -18,33 +18,28 @@ // Returns error object, or NULL if check succeeds. struct cmd_results *checkarg(int argc, const char *name, enum expected_args type, int val) { - struct cmd_results *error = NULL; + const char *error_name = NULL; switch (type) { case EXPECTED_AT_LEAST: - if (argc >= val) { - return NULL; + if (argc < val) { + error_name = "at least "; } - error = cmd_results_new(CMD_INVALID, name, "Invalid %s command " - "(expected at least %d argument%s, got %d)", - name, val, (char*[2]){"s", ""}[argc==1], argc); break; case EXPECTED_LESS_THAN: - if (argc < val) { - return NULL; - }; - error = cmd_results_new(CMD_INVALID, name, "Invalid %s command " - "(expected less than %d argument%s, got %d)", - name, val, (char*[2]){"s", ""}[argc==1], argc); + if (argc >= val) { + error_name = "less than "; + } break; case EXPECTED_EQUAL_TO: - if (argc == val) { - return NULL; - }; - error = cmd_results_new(CMD_INVALID, name, "Invalid %s command " - "(expected %d arguments, got %d)", name, val, argc); - break; + if (argc != val) { + error_name = ""; + } } - return error; + return error_name ? + cmd_results_new(CMD_INVALID, name, "Invalid %s command " + "(expected %s%d argument%s, got %d)", + name, error_name, val, val != 1 ? "s" : "", argc) + : NULL; } void apply_seat_config(struct seat_config *seat_config) { From 9227cb7d673b012a0c6730800d528f9d729972b6 Mon Sep 17 00:00:00 2001 From: Ian Fan Date: Tue, 23 Oct 2018 10:40:03 +0100 Subject: [PATCH 4/4] commands: replace EXPECTED_LESS_THAN with EXPECTED_AT_MOST This makes it a bit more obvious what the expected number of arguments is. --- include/sway/commands.h | 2 +- sway/commands.c | 6 +++--- sway/commands/bar/hidden_state.c | 2 +- sway/commands/bar/mode.c | 2 +- sway/commands/fullscreen.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/sway/commands.h b/include/sway/commands.h index 791b37dc..6606775a 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -40,7 +40,7 @@ struct cmd_results { enum expected_args { EXPECTED_AT_LEAST, - EXPECTED_LESS_THAN, + EXPECTED_AT_MOST, EXPECTED_EQUAL_TO }; diff --git a/sway/commands.c b/sway/commands.c index 6a5eb3dd..37c7169a 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -25,9 +25,9 @@ struct cmd_results *checkarg(int argc, const char *name, enum expected_args type error_name = "at least "; } break; - case EXPECTED_LESS_THAN: - if (argc >= val) { - error_name = "less than "; + case EXPECTED_AT_MOST: + if (argc > val) { + error_name = "at most "; } break; case EXPECTED_EQUAL_TO: diff --git a/sway/commands/bar/hidden_state.c b/sway/commands/bar/hidden_state.c index 28adf6c7..5be6c2dc 100644 --- a/sway/commands/bar/hidden_state.c +++ b/sway/commands/bar/hidden_state.c @@ -40,7 +40,7 @@ struct cmd_results *bar_cmd_hidden_state(int argc, char **argv) { if ((error = checkarg(argc, "hidden_state", EXPECTED_AT_LEAST, 1))) { return error; } - if ((error = checkarg(argc, "hidden_state", EXPECTED_LESS_THAN, 3))) { + if ((error = checkarg(argc, "hidden_state", EXPECTED_AT_MOST, 2))) { return error; } if (config->reading && argc > 1) { diff --git a/sway/commands/bar/mode.c b/sway/commands/bar/mode.c index dbdd3897..2cba785e 100644 --- a/sway/commands/bar/mode.c +++ b/sway/commands/bar/mode.c @@ -41,7 +41,7 @@ struct cmd_results *bar_cmd_mode(int argc, char **argv) { if ((error = checkarg(argc, "mode", EXPECTED_AT_LEAST, 1))) { return error; } - if ((error = checkarg(argc, "mode", EXPECTED_LESS_THAN, 3))) { + if ((error = checkarg(argc, "mode", EXPECTED_AT_MOST, 2))) { return error; } if (config->reading && argc > 1) { diff --git a/sway/commands/fullscreen.c b/sway/commands/fullscreen.c index 22d747b9..0204a73c 100644 --- a/sway/commands/fullscreen.c +++ b/sway/commands/fullscreen.c @@ -9,7 +9,7 @@ struct cmd_results *cmd_fullscreen(int argc, char **argv) { struct cmd_results *error = NULL; - if ((error = checkarg(argc, "fullscreen", EXPECTED_LESS_THAN, 2))) { + if ((error = checkarg(argc, "fullscreen", EXPECTED_AT_MOST, 1))) { return error; } struct sway_node *node = config->handler_context.node;