From e114b3dba2485606cf5a3d044c0de8430c9f80b4 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Tue, 7 Feb 2012 17:38:21 -0500 Subject: [PATCH] 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 | 10 +- include/commands.h | 73 ++++----- include/commands_parser.h | 19 ++- src/assignments.c | 10 +- src/commands.c | 302 ++++++++++++++++++------------------- src/commands_parser.c | 40 ++++- src/handlers.c | 9 +- src/ipc.c | 14 +- testcases/lib/i3test.pm | 2 +- 9 files changed, 261 insertions(+), 218 deletions(-) diff --git a/generate-command-parser.pl b/generate-command-parser.pl index 175d7101..993c64fc 100755 --- a/generate-command-parser.pl +++ b/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 ' char *output = NULL;'; +say $callfh 'static void GENERATED_call(const int call_identifier, struct CommandResult *result) {'; 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/\(/(¤t_match/; + $real_cmd =~ s/\(/(¤t_match, result/; } else { - $real_cmd =~ s/\(/(¤t_match, /; + $real_cmd =~ s/\(/(¤t_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); diff --git a/include/commands.h b/include/commands.h index 7b2fb88b..2b69422d 100644 --- a/include/commands.h +++ b/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 '. * */ -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 [ px] [or 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 '. * */ -char *cmd_nop(Match *current_match, char *comment); +void cmd_nop(I3_CMD, char *comment); /** * Implementation of 'append_layout '. * */ -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 ' * */ -char *cmd_workspace_name(Match *current_match, char *name); +void cmd_workspace_name(I3_CMD, char *name); /** * Implementation of 'mark ' * */ -char *cmd_mark(Match *current_match, char *mark); +void cmd_mark(I3_CMD, char *mark); /** * Implementation of 'mode '. * */ -char *cmd_mode(Match *current_match, char *mode); +void cmd_mode(I3_CMD, char *mode); /** * Implementation of 'move [window|container] [to] output '. * */ -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] '. * */ -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] '. * */ -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 [ [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 '. * */ -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 diff --git a/include/commands_parser.h b/include/commands_parser.h index 77569ec5..5a4472d8 100644 --- a/include/commands_parser.h +++ b/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 diff --git a/src/assignments.c b/src/assignments.c index 05da7554..eae87272 100644 --- a/src/assignments.c +++ b/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); - FREE(full_command); - FREE(json_result); + struct CommandResult *command_output = parse_command(full_command); + free(full_command); + + if (command_output->needs_tree_render) + tree_render(); + + free(command_output->json_output); } /* Store that we ran this assignment to not execute it again */ diff --git a/src/commands.c b/src/commands.c index da8bccba..8fc80a19 100644 --- a/src/commands.c +++ b/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 '. * */ -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) - return sstrdup("{\"sucess\": false}"); + if (match_is_empty(current_match) && focused->type == CT_WORKSPACE) { + 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 [ px] [or 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 [ px] [or 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 '. * */ -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 '. * */ -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 ' * */ -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 ' * */ -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 '. * */ -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 '. * */ -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) - return sstrdup("{\"sucess\": false}"); + if (!ws) { + 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] '. * */ -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] '. * */ -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(&json_output, "{\"success\":false, \"error\":\"You have to " + sasprintf(&(cmd_output->json_output), + "{\"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 [ [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(&json_output, "{\"success\":true, \"id\":%ld}", (long int)con); + sasprintf(&(cmd_output->json_output), + "{\"success\":true, \"id\":%ld}", (long int)con); - tree_render(); - - return json_output; + cmd_output->needs_tree_render = true; } /* * Implementation of 'focus 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) - return sstrdup("{\"sucess\": false}"); + if (!ws) { + 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}"); } diff --git a/src/commands_parser.c b/src/commands_parser.c index cdfafee2..f0459220 100644 --- a/src/commands_parser.c +++ b/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(¤t_match); + cmd_criteria_init(¤t_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(¤t_match); + cmd_criteria_init(¤t_match, &subcommand_output); #endif walk++; break; @@ -377,8 +396,13 @@ char *parse_command(const char *input) { } } - DLOG("json_output = %s\n", json_output); - return json_output; + 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); + 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 ac57e0a4..596d15a6 100644 --- a/src/handlers.c +++ b/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); - FREE(json_result); - return; + struct CommandResult *command_output = parse_command(bind->command); + + if (command_output->needs_tree_render) + tree_render(); + + free(command_output->json_output); } /* diff --git a/src/ipc.c b/src/ipc.c index dee68e5f..3733034d 100644 --- a/src/ipc.c +++ b/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); - char *save_reply = reply; + struct CommandResult *command_output = parse_command((const char*)command); free(command); + if (command_output->needs_tree_render) + tree_render(); + /* If no reply was provided, we just use the default success message */ - if (reply == NULL) - reply = "{\"success\":true}"; - ipc_send_message(fd, strlen(reply), I3_IPC_REPLY_TYPE_COMMAND, (const uint8_t*)reply); + ipc_send_message(fd, strlen(command_output->json_output), + I3_IPC_REPLY_TYPE_COMMAND, + (const uint8_t*)command_output->json_output); - FREE(save_reply); + free(command_output->json_output); } static void dump_rect(yajl_gen gen, const char *name, Rect r) { diff --git a/testcases/lib/i3test.pm b/testcases/lib/i3test.pm index 8204f842..979c60ac 100644 --- a/testcases/lib/i3test.pm +++ b/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 { -- 2.39.2