From 58df5aa6c4f5791ac18cddf8179fdbc4e66236fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Wed, 4 Mar 2015 14:01:42 +0100 Subject: [PATCH 1/3] Improve error messages on failing commands --- src/commands.c | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/commands.c b/src/commands.c index 498c25c8..13e0fa7e 100644 --- a/src/commands.c +++ b/src/commands.c @@ -27,16 +27,19 @@ y(map_close); \ } \ } while (0) -#define yerror(message) \ - do { \ - if (cmd_output->json_gen != NULL) { \ - y(map_open); \ - ystr("success"); \ - y(bool, false); \ - ystr("error"); \ - ystr(message); \ - y(map_close); \ - } \ +#define yerror(format, ...) \ + do { \ + if (cmd_output->json_gen != NULL) { \ + char *message; \ + sasprintf(&message, format, ##__VA_ARGS__); \ + y(map_open); \ + ystr("success"); \ + y(bool, false); \ + ystr("error"); \ + ystr(message); \ + y(map_close); \ + free(message); \ + } \ } while (0) /** When the command did not include match criteria (!), we use the currently @@ -574,8 +577,7 @@ void cmd_move_con_to_workspace_number(I3_CMD, char *which) { if (parsed_num == -1) { LOG("Could not parse initial part of \"%s\" as a number.\n", which); - // TODO: better error message - yerror("Could not parse number"); + yerror("Could not parse number \"%s\"", which); return; } @@ -1002,8 +1004,7 @@ void cmd_workspace_number(I3_CMD, char *which) { if (parsed_num == -1) { LOG("Could not parse initial part of \"%s\" as a number.\n", which); - // TODO: better error message - yerror("Could not parse number"); + yerror("Could not parse number \"%s\"", which); return; } @@ -2002,10 +2003,7 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) { } if (!workspace) { - // TODO: we should include the old workspace name here and use yajl for - // generating the reply. - // TODO: better error message - yerror("Old workspace not found"); + yerror("Old workspace \"%s\" not found", old_name); return; } @@ -2015,10 +2013,7 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) { !strcasecmp(child->name, new_name)); if (check_dest != NULL) { - // TODO: we should include the new workspace name here and use yajl for - // generating the reply. - // TODO: better error message - yerror("New workspace already exists"); + yerror("New workspace \"%s\" already exists", new_name); return; } From 6c675cc3f67427b17f1c28aa196ea017ea5ee55e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Sun, 1 Mar 2015 00:13:37 +0100 Subject: [PATCH 2/3] Glob filepath when calling append_layout fixes #1500 --- src/commands.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/commands.c b/src/commands.c index 13e0fa7e..0a64b2b8 100644 --- a/src/commands.c +++ b/src/commands.c @@ -902,11 +902,15 @@ void cmd_nop(I3_CMD, char *comment) { void cmd_append_layout(I3_CMD, char *path) { LOG("Appending layout \"%s\"\n", path); + /* Make sure we allow paths like '~/.i3/layout.json' */ + path = resolve_tilde(path); + json_content_t content = json_determine_content(path); LOG("JSON content = %d\n", content); if (content == JSON_CONTENT_UNKNOWN) { ELOG("Could not determine the contents of \"%s\", not loading.\n", path); ysuccess(false); + free(path); return; } @@ -948,6 +952,7 @@ void cmd_append_layout(I3_CMD, char *path) { if (content == JSON_CONTENT_WORKSPACE) ipc_send_workspace_event("restored", parent, NULL); + free(path); cmd_output->needs_tree_render = true; } From 9ebf17c39d6711a36651aa9d024114013286b3da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Wed, 4 Mar 2015 22:45:39 +0100 Subject: [PATCH 3/3] Properly error out when the layout file cannot be read. This will result in an actual error message for the user. fixes #1499 --- src/commands.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands.c b/src/commands.c index 0a64b2b8..92203e39 100644 --- a/src/commands.c +++ b/src/commands.c @@ -909,7 +909,7 @@ void cmd_append_layout(I3_CMD, char *path) { LOG("JSON content = %d\n", content); if (content == JSON_CONTENT_UNKNOWN) { ELOG("Could not determine the contents of \"%s\", not loading.\n", path); - ysuccess(false); + yerror("Could not determine the contents of \"%s\".", path); free(path); return; }