Send proper error messages upon parser failures, use yajl for generating command...

Fixes: #693

diff --git a/include/all.h b/include/all.h
index e550b08bcfa4a61ebae70ec434fc797a4764926a..11eaaba49d7f5a48dda5ca4729e6b3ccb4f5a375 100644 (file)
--- a/include/all.h
+++ b/include/all.h
@@ -35,6 +35,9 @@
 #include <xcb/xcb_keysyms.h>
 #include <xcb/xcb_icccm.h>
 
+#include <yajl/yajl_gen.h>
+#include <yajl/yajl_version.h>
+
 /* Contains compatibility definitions for old libxcb versions */
 #ifdef XCB_COMPAT
 #include "xcb_compat.h"

diff --git a/include/commands_parser.h b/include/commands_parser.h
index 5a4472d8acd2b4d090642d5a85a56c919f91e6ca..795cb0265715c44a5d41319a561d161682dc490f 100644 (file)
--- a/include/commands_parser.h
+++ b/include/commands_parser.h
@@ -10,6 +10,8 @@
 #ifndef _COMMANDS_PARSER_H
 #define _COMMANDS_PARSER_H
 
+#include <yajl/yajl_gen.h>
+
 /*
  * Holds the result of a call to any command. When calling
  * parse_command("floating enable, border none"), the parser will internally
@@ -20,8 +22,8 @@
  *
  */
 struct CommandResult {
-    /* The JSON-serialized output of this command. */
-    char *json_output;
+    /* The JSON generator to append a reply to. */
+    yajl_gen json_gen;
 
     /* Whether the command requires calling tree_render. */
     bool needs_tree_render;

diff --git a/src/assignments.c b/src/assignments.c
index 09793c38d7b456f9f4375b760bde9b7a930c21db..ae4affaa53ce82421562e9404911e38d63d2ddd6 100644 (file)
--- a/src/assignments.c
+++ b/src/assignments.c
@@ -49,7 +49,7 @@ void run_assignments(i3Window *window) {
             if (command_output->needs_tree_render)
                 needs_tree_render = true;
 
-            free(command_output->json_output);
+            yajl_gen_free(command_output->json_gen);
         }
 
         /* Store that we ran this assignment to not execute it again */

diff --git a/src/commands.c b/src/commands.c
index 86b8f68dec4f48073d2908669f7a51b7612318aa..40d1dc89fc8eb81db92ae9e21dded7ea8f658802 100644 (file)
--- a/src/commands.c
+++ b/src/commands.c
@@ -12,6 +12,16 @@
 
 #include "all.h"
 
+// Macros to make the YAJL API a bit easier to use.
+#define y(x, ...) yajl_gen_ ## x (cmd_output->json_gen, ##__VA_ARGS__)
+#define ystr(str) yajl_gen_string(cmd_output->json_gen, (unsigned char*)str, strlen(str))
+#define ysuccess(success) do { \
+    y(map_open); \
+    ystr("success"); \
+    y(bool, success); \
+    y(map_close); \
+} while (0)
+
 /** When the command did not include match criteria (!), we use the currently
  * focused command. Do not confuse this case with a command which included
  * criteria but which did not match any windows. This macro has to be called in
@@ -341,7 +351,7 @@ void cmd_move_con_to_workspace(I3_CMD, char *which) {
         ws = workspace_prev_on_output();
     else {
         ELOG("BUG: called with which=%s\n", which);
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -352,7 +362,7 @@ void cmd_move_con_to_workspace(I3_CMD, char *which) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -362,7 +372,7 @@ void cmd_move_con_to_workspace(I3_CMD, char *which) {
 void cmd_move_con_to_workspace_name(I3_CMD, char *name) {
     if (strncasecmp(name, "__i3_", strlen("__i3_")) == 0) {
         LOG("You cannot switch to the i3 internal workspaces.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -371,7 +381,7 @@ void cmd_move_con_to_workspace_name(I3_CMD, char *name) {
     /* Error out early to not create a non-existing workspace (in
      * workspace_get()) if we are not actually able to move anything. */
     if (match_is_empty(current_match) && focused->type == CT_WORKSPACE) {
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -388,7 +398,7 @@ void cmd_move_con_to_workspace_name(I3_CMD, char *name) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -401,7 +411,7 @@ void cmd_move_con_to_workspace_number(I3_CMD, char *which) {
     /* Error out early to not create a non-existing workspace (in
      * workspace_get()) if we are not actually able to move anything. */
     if (match_is_empty(current_match) && focused->type == CT_WORKSPACE) {
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -416,8 +426,13 @@ void cmd_move_con_to_workspace_number(I3_CMD, char *which) {
         parsed_num < 0 ||
         *endptr != '\0') {
         LOG("Could not parse \"%s\" as a number.\n", which);
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-                "\"error\": \"Could not parse number\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        // TODO: better error message
+        ystr("Could not parse number");
+        y(map_close);
         return;
     }
 
@@ -426,8 +441,13 @@ void cmd_move_con_to_workspace_number(I3_CMD, char *which) {
             child->num == parsed_num);
 
     if (!workspace) {
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-                "\"error\": \"No such workspace\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        // TODO: better error message
+        ystr("No such workspace");
+        y(map_close);
         return;
     }
 
@@ -440,7 +460,7 @@ void cmd_move_con_to_workspace_number(I3_CMD, char *which) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 static void cmd_resize_floating(I3_CMD, char *way, char *direction, Con *floating_con, int px) {
@@ -490,7 +510,7 @@ static void cmd_resize_tiling_direction(I3_CMD, char *way, char *direction, int
          (strcmp(direction, "left") == 0 || strcmp(direction, "right") == 0))) {
         LOG("You cannot resize in that direction. Your focus is in a %s split container currently.\n",
             (orientation == HORIZ ? "horizontal" : "vertical"));
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -501,7 +521,7 @@ static void cmd_resize_tiling_direction(I3_CMD, char *way, char *direction, int
     }
     if (other == TAILQ_END(workspaces)) {
         LOG("No other container in this direction found, cannot resize.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
     LOG("other->percent = %f\n", other->percent);
@@ -558,13 +578,13 @@ static void cmd_resize_tiling_width_height(I3_CMD, char *way, char *direction, i
          strcmp(direction, "width") == 0)) {
         LOG("You cannot resize in that direction. Your focus is in a %s split container currently.\n",
             (orientation == HORIZ ? "horizontal" : "vertical"));
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
     if (children == 1) {
         LOG("This is the only container, cannot resize.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -587,13 +607,13 @@ static void cmd_resize_tiling_width_height(I3_CMD, char *way, char *direction, i
             continue;
         if (!definitelyGreaterThan(child->percent - subtract_percent, 0.05, DBL_EPSILON)) {
             LOG("Not resizing, already at minimum size (child %p would end up with a size of %.f\n", child, child->percent - subtract_percent);
-            cmd_output->json_output = sstrdup("{\"success\": false}");
+            ysuccess(false);
             return;
         }
     }
     if (!definitelyGreaterThan(new_current_percent, 0.05, DBL_EPSILON)) {
         LOG("Not resizing, already at minimum size\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -635,7 +655,7 @@ void cmd_resize(I3_CMD, char *way, char *direction, char *resize_px, char *resiz
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -663,7 +683,7 @@ void cmd_border(I3_CMD, char *border_style_str) {
                 border_style = BS_1PIXEL;
             else {
                 ELOG("BUG: called with border_style=%s\n", border_style_str);
-                cmd_output->json_output = sstrdup("{\"success\": false}");
+                ysuccess(false);
                 return;
             }
         }
@@ -672,7 +692,7 @@ void cmd_border(I3_CMD, char *border_style_str) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -695,7 +715,7 @@ void cmd_append_layout(I3_CMD, char *path) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -717,7 +737,7 @@ void cmd_workspace(I3_CMD, char *which) {
         ws = workspace_prev_on_output();
     else {
         ELOG("BUG: called with which=%s\n", which);
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -725,7 +745,7 @@ void cmd_workspace(I3_CMD, char *which) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -742,8 +762,14 @@ void cmd_workspace_number(I3_CMD, char *which) {
         parsed_num < 0 ||
         *endptr != '\0') {
         LOG("Could not parse \"%s\" as a number.\n", which);
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-                "\"error\": \"Could not parse number\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        // TODO: better error message
+        ystr("Could not parse number");
+        y(map_close);
+
         return;
     }
 
@@ -753,8 +779,12 @@ void cmd_workspace_number(I3_CMD, char *which) {
 
     if (!workspace) {
         LOG("There is no workspace with number %d.\n", parsed_num);
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-                "\"error\": \"No such workspace\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        ystr("No such workspace");
+        y(map_close);
         return;
     }
 
@@ -762,7 +792,7 @@ void cmd_workspace_number(I3_CMD, char *which) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -774,7 +804,7 @@ void cmd_workspace_back_and_forth(I3_CMD) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -784,7 +814,7 @@ void cmd_workspace_back_and_forth(I3_CMD) {
 void cmd_workspace_name(I3_CMD, char *name) {
     if (strncasecmp(name, "__i3_", strlen("__i3_")) == 0) {
         LOG("You cannot switch to the i3 internal workspaces.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -799,7 +829,7 @@ void cmd_workspace_name(I3_CMD, char *name) {
             workspace_back_and_forth();
             tree_render();
         }
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -807,7 +837,7 @@ void cmd_workspace_name(I3_CMD, char *name) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -835,7 +865,7 @@ void cmd_mark(I3_CMD, char *mark) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -847,7 +877,7 @@ void cmd_mode(I3_CMD, char *mode) {
     switch_mode(mode);
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -885,7 +915,7 @@ void cmd_move_con_to_output(I3_CMD, char *name) {
 
     if (!output) {
         LOG("No such output found.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -893,7 +923,7 @@ void cmd_move_con_to_output(I3_CMD, char *name) {
     Con *ws = NULL;
     GREP_FIRST(ws, output_get_content(output->con), workspace_is_visible(child));
     if (!ws) {
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -904,7 +934,7 @@ void cmd_move_con_to_output(I3_CMD, char *name) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -935,7 +965,7 @@ void cmd_floating(I3_CMD, char *floating_mode) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -954,7 +984,7 @@ void cmd_move_workspace_to_output(I3_CMD, char *name) {
         Output *output = get_output_from_string(current_output, name);
         if (!output) {
             LOG("No such output\n");
-            cmd_output->json_output = sstrdup("{\"success\": false}");
+            ysuccess(false);
             return;
         }
 
@@ -1025,7 +1055,7 @@ void cmd_move_workspace_to_output(I3_CMD, char *name) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1039,7 +1069,7 @@ void cmd_split(I3_CMD, char *direction) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1060,7 +1090,7 @@ void cmd_kill(I3_CMD, char *kill_mode_str) {
         kill_mode = KILL_CLIENT;
     else {
         ELOG("BUG: called with kill_mode=%s\n", kill_mode_str);
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1076,7 +1106,7 @@ void cmd_kill(I3_CMD, char *kill_mode_str) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1090,7 +1120,7 @@ void cmd_exec(I3_CMD, char *nosn, char *command) {
     start_application(command, no_startup_id);
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1102,7 +1132,7 @@ void cmd_focus_direction(I3_CMD, char *direction) {
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1118,13 +1148,13 @@ void cmd_focus_direction(I3_CMD, char *direction) {
         tree_next('n', VERT);
     else {
         ELOG("Invalid focus direction (%s)\n", direction);
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1136,7 +1166,7 @@ void cmd_focus_window_mode(I3_CMD, char *window_mode) {
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1163,7 +1193,7 @@ void cmd_focus_window_mode(I3_CMD, char *window_mode) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1175,7 +1205,7 @@ void cmd_focus_level(I3_CMD, char *level) {
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1187,7 +1217,7 @@ void cmd_focus_level(I3_CMD, char *level) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1200,7 +1230,7 @@ void cmd_focus(I3_CMD) {
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1210,9 +1240,13 @@ void cmd_focus(I3_CMD) {
         ELOG("You have to specify which window/container should be focused.\n");
         ELOG("Example: [class=\"urxvt\" title=\"irssi\"] focus\n");
 
-        sasprintf(&(cmd_output->json_output),
-                  "{\"success\":false, \"error\":\"You have to "
-                  "specify which window/container should be focused\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        ystr("You have to specify which window/container should be focused");
+        y(map_close);
+
         return;
     }
 
@@ -1253,7 +1287,7 @@ void cmd_focus(I3_CMD) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1275,7 +1309,7 @@ void cmd_fullscreen(I3_CMD, char *fullscreen_mode) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1310,7 +1344,7 @@ void cmd_move_direction(I3_CMD, char *direction, char *move_px) {
     }
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1338,7 +1372,7 @@ void cmd_layout(I3_CMD, char *layout_str) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1365,7 +1399,7 @@ void cmd_reload(I3_CMD) {
     ipc_send_event("workspace", I3_IPC_EVENT_WORKSPACE, "{\"change\":\"reload\"}");
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1377,7 +1411,7 @@ void cmd_restart(I3_CMD) {
     i3_restart(false);
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1388,8 +1422,13 @@ void cmd_open(I3_CMD) {
     LOG("opening new container\n");
     Con *con = tree_open_con(NULL, NULL);
     con_focus(con);
-    sasprintf(&(cmd_output->json_output),
-              "{\"success\":true, \"id\":%ld}", (long int)con);
+
+    y(map_open);
+    ystr("success");
+    y(bool, true);
+    ystr("id");
+    y(integer, (long int)con);
+    y(map_close);
 
     cmd_output->needs_tree_render = true;
 }
@@ -1417,7 +1456,7 @@ void cmd_focus_output(I3_CMD, char *name) {
 
     if (!output) {
         LOG("No such output found.\n");
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1425,7 +1464,7 @@ void cmd_focus_output(I3_CMD, char *name) {
     Con *ws = NULL;
     GREP_FIRST(ws, output_get_content(output->con), workspace_is_visible(child));
     if (!ws) {
-        cmd_output->json_output = sstrdup("{\"success\": false}");
+        ysuccess(false);
         return;
     }
 
@@ -1433,7 +1472,7 @@ void cmd_focus_output(I3_CMD, char *name) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1447,9 +1486,12 @@ void cmd_move_window_to_position(I3_CMD, char *method, char *cx, char *cy) {
 
     if (!con_is_floating(focused)) {
         ELOG("Cannot change position. The window/container is not floating\n");
-        sasprintf(&(cmd_output->json_output),
-                  "{\"success\":false, \"error\":\"Cannot change position. "
-                  "The window/container is not floating.\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        ystr("Cannot change position. The window/container is not floating.");
+        y(map_close);
         return;
     }
 
@@ -1473,7 +1515,7 @@ void cmd_move_window_to_position(I3_CMD, char *method, char *cx, char *cy) {
     }
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1484,9 +1526,12 @@ void cmd_move_window_to_center(I3_CMD, char *method) {
 
     if (!con_is_floating(focused)) {
         ELOG("Cannot change position. The window/container is not floating\n");
-        sasprintf(&(cmd_output->json_output),
-                  "{\"success\":false, \"error\":\"Cannot change position. "
-                  "The window/container is not floating.\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        ystr("Cannot change position. The window/container is not floating.");
+        y(map_close);
     }
 
     if (strcmp(method, "absolute") == 0) {
@@ -1512,7 +1557,7 @@ void cmd_move_window_to_center(I3_CMD, char *method) {
     }
 
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1532,7 +1577,7 @@ void cmd_move_scratchpad(I3_CMD) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1554,7 +1599,7 @@ void cmd_scratchpad_show(I3_CMD) {
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 }
 
 /*
@@ -1572,8 +1617,13 @@ 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.
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-            "\"error\":\"Old workspace not found\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        // TODO: better error message
+        ystr("Old workspace not found");
+        y(map_close);
         return;
     }
 
@@ -1585,8 +1635,13 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) {
     if (check_dest != NULL) {
         // TODO: we should include the new workspace name here and use yajl for
         // generating the reply.
-        cmd_output->json_output = sstrdup("{\"success\": false, "
-            "\"error\":\"New workspace already exists\"}");
+        y(map_open);
+        ystr("success");
+        y(bool, false);
+        ystr("error");
+        // TODO: better error message
+        ystr("New workspace already exists");
+        y(map_close);
         return;
     }
 
@@ -1612,7 +1667,7 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) {
     con_focus(previously_focused);
 
     cmd_output->needs_tree_render = true;
-    cmd_output->json_output = sstrdup("{\"success\": true}");
+    ysuccess(true);
 
     ipc_send_event("workspace", I3_IPC_EVENT_WORKSPACE, "{\"change\":\"rename\"}");
 }

diff --git a/src/commands_parser.c b/src/commands_parser.c
index d15fea35569f7e3f2f5796e7639f091c84f3bd9f..73a14565ff3201ff0f8b741fcfdadf8ee51cd3a6 100644 (file)
--- a/src/commands_parser.c
+++ b/src/commands_parser.c
@@ -32,6 +32,10 @@
 
 #include "all.h"
 
+// Macros to make the YAJL API a bit easier to use.
+#define y(x, ...) yajl_gen_ ## x (command_output.json_gen, ##__VA_ARGS__)
+#define ystr(str) yajl_gen_string(command_output.json_gen, (unsigned char*)str, strlen(str))
+
 /*******************************************************************************
  * The data structures used for parsing. Essentially the current state and a
  * list of tokens for that state.
@@ -185,20 +189,9 @@ static void next_state(const cmdp_token *token) {
     if (token->next_state == __CALL) {
         DLOG("should call stuff, yay. call_id = %d\n",
                 token->extra.call_identifier);
-        subcommand_output.json_output = NULL;
+        subcommand_output.json_gen = command_output.json_gen;
         subcommand_output.needs_tree_render = false;
         GENERATED_call(token->extra.call_identifier, &subcommand_output);
-        if (subcommand_output.json_output) {
-            DLOG("Subcommand JSON output: %s\n", subcommand_output.json_output);
-            char *buffer;
-            /* In the beginning, the contents of json_output are "[\0". */
-            if (command_output.json_output[1] == '\0')
-                sasprintf(&buffer, "%s%s", command_output.json_output, subcommand_output.json_output);
-            else sasprintf(&buffer, "%s, %s", command_output.json_output, subcommand_output.json_output);
-            free(command_output.json_output);
-            command_output.json_output = buffer;
-            DLOG("merged command JSON output: %s\n", command_output.json_output);
-        }
         /* If any subcommand requires a tree_render(), we need to make the
          * whole parser result request a tree_render(). */
         if (subcommand_output.needs_tree_render)
@@ -217,7 +210,15 @@ static void next_state(const cmdp_token *token) {
 struct CommandResult *parse_command(const char *input) {
     DLOG("new parser handling: %s\n", input);
     state = INITIAL;
-    command_output.json_output = sstrdup("[");
+
+/* A YAJL JSON generator used for formatting replies. */
+#if YAJL_MAJOR >= 2
+    command_output.json_gen = yajl_gen_alloc(NULL);
+#else
+    command_output.json_gen = yajl_gen_alloc(NULL, NULL);
+#endif
+
+    y(array_open);
     command_output.needs_tree_render = false;
 
     const char *walk = input;
@@ -392,6 +393,18 @@ struct CommandResult *parse_command(const char *input) {
             printf("Your command: %s\n", input);
             printf("              %s\n", position);
 
+            /* Format this error message as a JSON reply. */
+            y(map_open);
+            ystr("success");
+            y(bool, false);
+            ystr("error");
+            ystr(errormessage);
+            ystr("input");
+            ystr(input);
+            ystr("errorposition");
+            ystr(position);
+            y(map_close);
+
             free(position);
             free(errormessage);
             clear_stack();
@@ -399,11 +412,8 @@ struct CommandResult *parse_command(const char *input) {
         }
     }
 
-    char *buffer;
-    sasprintf(&buffer, "%s]", command_output.json_output);
-    free(command_output.json_output);
-    command_output.json_output = buffer;
-    DLOG("command_output.json_output = %s\n", command_output.json_output);
+    y(array_close);
+
     DLOG("command_output.needs_tree_render = %d\n", command_output.needs_tree_render);
     return &command_output;
 }

diff --git a/src/handlers.c b/src/handlers.c
index 490bf6eb5edc4f6cbffd4fd76574fef09ba9c762..148d63370a8b5cc2e4b1a45e04ae6ea382b00292 100644 (file)
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -124,7 +124,7 @@ static void handle_key_press(xcb_key_press_event_t *event) {
     if (command_output->needs_tree_render)
         tree_render();
 
-    free(command_output->json_output);
+    yajl_gen_free(command_output->json_gen);
 }
 
 /*

diff --git a/src/ipc.c b/src/ipc.c
index 5e91e21de27e325595602002b32a2c9ce1d76aba..60ce814540d6c7a3c61ac3969b1a652770226c06 100644 (file)
--- a/src/ipc.c
+++ b/src/ipc.c
@@ -125,12 +125,18 @@ IPC_HANDLER(command) {
     if (command_output->needs_tree_render)
         tree_render();
 
-    /* If no reply was provided, we just use the default success message */
-    ipc_send_message(fd, strlen(command_output->json_output),
-                     I3_IPC_REPLY_TYPE_COMMAND,
-                     (const uint8_t*)command_output->json_output);
+    const unsigned char *reply;
+#if YAJL_MAJOR >= 2
+    size_t length;
+#else
+    unsigned int length;
+#endif
+    yajl_gen_get_buf(command_output->json_gen, &reply, &length);
+
+    ipc_send_message(fd, length, I3_IPC_REPLY_TYPE_COMMAND,
+                     (const uint8_t*)reply);
 
-    free(command_output->json_output);
+    yajl_gen_free(command_output->json_gen);
 }
 
 static void dump_rect(yajl_gen gen, const char *name, Rect r) {

diff --git a/testcases/t/120-multiple-cmds.t b/testcases/t/120-multiple-cmds.t
index 2d0ddcf440fd38bea138e7bf9533399a25c6f5cc..088caf7199eb7e7ed44698dd1a0da13490a39c09 100644 (file)
--- a/testcases/t/120-multiple-cmds.t
+++ b/testcases/t/120-multiple-cmds.t
@@ -63,4 +63,12 @@ cmd 'move gibberish' for (0 .. 10);
 
 does_i3_live;
 
+################################################################################
+# regression test: an invalid command should come back with an error.
+################################################################################
+
+my $reply = cmd 'bullshit-command-which-we-never-implement meh';
+is(scalar @$reply, 1, 'got one command reply');
+ok(!$reply->[0]->{success}, 'reply has success == false');
+
 done_testing;