i3 - improved tiling WM

Reviews
Bugs

Refactor parse_command

Patch status: merged

Patch by Tony Crisci

Long description:

parse_command returns a struct that contains useful information about
the result of a command as a whole (instead of the intermediate
representation used during parsing).

parse_command now requires the caller to allocate the yajl_gen used for
generating a json reply. This is passed as the second parameter to
parse_command. If NULL is passed, no json reply will be generated.

To apply this patch, use:
curl http://cr.i3wm.org/patch/560/raw.patch | git am

b/include/commands_parser.h

@@ -17,7 +17,7 @@
  * internally use this struct when calling cmd_floating and cmd_border.
  */
 struct CommandResultIR {
-    /* The JSON generator to append a reply to. */
+    /* The JSON generator to append a reply to (may be NULL). */
     yajl_gen json_gen;
 
     /* The next state to transition to. Passed to the function so that we can
@@ -29,4 +29,31 @@ struct CommandResultIR {
     bool needs_tree_render;
 };
 
-struct CommandResultIR *parse_command(const char *input);
+typedef struct CommandResult CommandResult;
+
+/**
+ * A struct that contains useful information about the result of a command as a
+ * whole (e.g. a compound command like "floating enable, border none").
+ * needs_tree_render is true if needs_tree_render of any individual command was
+ * true.
+ */
+struct CommandResult {
+    bool parse_error;
+    /* the error_message is currently only set for parse errors */
+    char *error_message;
+    bool needs_tree_render;
+};
+
+/**
+ * Parses and executes the given command. If a caller-allocated yajl_gen is
+ * passed, a json reply will be generated in the format specified by the ipc
+ * protocol. Pass NULL if no json reply is required.
+ *
+ * Free the returned CommandResult with command_result_free().
+ */
+CommandResult *parse_command(const char *input, yajl_gen gen);
+
+/**
+ * Frees a CommandResult
+ */
+void command_result_free(CommandResult *result);

b/src/assignments.c

@@ -45,13 +45,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);
-            struct CommandResultIR *command_output = parse_command(full_command);
+            CommandResult *result = parse_command(full_command, NULL);
             free(full_command);
 
-            if (command_output->needs_tree_render)
+            if (result->needs_tree_render)
                 needs_tree_render = true;
 
-            yajl_gen_free(command_output->json_gen);
+            command_result_free(result);
         }
 
         /* Store that we ran this assignment to not execute it again */

b/src/commands.c

@@ -16,21 +16,25 @@
 #include "shmlog.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 y(x, ...) (cmd_output->json_gen != NULL ? yajl_gen_ ## x (cmd_output->json_gen, ##__VA_ARGS__) : 0)
+#define ystr(str) (cmd_output->json_gen != NULL ? yajl_gen_string(cmd_output->json_gen, (unsigned char*)str, strlen(str)) : 0)
 #define ysuccess(success) do { \
-    y(map_open); \
-    ystr("success"); \
-    y(bool, success); \
-    y(map_close); \
+    if (cmd_output->json_gen != NULL) { \
+        y(map_open); \
+        ystr("success"); \
+        y(bool, success); \
+        y(map_close); \
+    } \
 } while (0)
 #define yerror(message) do { \
