Refactor the interface of commands.c

This change has two implications:

1) tree_render() will now be called precisely once for input which consists of
   multiple commands (like "focus left; focus right"). Also, the caller of
   parse_command() has to call it. This makes us able to fix tickets such as
   ticket #608 (where multiple tree_render() calls are noticable).

2) The output of a command is now a JSON array of return values of the
   individual subcommands. In the case of "focus left; focus right", this is:

   [{"success":true}, {"success":true}]

   While this is incompatible with what i3 returned before, the return value of
   commands was undocumented and therefore not subject to our API stability.

generate-command-parser.pl

@@ -118,8 +118,7 @@ close($enumfh);
 
 # Third step: Generate the call function.
 open(my $callfh, '>', 'GENERATED_call.h');
-say $callfh 'static char *GENERATED_call(const int call_identifier) {';
+say $callfh 'static void GENERATED_call(const int call_identifier, struct CommandResult *result) {';
-say $callfh '    char *output = NULL;';
 say $callfh '    switch (call_identifier) {';
 my $call_id = 0;
 for my $state (@keys) {
@@ -143,11 +142,11 @@ for my $state (@keys) {
         say $callfh '#ifndef TEST_PARSER';
         my $real_cmd = $cmd;
         if ($real_cmd =~ /\(\)/) {
-            $real_cmd =~ s/\(/(&current_match/;
+            $real_cmd =~ s/\(/(&current_match, result/;
         } else {
-            $real_cmd =~ s/\(/(&current_match, /;
+            $real_cmd =~ s/\(/(&current_match, result, /;
         }
-        say $callfh "             output = $real_cmd;";
+        say $callfh "             $real_cmd;";
         say $callfh '#else';
         # debug
         $cmd =~ s/[^(]+\(//;
@@ -164,7 +163,6 @@ for my $state (@keys) {
 say $callfh '        default:';
 say $callfh '            printf("BUG in the parser. state = %d\n", call_identifier);';
 say $callfh '    }';
-say $callfh '    return output;';
 say $callfh '}';
 close($callfh);
 

include/commands.h

@@ -10,6 +10,11 @@
 #ifndef _COMMANDS_H
 #define _COMMANDS_H
 
+#include "commands_parser.h"
+
+/** The beginning of the prototype for every cmd_ function. */
+#define I3_CMD Match *current_match, struct CommandResult *cmd_output
+
 /*
  * Helper data structure for an operation window (window on which the operation
  * will be performed). Used to build the TAILQ owindows.
@@ -27,207 +32,207 @@ typedef TAILQ_HEAD(owindows_head, owindow) owindows_head;
  * commands.c for matching target windows of a command.
  *
  */
-char *cmd_criteria_init(Match *current_match);
+void cmd_criteria_init(I3_CMD);
 
 /**
  * A match specification just finished (the closing square bracket was found),
  * so we filter the list of owindows.
  *
  */
-char *cmd_criteria_match_windows(Match *current_match);
+void cmd_criteria_match_windows(I3_CMD);
 
 /**
  * Interprets a ctype=cvalue pair and adds it to the current match
  * specification.
  *
  */
-char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue);
+void cmd_criteria_add(I3_CMD, char *ctype, char *cvalue);
 
 /**
  * Implementation of 'move [window|container] [to] workspace
  * next|prev|next_on_output|prev_on_output'.
  *
  */
-char *cmd_move_con_to_workspace(Match *current_match, char *which);
+void cmd_move_con_to_workspace(I3_CMD, char *which);
 
 /**
  * Implementation of 'move [window|container] [to] workspace <name>'.
  *
  */
-char *cmd_move_con_to_workspace_name(Match *current_match, char *name);
+void cmd_move_con_to_workspace_name(I3_CMD, char *name);
 
 /**
  * Implementation of 'resize grow|shrink <direction> [<px> px] [or <ppt> ppt]'.
  *
  */
-char *cmd_resize(Match *current_match, char *way, char *direction, char *resize_px, char *resize_ppt);
+void cmd_resize(I3_CMD, char *way, char *direction, char *resize_px, char *resize_ppt);
 
 /**
  * Implementation of 'border normal|none|1pixel|toggle'.
  *
  */
-char *cmd_border(Match *current_match, char *border_style_str);
+void cmd_border(I3_CMD, char *border_style_str);
 
 /**
  * Implementation of 'nop <comment>'.
  *
  */
-char *cmd_nop(Match *current_match, char *comment);
+void cmd_nop(I3_CMD, char *comment);
 
 /**
  * Implementation of 'append_layout <path>'.
  *
  */
-char *cmd_append_layout(Match *current_match, char *path);
+void cmd_append_layout(I3_CMD, char *path);
 
 /**
  * Implementation of 'workspace next|prev|next_on_output|prev_on_output'.
  *
  */
-char *cmd_workspace(Match *current_match, char *which);
+void cmd_workspace(I3_CMD, char *which);
 
 /**
  * Implementation of 'workspace back_and_forth'.
  *
  */
-char *cmd_workspace_back_and_forth(Match *current_match);
+void cmd_workspace_back_and_forth(I3_CMD);
 
 /**
  * Implementation of 'workspace <name>'
  *
  */
-char *cmd_workspace_name(Match *current_match, char *name);
+void cmd_workspace_name(I3_CMD, char *name);
 
 /**
  * Implementation of 'mark <mark>'
  *
  */
-char *cmd_mark(Match *current_match, char *mark);
+void cmd_mark(I3_CMD, char *mark);
 
 /**
  * Implementation of 'mode <string>'.
  *
  */
-char *cmd_mode(Match *current_match, char *mode);
+void cmd_mode(I3_CMD, char *mode);
 
 /**
  * Implementation of 'move [window|container] [to] output <str>'.
  *
  */
-char *cmd_move_con_to_output(Match *current_match, char *name);
+void cmd_move_con_to_output(I3_CMD, char *name);
 
 /**
  * Implementation of 'floating enable|disable|toggle'
  *
  */
-char *cmd_floating(Match *current_match, char *floating_mode);
+void cmd_floating(I3_CMD, char *floating_mode);
 
 /**
  * Implementation of 'move workspace to [output] <str>'.
  *
  */
-char *cmd_move_workspace_to_output(Match *current_match, char *name);
+void cmd_move_workspace_to_output(I3_CMD, char *name);
 
 /**
  * Implementation of 'split v|h|vertical|horizontal'.
  *
  */
-char *cmd_split(Match *current_match, char *direction);
+void cmd_split(I3_CMD, char *direction);
 
 /**
  * Implementaiton of 'kill [window|client]'.
  *
  */
-char *cmd_kill(Match *current_match, char *kill_mode);
+void cmd_kill(I3_CMD, char *kill_mode_str);
 
 /**
  * Implementation of 'exec [--no-startup-id] <command>'.
  *
  */
-char *cmd_exec(Match *current_match, char *nosn, char *command);
+void cmd_exec(I3_CMD, char *nosn, char *command);
 
 /**
  * Implementation of 'focus left|right|up|down'.
  *
  */
-char *cmd_focus_direction(Match *current_match, char *direction);
+void cmd_focus_direction(I3_CMD, char *direction);
 
 /**
  * Implementation of 'focus tiling|floating|mode_toggle'.
  *
  */
-char *cmd_focus_window_mode(Match *current_match, char *window_mode);
+void cmd_focus_window_mode(I3_CMD, char *window_mode);
 
 /**
  * Implementation of 'focus parent|child'.
  *
  */
-char *cmd_focus_level(Match *current_match, char *level);
+void cmd_focus_level(I3_CMD, char *level);
 
 /**
  * Implementation of 'focus'.
  *
  */
-char *cmd_focus(Match *current_match);
+void cmd_focus(I3_CMD);
 
 /**
  * Implementation of 'fullscreen [global]'.
  *
  */
-char *cmd_fullscreen(Match *current_match, char *fullscreen_mode);
+void cmd_fullscreen(I3_CMD, char *fullscreen_mode);
 
 /**
  * Implementation of 'move <direction> [<pixels> [px]]'.
  *
  */
-char *cmd_move_direction(Match *current_match, char *direction, char *px);
+void cmd_move_direction(I3_CMD, char *direction, char *move_px);
 
 /**
  * Implementation of 'layout default|stacked|stacking|tabbed'.
  *
  */
-char *cmd_layout(Match *current_match, char *layout);
+void cmd_layout(I3_CMD, char *layout_str);
 
 /**
  * Implementaiton of 'exit'.
  *
  */
-char *cmd_exit(Match *current_match);
+void cmd_exit(I3_CMD);
 
 /**
  * Implementaiton of 'reload'.
  *
  */
-char *cmd_reload(Match *current_match);
+void cmd_reload(I3_CMD);
 
 /**
  * Implementaiton of 'restart'.
  *
  */
-char *cmd_restart(Match *current_match);
+void cmd_restart(I3_CMD);
 
 /**
  * Implementaiton of 'open'.
  *
  */
-char *cmd_open(Match *current_match);
+void cmd_open(I3_CMD);
 
 /**
  * Implementation of 'focus output <output>'.
  *
  */
-char *cmd_focus_output(Match *current_match, char *name);
+void cmd_focus_output(I3_CMD, char *name);
 
 /**
  * Implementation of 'move scratchpad'.
  *
  */
-char *cmd_move_scratchpad(Match *current_match);
+void cmd_move_scratchpad(I3_CMD);
 
 /**
  * Implementation of 'scratchpad show'.
  *
  */
-char *cmd_scratchpad_show(Match *current_match);
+void cmd_scratchpad_show(I3_CMD);
 
 #endif

include/commands_parser.h

@@ -10,6 +10,23 @@
 #ifndef _COMMANDS_PARSER_H
 #define _COMMANDS_PARSER_H
 
-char *parse_command(const char *input);
+/*
+ * Holds the result of a call to any command. When calling
+ * parse_command("floating enable, border none"), the parser will internally
+ * use a struct CommandResult when calling cmd_floating and cmd_border.
+ * parse_command will also return another struct CommandResult, whose
+ * json_output is set to a map of individual json_outputs and whose
+ * needs_tree_trender is true if any individual needs_tree_render was true.
+ *
+ */
+struct CommandResult {
+    /* The JSON-serialized output of this command. */
+    char *json_output;
+
+    /* Whether the command requires calling tree_render. */
+    bool needs_tree_render;
+};
+
+struct CommandResult *parse_command(const char *input);
 
 #endif

src/assignments.c

@@ -41,9 +41,13 @@ void run_assignments(i3Window *window) {
             DLOG("execute command %s\n", current->dest.command);
             char *full_command;
             sasprintf(&full_command, "[id=\"%d\"] %s", window->id, current->dest.command);
-            char *json_result = parse_command(full_command);
+            struct CommandResult *command_output = parse_command(full_command);
-            FREE(full_command);
+            free(full_command);
-            FREE(json_result);
+
+            if (command_output->needs_tree_render)
+                tree_render();
+
+            free(command_output->json_output);
         }
 
         /* Store that we ran this assignment to not execute it again */

src/commands.c

@@ -170,7 +170,7 @@ void cmd_MIGRATION_start_nagbar() {
  * commands.c for matching target windows of a command.
  *
  */
-char *cmd_criteria_init(Match *current_match) {
+void cmd_criteria_init(I3_CMD) {
     Con *con;
     owindow *ow;
 
@@ -188,9 +188,6 @@ char *cmd_criteria_init(Match *current_match) {
         ow->con = con;
         TAILQ_INSERT_TAIL(&owindows, ow, owindows);
     }
-
-    /* This command is internal and does not generate a JSON reply. */
-    return NULL;
 }
 
 /*
@@ -198,7 +195,7 @@ char *cmd_criteria_init(Match *current_match) {
  * so we filter the list of owindows.
  *
  */
-char *cmd_criteria_match_windows(Match *current_match) {
+void cmd_criteria_match_windows(I3_CMD) {
     owindow *next, *current;
 
     DLOG("match specification finished, matching...\n");
@@ -239,9 +236,6 @@ char *cmd_criteria_match_windows(Match *current_match) {
     TAILQ_FOREACH(current, &owindows, owindows) {
         DLOG("matching: %p / %s\n", current->con, current->con->name);
     }
-
-    /* This command is internal and does not generate a JSON reply. */
-    return NULL;
 }
 
 /*
@@ -249,22 +243,22 @@ char *cmd_criteria_match_windows(Match *current_match) {
  * specification.
  *
  */
-char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue) {
+void cmd_criteria_add(I3_CMD, char *ctype, char *cvalue) {
     DLOG("ctype=*%s*, cvalue=*%s*\n", ctype, cvalue);
 
     if (strcmp(ctype, "class") == 0) {
         current_match->class = regex_new(cvalue);
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "instance") == 0) {
         current_match->instance = regex_new(cvalue);
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "window_role") == 0) {
         current_match->role = regex_new(cvalue);
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "con_id") == 0) {
@@ -279,7 +273,7 @@ char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue) {
             current_match->con_id = (Con*)parsed;
             printf("id as int = %p\n", current_match->con_id);
         }
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "id") == 0) {
@@ -294,17 +288,17 @@ char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue) {
             current_match->id = parsed;
             printf("window id as int = %d\n", current_match->id);
         }
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "con_mark") == 0) {
         current_match->mark = regex_new(cvalue);
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "title") == 0) {
         current_match->title = regex_new(cvalue);
-        return NULL;
+        return;
     }
 
     if (strcmp(ctype, "urgent") == 0) {
@@ -317,13 +311,10 @@ char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue) {
                    strcasecmp(cvalue, "first") == 0) {
             current_match->urgent = U_OLDEST;
         }
-        return NULL;
+        return;
     }
 
     ELOG("Unknown criterion: %s\n", ctype);
-
-    /* This command is internal and does not generate a JSON reply. */
-    return NULL;
 }
 
 /*
@@ -331,7 +322,7 @@ char *cmd_criteria_add(Match *current_match, char *ctype, char *cvalue) {
  * next|prev|next_on_output|prev_on_output'.
  *
  */
-char *cmd_move_con_to_workspace(Match *current_match, char *which) {
+void cmd_move_con_to_workspace(I3_CMD, char *which) {
     owindow *current;
 
     DLOG("which=%s\n", which);
@@ -350,7 +341,8 @@ char *cmd_move_con_to_workspace(Match *current_match, char *which) {
         ws = workspace_prev_on_output();
     else {
         ELOG("BUG: called with which=%s\n", which);
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     TAILQ_FOREACH(current, &owindows, owindows) {
@@ -358,28 +350,30 @@ char *cmd_move_con_to_workspace(Match *current_match, char *which) {
         con_move_to_workspace(current->con, ws, true, false);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'move [window|container] [to] workspace <name>'.
  *
  */
-char *cmd_move_con_to_workspace_name(Match *current_match, char *name) {
+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");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     owindow *current;
 
     /* 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)
+    if (match_is_empty(current_match) && focused->type == CT_WORKSPACE) {
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
+    }
 
     LOG("should move window to workspace %s\n", name);
     /* get the workspace */
@@ -392,17 +386,16 @@ char *cmd_move_con_to_workspace_name(Match *current_match, char *name) {
         con_move_to_workspace(current->con, ws, true, false);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'resize grow|shrink <direction> [<px> px] [or <ppt> ppt]'.
  *
  */
-char *cmd_resize(Match *current_match, char *way, char *direction, char *resize_px, char *resize_ppt) {
+void cmd_resize(I3_CMD, char *way, char *direction, char *resize_px, char *resize_ppt) {
     /* resize <grow|shrink> <direction> [<px> px] [or <ppt> ppt] */
     DLOG("resizing in way %s, direction %s, px %s or ppt %s\n", way, direction, resize_px, resize_ppt);
     // TODO: We could either handle this in the parser itself as a separate token (and make the stack typed) or we need a better way to convert a string to a number with error checking
@@ -459,7 +452,8 @@ char *cmd_resize(Match *current_match, char *way, char *direction, char *resize_
              (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"));
-            return sstrdup("{\"sucess\": false}");
+            cmd_output->json_output = sstrdup("{\"sucess\": false}");
+            return;
         }
 
         if (strcmp(direction, "up") == 0 || strcmp(direction, "left") == 0) {
@@ -469,7 +463,8 @@ char *cmd_resize(Match *current_match, char *way, char *direction, char *resize_
         }
         if (other == TAILQ_END(workspaces)) {
             LOG("No other container in this direction found, cannot resize.\n");
-            return sstrdup("{\"sucess\": false}");
+            cmd_output->json_output = sstrdup("{\"sucess\": false}");
+            return;
         }
         LOG("other->percent = %f\n", other->percent);
         LOG("current->percent before = %f\n", current->percent);
@@ -494,17 +489,16 @@ char *cmd_resize(Match *current_match, char *way, char *direction, char *resize_
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'border normal|none|1pixel|toggle'.
  *
  */
-char *cmd_border(Match *current_match, char *border_style_str) {
+void cmd_border(I3_CMD, char *border_style_str) {
     DLOG("border style should be changed to %s\n", border_style_str);
     owindow *current;
 
@@ -525,48 +519,46 @@ char *cmd_border(Match *current_match, char *border_style_str) {
                 border_style = BS_1PIXEL;
             else {
                 ELOG("BUG: called with border_style=%s\n", border_style_str);
-                return sstrdup("{\"sucess\": false}");
+                cmd_output->json_output = sstrdup("{\"sucess\": false}");
+                return;
             }
         }
         con_set_border_style(current->con, border_style);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'nop <comment>'.
  *
  */
-char *cmd_nop(Match *current_match, char *comment) {
+void cmd_nop(I3_CMD, char *comment) {
     LOG("-------------------------------------------------\n");
     LOG("  NOP: %s\n", comment);
     LOG("-------------------------------------------------\n");
-
-    return NULL;
 }
 
 /*
  * Implementation of 'append_layout <path>'.
  *
  */
-char *cmd_append_layout(Match *current_match, char *path) {
+void cmd_append_layout(I3_CMD, char *path) {
     LOG("Appending layout \"%s\"\n", path);
     tree_append_json(path);
-    tree_render();
 
+    cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'workspace next|prev|next_on_output|prev_on_output'.
  *
  */
-char *cmd_workspace(Match *current_match, char *which) {
+void cmd_workspace(I3_CMD, char *which) {
     Con *ws;
 
     DLOG("which=%s\n", which);
@@ -581,36 +573,38 @@ char *cmd_workspace(Match *current_match, char *which) {
         ws = workspace_prev_on_output();
     else {
         ELOG("BUG: called with which=%s\n", which);
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     workspace_show(ws);
-    tree_render();
 
+    cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'workspace back_and_forth'.
  *
  */
-char *cmd_workspace_back_and_forth(Match *current_match) {
+void cmd_workspace_back_and_forth(I3_CMD) {
     workspace_back_and_forth();
-    tree_render();
 
+    cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'workspace <name>'
  *
  */
-char *cmd_workspace_name(Match *current_match, char *name) {
+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");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     DLOG("should switch to workspace %s\n", name);
@@ -624,22 +618,22 @@ char *cmd_workspace_name(Match *current_match, char *name) {
             workspace_back_and_forth();
             tree_render();
         }
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     workspace_show_by_name(name);
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'mark <mark>'
  *
  */
-char *cmd_mark(Match *current_match, char *mark) {
+void cmd_mark(I3_CMD, char *mark) {
     DLOG("Clearing all windows which have that mark first\n");
 
     Con *con;
@@ -658,29 +652,28 @@ char *cmd_mark(Match *current_match, char *mark) {
         current->con->mark = sstrdup(mark);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'mode <string>'.
  *
  */
-char *cmd_mode(Match *current_match, char *mode) {
+void cmd_mode(I3_CMD, char *mode) {
     DLOG("mode=%s\n", mode);
     switch_mode(mode);
 
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'move [window|container] [to] output <str>'.
  *
  */
-char *cmd_move_con_to_output(Match *current_match, char *name) {
+void cmd_move_con_to_output(I3_CMD, char *name) {
     owindow *current;
 
     DLOG("should move window to output %s\n", name);
@@ -711,31 +704,33 @@ char *cmd_move_con_to_output(Match *current_match, char *name) {
 
     if (!output) {
         LOG("No such output found.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     /* get visible workspace on output */
     Con *ws = NULL;
     GREP_FIRST(ws, output_get_content(output->con), workspace_is_visible(child));
-    if (!ws)
+    if (!ws) {
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
+    }
 
     TAILQ_FOREACH(current, &owindows, owindows) {
         DLOG("matching: %p / %s\n", current->con, current->con->name);
         con_move_to_workspace(current->con, ws, true, false);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'floating enable|disable|toggle'
  *
  */
-char *cmd_floating(Match *current_match, char *floating_mode) {
+void cmd_floating(I3_CMD, char *floating_mode) {
     owindow *current;
 
     DLOG("floating_mode=%s\n", floating_mode);
@@ -757,17 +752,16 @@ char *cmd_floating(Match *current_match, char *floating_mode) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'move workspace to [output] <str>'.
  *
  */
-char *cmd_move_workspace_to_output(Match *current_match, char *name) {
+void cmd_move_workspace_to_output(I3_CMD, char *name) {
     DLOG("should move workspace to output %s\n", name);
 
     HANDLE_EMPTY_MATCH;
@@ -779,7 +773,8 @@ char *cmd_move_workspace_to_output(Match *current_match, char *name) {
         Output *output = get_output_from_string(current_output, name);
         if (!output) {
             LOG("No such output\n");
-            return sstrdup("{\"sucess\": false}");
+            cmd_output->json_output = sstrdup("{\"sucess\": false}");
+            return;
         }
 
         Con *content = output_get_content(output->con);
@@ -847,32 +842,30 @@ char *cmd_move_workspace_to_output(Match *current_match, char *name) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'split v|h|vertical|horizontal'.
  *
  */
-char *cmd_split(Match *current_match, char *direction) {
+void cmd_split(I3_CMD, char *direction) {
     /* TODO: use matches */
     LOG("splitting in direction %c\n", direction[0]);
     tree_split(focused, (direction[0] == 'v' ? VERT : HORIZ));
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementaiton of 'kill [window|client]'.
  *
  */
-char *cmd_kill(Match *current_match, char *kill_mode_str) {
+void cmd_kill(I3_CMD, char *kill_mode_str) {
     if (kill_mode_str == NULL)
         kill_mode_str = "window";
     owindow *current;
@@ -886,7 +879,8 @@ char *cmd_kill(Match *current_match, char *kill_mode_str) {
         kill_mode = KILL_CLIENT;
     else {
         ELOG("BUG: called with kill_mode=%s\n", kill_mode_str);
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     /* check if the match is empty, not if the result is empty */
@@ -899,36 +893,36 @@ char *cmd_kill(Match *current_match, char *kill_mode_str) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'exec [--no-startup-id] <command>'.
  *
  */
-char *cmd_exec(Match *current_match, char *nosn, char *command) {
+void cmd_exec(I3_CMD, char *nosn, char *command) {
     bool no_startup_id = (nosn != NULL);
 
     DLOG("should execute %s, no_startup_id = %d\n", command, no_startup_id);
     start_application(command, no_startup_id);
 
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'focus left|right|up|down'.
  *
  */
-char *cmd_focus_direction(Match *current_match, char *direction) {
+void cmd_focus_direction(I3_CMD, char *direction) {
     if (focused &&
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     DLOG("direction = *%s*\n", direction);
@@ -943,25 +937,26 @@ char *cmd_focus_direction(Match *current_match, char *direction) {
         tree_next('n', VERT);
     else {
         ELOG("Invalid focus direction (%s)\n", direction);
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'focus tiling|floating|mode_toggle'.
  *
  */
-char *cmd_focus_window_mode(Match *current_match, char *window_mode) {
+void cmd_focus_window_mode(I3_CMD, char *window_mode) {
     if (focused &&
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     DLOG("window_mode = %s\n", window_mode);
@@ -985,22 +980,22 @@ char *cmd_focus_window_mode(Match *current_match, char *window_mode) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'focus parent|child'.
  *
  */
-char *cmd_focus_level(Match *current_match, char *level) {
+void cmd_focus_level(I3_CMD, char *level) {
     if (focused &&
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     DLOG("level = %s\n", level);
@@ -1009,23 +1004,23 @@ char *cmd_focus_level(Match *current_match, char *level) {
         level_up();
     else level_down();
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'focus'.
  *
  */
-char *cmd_focus(Match *current_match) {
+void cmd_focus(I3_CMD) {
     DLOG("current_match = %p\n", current_match);
     if (focused &&
         focused->type != CT_WORKSPACE &&
         focused->fullscreen_mode != CF_NONE) {
         LOG("Cannot change focus while in fullscreen mode.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     owindow *current;
@@ -1034,10 +1029,10 @@ char *cmd_focus(Match *current_match) {
         ELOG("You have to specify which window/container should be focused.\n");
         ELOG("Example: [class=\"urxvt\" title=\"irssi\"] focus\n");
 
-        char *json_output;
+        sasprintf(&(cmd_output->json_output),
-        sasprintf(&json_output, "{\"success\":false, \"error\":\"You have to "
+                  "{\"success\":false, \"error\":\"You have to "
                   "specify which window/container should be focused\"}");
-        return json_output;
+        return;
     }
 
     int count = 0;
@@ -1075,17 +1070,16 @@ char *cmd_focus(Match *current_match) {
         LOG("WARNING: Your criteria for the focus command matches %d containers, "
             "while only exactly one container can be focused at a time.\n", count);
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'fullscreen [global]'.
  *
  */
-char *cmd_fullscreen(Match *current_match, char *fullscreen_mode) {
+void cmd_fullscreen(I3_CMD, char *fullscreen_mode) {
     if (fullscreen_mode == NULL)
         fullscreen_mode = "output";
     DLOG("toggling fullscreen, mode = %s\n", fullscreen_mode);
@@ -1098,17 +1092,16 @@ char *cmd_fullscreen(Match *current_match, char *fullscreen_mode) {
         con_toggle_fullscreen(current->con, (strcmp(fullscreen_mode, "global") == 0 ? CF_GLOBAL : CF_OUTPUT));
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'move <direction> [<pixels> [px]]'.
  *
  */
-char *cmd_move_direction(Match *current_match, char *direction, char *move_px) {
+void cmd_move_direction(I3_CMD, char *direction, char *move_px) {
     // TODO: We could either handle this in the parser itself as a separate token (and make the stack typed) or we need a better way to convert a string to a number with error checking
     int px = atoi(move_px);
 
@@ -1132,19 +1125,18 @@ char *cmd_move_direction(Match *current_match, char *direction, char *move_px) {
                    (strcmp(direction, "left") == 0 ? D_LEFT :
                     (strcmp(direction, "up") == 0 ? D_UP :
                      D_DOWN))));
-        tree_render();
+        cmd_output->needs_tree_render = true;
     }
 
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'layout default|stacked|stacking|tabbed'.
  *
  */
-char *cmd_layout(Match *current_match, char *layout_str) {
+void cmd_layout(I3_CMD, char *layout_str) {
     if (strcmp(layout_str, "stacking") == 0)
         layout_str = "stacked";
     DLOG("changing layout to %s\n", layout_str);
@@ -1163,17 +1155,16 @@ char *cmd_layout(Match *current_match, char *layout_str) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementaiton of 'exit'.
  *
  */
-char *cmd_exit(Match *current_match) {
+void cmd_exit(I3_CMD) {
     LOG("Exiting due to user command.\n");
     exit(0);
 
@@ -1184,7 +1175,7 @@ char *cmd_exit(Match *current_match) {
  * Implementaiton of 'reload'.
  *
  */
-char *cmd_reload(Match *current_match) {
+void cmd_reload(I3_CMD) {
     LOG("reloading\n");
     kill_configerror_nagbar(false);
     load_configuration(conn, NULL, true);
@@ -1193,42 +1184,40 @@ char *cmd_reload(Match *current_match) {
     ipc_send_event("workspace", I3_IPC_EVENT_WORKSPACE, "{\"change\":\"reload\"}");
 
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementaiton of 'restart'.
  *
  */
-char *cmd_restart(Match *current_match) {
+void cmd_restart(I3_CMD) {
     LOG("restarting i3\n");
     i3_restart(false);
 
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementaiton of 'open'.
  *
  */
-char *cmd_open(Match *current_match) {
+void cmd_open(I3_CMD) {
     LOG("opening new container\n");
     Con *con = tree_open_con(NULL, NULL);
     con_focus(con);
-    char *json_output;
+    sasprintf(&(cmd_output->json_output),
-    sasprintf(&json_output, "{\"success\":true, \"id\":%ld}", (long int)con);
+              "{\"success\":true, \"id\":%ld}", (long int)con);
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
-    return json_output;
 }
 
 /*
  * Implementation of 'focus output <output>'.
  *
  */
-char *cmd_focus_output(Match *current_match, char *name) {
+void cmd_focus_output(I3_CMD, char *name) {
     owindow *current;
 
     DLOG("name = %s\n", name);
@@ -1247,27 +1236,30 @@ char *cmd_focus_output(Match *current_match, char *name) {
 
     if (!output) {
         LOG("No such output found.\n");
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
     }
 
     /* get visible workspace on output */
     Con *ws = NULL;
     GREP_FIRST(ws, output_get_content(output->con), workspace_is_visible(child));
-    if (!ws)
+    if (!ws) {
-        return sstrdup("{\"sucess\": false}");
+        cmd_output->json_output = sstrdup("{\"sucess\": false}");
+        return;
+    }
 
     workspace_show(ws);
-    tree_render();
 
+    cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'move scratchpad'.
  *
  */
-char *cmd_move_scratchpad(Match *current_match) {
+void cmd_move_scratchpad(I3_CMD) {
     DLOG("should move window to scratchpad\n");
     owindow *current;
 
@@ -1278,17 +1270,16 @@ char *cmd_move_scratchpad(Match *current_match) {
         scratchpad_move(current->con);
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }
 
 /*
  * Implementation of 'scratchpad show'.
  *
  */
-char *cmd_scratchpad_show(Match *current_match) {
+void cmd_scratchpad_show(I3_CMD) {
     DLOG("should show scratchpad window\n");
     owindow *current;
 
@@ -1301,8 +1292,7 @@ char *cmd_scratchpad_show(Match *current_match) {
         }
     }
 
-    tree_render();
+    cmd_output->needs_tree_render = true;
-
     // XXX: default reply for now, make this a better reply
-    return sstrdup("{\"success\": true}");
+    cmd_output->json_output = sstrdup("{\"success\": true}");
 }

src/commands_parser.c

@@ -173,7 +173,8 @@ static cmdp_state state;
 #ifndef TEST_PARSER
 static Match current_match;
 #endif
-static char *json_output;
+static struct CommandResult subcommand_output;
+static struct CommandResult command_output;
 
 #include "GENERATED_call.h"
 
@@ -182,7 +183,24 @@ 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);
-        json_output = GENERATED_call(token->extra.call_identifier);
+        subcommand_output.json_output = NULL;
+        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)
+            command_output.needs_tree_render = true;
         clear_stack();
         return;
     }
@@ -194,10 +212,11 @@ static void next_state(const cmdp_token *token) {
 }
 
 /* TODO: Return parsing errors via JSON. */
-char *parse_command(const char *input) {
+struct CommandResult *parse_command(const char *input) {
     DLOG("new parser handling: %s\n", input);
     state = INITIAL;
-    json_output = NULL;
+    command_output.json_output = sstrdup("[");
+    command_output.needs_tree_render = false;
 
     const char *walk = input;
     const size_t len = strlen(input);
@@ -207,7 +226,7 @@ char *parse_command(const char *input) {
 
     // TODO: make this testable
 #ifndef TEST_PARSER
-    cmd_criteria_init(&current_match);
+    cmd_criteria_init(&current_match, &subcommand_output);
 #endif
 
     /* The "<=" operator is intentional: We also handle the terminating 0-byte
@@ -312,7 +331,7 @@ char *parse_command(const char *input) {
                     // TODO: make this testable
 #ifndef TEST_PARSER
                     if (*walk == '\0' || *walk == ';')
-                        cmd_criteria_init(&current_match);
+                        cmd_criteria_init(&current_match, &subcommand_output);
 #endif
                     walk++;
                     break;
@@ -377,8 +396,13 @@ char *parse_command(const char *input) {
         }
     }
 
-    DLOG("json_output = %s\n", json_output);
+    char *buffer;
-    return json_output;
+    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);
+    DLOG("command_output.needs_tree_render = %d\n", command_output.needs_tree_render);
+    return &command_output;
 }
 
 /*******************************************************************************

src/handlers.c

@@ -118,9 +118,12 @@ static void handle_key_press(xcb_key_press_event_t *event) {
         }
     }
 
-    char *json_result = parse_command(bind->command);
+    struct CommandResult *command_output = parse_command(bind->command);
-    FREE(json_result);
+
-    return;
+    if (command_output->needs_tree_render)
+        tree_render();
+
+    free(command_output->json_output);
 }
 
 /*

src/ipc.c

@@ -119,16 +119,18 @@ IPC_HANDLER(command) {
     char *command = scalloc(message_size + 1);
     strncpy(command, (const char*)message, message_size);
     LOG("IPC: received: *%s*\n", command);
-    char *reply = parse_command((const char*)command);
+    struct CommandResult *command_output = parse_command((const char*)command);
-    char *save_reply = reply;
     free(command);
 
-    /* If no reply was provided, we just use the default success message */
+    if (command_output->needs_tree_render)
-    if (reply == NULL)
+        tree_render();
-        reply = "{\"success\":true}";
-    ipc_send_message(fd, strlen(reply), I3_IPC_REPLY_TYPE_COMMAND, (const uint8_t*)reply);
 
-    FREE(save_reply);
+    /* 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);
+
+    free(command_output->json_output);
 }
 
 static void dump_rect(yajl_gen gen, const char *name, Rect r) {

testcases/lib/i3test.pm

@@ -247,7 +247,7 @@ sub open_empty_con {
     my ($i3) = @_;
 
     my $reply = $i3->command('open')->recv;
-    return $reply->{id};
+    return $reply->[0]->{id};
 }
 
 sub get_workspace_names {