-    y(map_open); \
-    ystr("success"); \
-    y(bool, false); \
-    ystr("error"); \
-    ystr(message); \
-    y(map_close); \
+    if (cmd_output->json_gen != NULL) { \
+        y(map_open); \
+        ystr("success"); \
+        y(bool, false); \
+        ystr("error"); \
+        ystr(message); \
+        y(map_close); \
+    } \
 } while (0)
 
 /** When the command did not include match criteria (!), we use the currently

b/src/commands_parser.c

@@ -35,8 +35,8 @@
 #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))
+#define y(x, ...) (command_output.json_gen != NULL ? yajl_gen_ ## x (command_output.json_gen, ##__VA_ARGS__) : 0)
+#define ystr(str) (command_output.json_gen != NULL ? yajl_gen_string(command_output.json_gen, (unsigned char*)str, strlen(str)) : 0)
 
 /*******************************************************************************
  * The data structures used for parsing. Essentially the current state and a
@@ -205,12 +205,20 @@ static void next_state(const cmdp_token *token) {
     }
 }
 
-struct CommandResultIR *parse_command(const char *input) {
+/*
+ * Parses and executes the given command. If a caller-allocated yajl_gen is
+ * passed, a json reply will be generated in the format specified by the ipc
+ * protocol. Pass NULL if no json reply is required.
+ *
+ * Free the returned CommandResult with command_result_free().
+ */
+CommandResult *parse_command(const char *input, yajl_gen gen) {
     DLOG("COMMAND: *%s*\n", input);
     state = INITIAL;
+    CommandResult *result = scalloc(sizeof(CommandResult));
 
     /* A YAJL JSON generator used for formatting replies. */
-    command_output.json_gen = yajl_gen_alloc(NULL);
+    command_output.json_gen = (gen != NULL ? gen : NULL);
 
     y(array_open);
     command_output.needs_tree_render = false;
@@ -378,6 +386,9 @@ struct CommandResultIR *parse_command(const char *input) {
             ELOG("Your command: %s\n", input);
             ELOG("              %s\n", position);
 
+            result->parse_error = true;
+            result->error_message = errormessage;
+
             /* Format this error message as a JSON reply. */
             y(map_open);
             ystr("success");
@@ -396,7 +407,6 @@ struct CommandResultIR *parse_command(const char *input) {
             y(map_close);
 
             free(position);
-            free(errormessage);
             clear_stack();
             break;
         }
@@ -404,7 +414,19 @@ struct CommandResultIR *parse_command(const char *input) {
 
     y(array_close);
 
-    return &command_output;
+    result->needs_tree_render = command_output.needs_tree_render;
+    return result;
+}
+
+/*
+ * Frees a CommandResult
+ */
+void command_result_free(CommandResult *result) {
+    if (result == NULL)
+        return;
+
+    FREE(result->error_message);
+    FREE(result);
 }
 
 /*******************************************************************************
@@ -442,6 +464,12 @@ int main(int argc, char *argv[]) {
         fprintf(stderr, "Syntax: %s <command>\n", argv[0]);
         return 1;
     }
-    parse_command(argv[1]);
+    yajl_gen gen = yajl_gen_alloc(NULL);
+
+    CommandResult *result = parse_command(argv[1], gen);
+
+    command_result_free(result);
+
+    yajl_gen_free(gen);
 }
 #endif

b/src/ipc.c

@@ -117,20 +117,24 @@ IPC_HANDLER(command) {
     char *command = scalloc(message_size + 1);
     strncpy(command, (const char*)message, message_size);
     LOG("IPC: received: *%s*\n", command);
-    struct CommandResultIR *command_output = parse_command((const char*)command);
+    yajl_gen gen = yajl_gen_alloc(NULL);
+
+    CommandResult *result = parse_command((const char*)command, gen);
     free(command);
 
-    if (command_output->needs_tree_render)
+    if (result->needs_tree_render)
         tree_render();
 
+    command_result_free(result);
+
     const unsigned char *reply;
     ylength length;
-    yajl_gen_get_buf(command_output->json_gen, &reply, &length);
+    yajl_gen_get_buf(gen, &reply, &length);
 
     ipc_send_message(fd, length, I3_IPC_REPLY_TYPE_COMMAND,
                      (const uint8_t*)reply);
 
-    yajl_gen_free(command_output->json_gen);
+    yajl_gen_free(gen);
 }
 
 static void dump_rect(yajl_gen gen, const char *name, Rect r) {

b/src/key_press.c

@@ -72,19 +72,23 @@ void handle_key_press(xcb_key_press_event_t *event) {
     if (bind == NULL)
         return;
 
+    yajl_gen gen = yajl_gen_alloc(NULL);
+
     char *command_copy = sstrdup(bind->command);
-    struct CommandResultIR *command_output = parse_command(command_copy);
+    CommandResult *result = parse_command(command_copy, gen);
     free(command_copy);
 
-    if (command_output->needs_tree_render)
+    if (result->needs_tree_render)
         tree_render();
 
+    command_result_free(result);
+
     /* We parse the JSON reply to figure out whether there was an error
      * ("success" being false in on of the returned dictionaries). */
     const unsigned char *reply;
     size_t length;
     yajl_handle handle = yajl_alloc(&command_error_callbacks, NULL, NULL);
-    yajl_gen_get_buf(command_output->json_gen, &reply, &length);
+    yajl_gen_get_buf(gen, &reply, &length);
 
     current_nesting_level = 0;
     parse_error_key = false;
@@ -116,5 +120,5 @@ void handle_key_press(xcb_key_press_event_t *event) {
 
     yajl_free(handle);
 
-    yajl_gen_free(command_output->json_gen);
+    yajl_gen_free(gen);
 }

134	@@ -35,8 +35,8 @@
135	#include "all.h"
136
137	// Macros to make the YAJL API a bit easier to use.
138	-#define y(x, ...) yajl_gen_ ## x (command_output.json_gen, ##__VA_ARGS__)
139	-#define ystr(str) yajl_gen_string(command_output.json_gen, (unsigned char*)str, strlen(str))
140	+#define y(x, ...) (command_output.json_gen != NULL ? yajl_gen_ ## x (command_output.json_gen, ##__VA_ARGS__) : 0)
141	+#define ystr(str) (command_output.json_gen != NULL ? yajl_gen_string(command_output.json_gen, (unsigned char*)str, strlen(str)) : 0)
142
143	/*******************************************************************************
144	* The data structures used for parsing. Essentially the current state and a
145	@@ -205,12 +205,20 @@ static void next_state(const cmdp_token *token) {
146	}
147	}
148
149	-struct CommandResultIR parse_command(const char input) {
150	+/*
151	+ * Parses and executes the given command. If a caller-allocated yajl_gen is
152	+ * passed, a json reply will be generated in the format specified by the ipc
153	+ * protocol. Pass NULL if no json reply is required.
154	+ *
155	+ * Free the returned CommandResult with command_result_free().
156	+ */
157	+CommandResult parse_command(const char input, yajl_gen gen) {
158	DLOG("COMMAND: %s\n", input);
159	state = INITIAL;
160	+ CommandResult *result = scalloc(sizeof(CommandResult));
161
162	/* A YAJL JSON generator used for formatting replies. */
163	- command_output.json_gen = yajl_gen_alloc(NULL);
164	+ command_output.json_gen = (gen != NULL ? gen : NULL);
165
166	y(array_open);
167	command_output.needs_tree_render = false;
168	@@ -378,6 +386,9 @@ struct CommandResultIR parse_command(const char input) {
169	ELOG("Your command: %s\n", input);
170	ELOG(" %s\n", position);
171
172	+ result->parse_error = true;
173	+ result->error_message = errormessage;
174	+
175	/* Format this error message as a JSON reply. */
176	y(map_open);
177	ystr("success");
178	@@ -396,7 +407,6 @@ struct CommandResultIR parse_command(const char input) {
179	y(map_close);
180
181	free(position);
182	- free(errormessage);
183	clear_stack();
184	break;
185	}
186	@@ -404,7 +414,19 @@ struct CommandResultIR parse_command(const char input) {
187
188	y(array_close);
189
190	- return &command_output;
191	+ result->needs_tree_render = command_output.needs_tree_render;
192	+ return result;
193	+}
194	+
195	+/*
196	+ * Frees a CommandResult
197	+ */
198	+void command_result_free(CommandResult *result) {
199	+ if (result == NULL)
200	+ return;
201	+
202	+ FREE(result->error_message);
203	+ FREE(result);
204	}
205
206	/*******************************************************************************
207	@@ -442,6 +464,12 @@ int main(int argc, char *argv[]) {
208	fprintf(stderr, "Syntax: %s <command>\n", argv[0]);
209	return 1;
210	}
211	- parse_command(argv[1]);
212	+ yajl_gen gen = yajl_gen_alloc(NULL);
213	+
214	+ CommandResult *result = parse_command(argv[1], gen);
215	+
216	+ command_result_free(result);
217	+
218	+ yajl_gen_free(gen);
219	}
220	#endif

225	@@ -117,20 +117,24 @@ IPC_HANDLER(command) {
226	char *command = scalloc(message_size + 1);
227	strncpy(command, (const char*)message, message_size);
228	LOG("IPC: received: %s\n", command);
229	- struct CommandResultIR command_output = parse_command((const char)command);
230	+ yajl_gen gen = yajl_gen_alloc(NULL);
231	+
232	+ CommandResult result = parse_command((const char)command, gen);
233	free(command);
234
235	- if (command_output->needs_tree_render)
236	+ if (result->needs_tree_render)
237	tree_render();
238
239	+ command_result_free(result);
240	+
241	const unsigned char *reply;
242	ylength length;
243	- yajl_gen_get_buf(command_output->json_gen, &reply, &length);
244	+ yajl_gen_get_buf(gen, &reply, &length);
245
246	ipc_send_message(fd, length, I3_IPC_REPLY_TYPE_COMMAND,
247	(const uint8_t*)reply);
248
249	- yajl_gen_free(command_output->json_gen);
250	+ yajl_gen_free(gen);
251	}
252
253	static void dump_rect(yajl_gen gen, const char *name, Rect r) {

258	@@ -72,19 +72,23 @@ void handle_key_press(xcb_key_press_event_t *event) {
259	if (bind == NULL)
260	return;
261
262	+ yajl_gen gen = yajl_gen_alloc(NULL);
263	+
264	char *command_copy = sstrdup(bind->command);
265	- struct CommandResultIR *command_output = parse_command(command_copy);
266	+ CommandResult *result = parse_command(command_copy, gen);
267	free(command_copy);
268
269	- if (command_output->needs_tree_render)
270	+ if (result->needs_tree_render)
271	tree_render();
272
273	+ command_result_free(result);
274	+
275	/* We parse the JSON reply to figure out whether there was an error
276	* ("success" being false in on of the returned dictionaries). */
277	const unsigned char *reply;
278	size_t length;
279	yajl_handle handle = yajl_alloc(&command_error_callbacks, NULL, NULL);
280	- yajl_gen_get_buf(command_output->json_gen, &reply, &length);
281	+ yajl_gen_get_buf(gen, &reply, &length);
282
283	current_nesting_level = 0;
284	parse_error_key = false;
285	@@ -116,5 +120,5 @@ void handle_key_press(xcb_key_press_event_t *event) {
286
287	yajl_free(handle);
288
289	- yajl_gen_free(command_output->json_gen);
290	+ yajl_gen_free(gen);
291	}

25	@@ -17,7 +17,7 @@
26	* internally use this struct when calling cmd_floating and cmd_border.
27	*/
28	struct CommandResultIR {
29	- /* The JSON generator to append a reply to. */
30	+ /* The JSON generator to append a reply to (may be NULL). */
31	yajl_gen json_gen;
32
33	/* The next state to transition to. Passed to the function so that we can
34	@@ -29,4 +29,31 @@ struct CommandResultIR {
35	bool needs_tree_render;
36	};
37
38	-struct CommandResultIR parse_command(const char input);
39	+typedef struct CommandResult CommandResult;
40	+
41	+/**
42	+ * A struct that contains useful information about the result of a command as a
43	+ * whole (e.g. a compound command like "floating enable, border none").
44	+ * needs_tree_render is true if needs_tree_render of any individual command was
45	+ * true.
46	+ */
47	+struct CommandResult {
48	+ bool parse_error;
49	+ /* the error_message is currently only set for parse errors */
50	+ char *error_message;
51	+ bool needs_tree_render;
52	+};
53	+
54	+/**
55	+ * Parses and executes the given command. If a caller-allocated yajl_gen is
56	+ * passed, a json reply will be generated in the format specified by the ipc
57	+ * protocol. Pass NULL if no json reply is required.
58	+ *
59	+ * Free the returned CommandResult with command_result_free().
60	+ */
61	+CommandResult parse_command(const char input, yajl_gen gen);
62	+
63	+/**
64	+ * Frees a CommandResult
65	+ */
66	+void command_result_free(CommandResult *result);

71	@@ -45,13 +45,13 @@ void run_assignments(i3Window *window) {
72	DLOG("execute command %s\n", current->dest.command);
73	char *full_command;
74	sasprintf(&full_command, "[id=\"%d\"] %s", window->id, current->dest.command);
75	- struct CommandResultIR *command_output = parse_command(full_command);
76	+ CommandResult *result = parse_command(full_command, NULL);
77	free(full_command);
78
79	- if (command_output->needs_tree_render)
80	+ if (result->needs_tree_render)
81	needs_tree_render = true;
82
83	- yajl_gen_free(command_output->json_gen);
84	+ command_result_free(result);
85	}
86
87	/* Store that we ran this assignment to not execute it again */

92	@@ -16,21 +16,25 @@
93	#include "shmlog.h"
94
95	// Macros to make the YAJL API a bit easier to use.
96	-#define y(x, ...) yajl_gen_ ## x (cmd_output->json_gen, ##__VA_ARGS__)
97	-#define ystr(str) yajl_gen_string(cmd_output->json_gen, (unsigned char*)str, strlen(str))
98	+#define y(x, ...) (cmd_output->json_gen != NULL ? yajl_gen_ ## x (cmd_output->json_gen, ##__VA_ARGS__) : 0)
99	+#define ystr(str) (cmd_output->json_gen != NULL ? yajl_gen_string(cmd_output->json_gen, (unsigned char*)str, strlen(str)) : 0)
100	#define ysuccess(success) do { \
101	- y(map_open); \
102	- ystr("success"); \
103	- y(bool, success); \
104	- y(map_close); \
105	+ if (cmd_output->json_gen != NULL) { \
106	+ y(map_open); \
107	+ ystr("success"); \
108	+ y(bool, success); \
109	+ y(map_close); \
110	+ } \
111	} while (0)
112	#define yerror(message) do { \
113	- y(map_open); \
114	- ystr("success"); \
115	- y(bool, false); \
116	- ystr("error"); \
117	- ystr(message); \
118	- y(map_close); \
119	+ if (cmd_output->json_gen != NULL) { \
120	+ y(map_open); \
121	+ ystr("success"); \
122	+ y(bool, false); \
123	+ ystr("error"); \
124	+ ystr(message); \
125	+ y(map_close); \
126	+ } \
127	} while (0)
128
129	/** When the command did not include match criteria (!), we use the currently