From 679a5de8cfb9211afb1a1dd882e53bd672084754 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Tue, 15 Jul 2014 10:15:04 +0200 Subject: [PATCH] Bugfix: properly restore workspace containers (Thanks vals) fixes #1306 --- include/load_layout.h | 23 ++- src/commands.c | 31 +++- src/load_layout.c | 112 +++++++++++++- testcases/t/234-layout-restore-output.t | 186 ++++++++++++++++++++++++ 4 files changed, 339 insertions(+), 13 deletions(-) create mode 100644 testcases/t/234-layout-restore-output.t diff --git a/include/load_layout.h b/include/load_layout.h index a598c8c6..8736a50c 100644 --- a/include/load_layout.h +++ b/include/load_layout.h @@ -2,7 +2,7 @@ * vim:ts=4:sw=4:expandtab * * i3 - an improved dynamic tiling window manager - * © 2009-2011 Michael Stapelberg and contributors (see also: LICENSE) + * © 2009-2014 Michael Stapelberg and contributors (see also: LICENSE) * * load_layout.c: Restore (parts of) the layout, for example after an inplace * restart. @@ -10,4 +10,25 @@ */ #pragma once +typedef enum { + // We could not determine the content of the JSON file. This typically + // means it’s unreadable or contains garbage. + JSON_CONTENT_UNKNOWN = 0, + + // The JSON file contains a “normal” container, i.e. a container to be + // appended to an existing workspace (or split container!). + JSON_CONTENT_CON = 1, + + // The JSON file contains a workspace container, which needs to be appended + // to the output (next to the other workspaces) with special care to avoid + // naming conflicts and ensuring that the workspace _has_ a name. + JSON_CONTENT_WORKSPACE = 2, +} json_content_t; + +/* Parses the given JSON file until it encounters the first “type” property to + * determine whether the file contains workspaces or regular containers, which + * is important to know when deciding where (and how) to append the contents. + * */ +json_content_t json_determine_content(const char *filename); + void tree_append_json(Con *con, const char *filename, char **errormsg); diff --git a/src/commands.c b/src/commands.c index bb44ffde..73db2802 100644 --- a/src/commands.c +++ b/src/commands.c @@ -882,15 +882,27 @@ void cmd_nop(I3_CMD, char *comment) { */ void cmd_append_layout(I3_CMD, char *path) { LOG("Appending layout \"%s\"\n", path); + + json_content_t content = json_determine_content(path); + LOG("JSON content = %d\n", content); + if (content == JSON_CONTENT_UNKNOWN) { + ELOG("Could not determine the contents of \"%s\", not loading.\n", path); + ysuccess(false); + return; + } + Con *parent = focused; - /* We need to append the layout to a split container, since a leaf - * container must not have any children (by definition). - * Note that we explicitly check for workspaces, since they are okay for - * this purpose, but con_accepts_window() returns false for workspaces. */ - while (parent->type != CT_WORKSPACE && !con_accepts_window(parent)) - parent = parent->parent; - DLOG("Appending to parent=%p instead of focused=%p\n", - parent, focused); + if (content == JSON_CONTENT_WORKSPACE) { + parent = output_get_content(con_get_output(parent)); + } else { + /* We need to append the layout to a split container, since a leaf + * container must not have any children (by definition). + * Note that we explicitly check for workspaces, since they are okay for + * this purpose, but con_accepts_window() returns false for workspaces. */ + while (parent->type != CT_WORKSPACE && !con_accepts_window(parent)) + parent = parent->parent; + } + DLOG("Appending to parent=%p instead of focused=%p\n", parent, focused); char *errormsg = NULL; tree_append_json(parent, path, &errormsg); if (errormsg != NULL) { @@ -914,6 +926,9 @@ void cmd_append_layout(I3_CMD, char *path) { restore_open_placeholder_windows(parent); + if (content == JSON_CONTENT_WORKSPACE) + ipc_send_event("workspace", I3_IPC_EVENT_WORKSPACE, "{\"change\":\"restored\"}"); + cmd_output->needs_tree_render = true; } diff --git a/src/load_layout.c b/src/load_layout.c index 494dbd06..6fcd02df 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -52,11 +52,13 @@ static int json_start_map(void *ctx) { DLOG("New floating_node\n"); Con *ws = con_get_workspace(json_node); json_node = con_new_skeleton(NULL, NULL); + json_node->name = NULL; json_node->parent = ws; DLOG("Parent is workspace = %p\n", ws); } else { Con *parent = json_node; json_node = con_new_skeleton(NULL, NULL); + json_node->name = NULL; json_node->parent = parent; } } @@ -84,6 +86,40 @@ static int json_end_map(void *ctx) { } } + if (json_node->type == CT_WORKSPACE) { + /* Ensure the workspace has a name. */ + DLOG("Attaching workspace. name = %s\n", json_node->name); + if (json_node->name == NULL || strcmp(json_node->name, "") == 0) { + json_node->name = sstrdup("unnamed"); + } + + /* Prevent name clashes when appending a workspace, e.g. when the + * user tries to restore a workspace called “1” but already has a + * workspace called “1”. */ + Con *output; + Con *workspace = NULL; + TAILQ_FOREACH (output, &(croot->nodes_head), nodes) + GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, json_node->name)); + char *base = sstrdup(json_node->name); + int cnt = 1; + while (workspace != NULL) { + FREE(json_node->name); + asprintf(&(json_node->name), "%s_%d", base, cnt++); + workspace = NULL; + TAILQ_FOREACH (output, &(croot->nodes_head), nodes) + GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, json_node->name)); + } + free(base); + + /* Set num accordingly so that i3bar will properly sort it. */ + json_node->num = ws_name_to_number(json_node->name); + } else { + // TODO: remove this in the “next” branch. + if (json_node->name == NULL || strcmp(json_node->name, "") == 0) { + json_node->name = sstrdup("#ff0000"); + } + } + LOG("attaching\n"); con_attach(json_node, json_node->parent, true); LOG("Creating window\n"); @@ -390,26 +426,94 @@ static int json_double(void *ctx, double val) { return 1; } +static json_content_t content_result; + +static int json_determine_content_string(void *ctx, const unsigned char *val, size_t len) { + if (strcasecmp(last_key, "type") != 0) + return 1; + + DLOG("string = %.*s, last_key = %s\n", (int)len, val, last_key); + if (strncasecmp((const char *)val, "workspace", len) == 0) + content_result = JSON_CONTENT_WORKSPACE; + return 0; +} + +/* Parses the given JSON file until it encounters the first “type” property to + * determine whether the file contains workspaces or regular containers, which + * is important to know when deciding where (and how) to append the contents. + * */ +json_content_t json_determine_content(const char *filename) { + FILE *f; + if ((f = fopen(filename, "r")) == NULL) { + ELOG("Cannot open file \"%s\"\n", filename); + return JSON_CONTENT_UNKNOWN; + } + struct stat stbuf; + if (fstat(fileno(f), &stbuf) != 0) { + ELOG("Cannot fstat() \"%s\"\n", filename); + fclose(f); + return JSON_CONTENT_UNKNOWN; + } + char *buf = smalloc(stbuf.st_size); + int n = fread(buf, 1, stbuf.st_size, f); + if (n != stbuf.st_size) { + ELOG("File \"%s\" could not be read entirely, not loading.\n", filename); + fclose(f); + return JSON_CONTENT_UNKNOWN; + } + DLOG("read %d bytes\n", n); + // We default to JSON_CONTENT_CON because it is legal to not include + // “"type": "con"” in the JSON files for better readability. + content_result = JSON_CONTENT_CON; + yajl_gen g; + yajl_handle hand; + static yajl_callbacks callbacks = { + .yajl_string = json_determine_content_string, + .yajl_map_key = json_key, + }; + g = yajl_gen_alloc(NULL); + hand = yajl_alloc(&callbacks, NULL, (void *)g); + /* Allowing comments allows for more user-friendly layout files. */ + yajl_config(hand, yajl_allow_comments, true); + /* Allow multiple values, i.e. multiple nodes to attach */ + yajl_config(hand, yajl_allow_multiple_values, true); + yajl_status stat; + setlocale(LC_NUMERIC, "C"); + stat = yajl_parse(hand, (const unsigned char *)buf, n); + if (stat != yajl_status_ok && stat != yajl_status_client_canceled) { + unsigned char *str = yajl_get_error(hand, 1, (const unsigned char *)buf, n); + ELOG("JSON parsing error: %s\n", str); + yajl_free_error(hand, str); + } + + setlocale(LC_NUMERIC, ""); + yajl_complete_parse(hand); + + fclose(f); + + return content_result; +} + void tree_append_json(Con *con, const char *filename, char **errormsg) { FILE *f; if ((f = fopen(filename, "r")) == NULL) { - LOG("Cannot open file \"%s\"\n", filename); + ELOG("Cannot open file \"%s\"\n", filename); return; } struct stat stbuf; if (fstat(fileno(f), &stbuf) != 0) { - LOG("Cannot fstat() the file\n"); + ELOG("Cannot fstat() \"%s\"\n", filename); fclose(f); return; } char *buf = smalloc(stbuf.st_size); int n = fread(buf, 1, stbuf.st_size, f); if (n != stbuf.st_size) { - LOG("File \"%s\" could not be read entirely, not loading.\n", filename); + ELOG("File \"%s\" could not be read entirely, not loading.\n", filename); fclose(f); return; } - LOG("read %d bytes\n", n); + DLOG("read %d bytes\n", n); yajl_gen g; yajl_handle hand; static yajl_callbacks callbacks = { diff --git a/testcases/t/234-layout-restore-output.t b/testcases/t/234-layout-restore-output.t new file mode 100644 index 00000000..b75f4be5 --- /dev/null +++ b/testcases/t/234-layout-restore-output.t @@ -0,0 +1,186 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • http://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • http://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • http://build.i3wm.org/docs/ipc.html +# (or docs/ipc) +# +# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf +# (unless you are already familiar with Perl) +# +# Verifies that entire outputs can be saved and restored properly by i3. +# Ticket: #1306 +# Bug still in: 4.8-26-gf96ec19 +use i3test; +use File::Temp qw(tempfile); +use List::MoreUtils qw(uniq); +use IO::Handle; + +my $ws = fresh_workspace; + +################################################################################ +# Append a new workspace with a name. +################################################################################ + +ok(!workspace_exists('ws_new'), 'workspace "ws_new" does not exist yet'); + +my ($fh, $filename) = tempfile(UNLINK => 1); +print $fh <<'EOT'; +// vim:ts=4:sw=4:et +{ + // workspace with 1 children + "border": "pixel", + "floating": "auto_off", + "layout": "splith", + "percent": null, + "type": "workspace", + "name": "ws_new", + "nodes": [ + { + "border": "pixel", + "floating": "auto_off", + "geometry": { + "height": 268, + "width": 484, + "x": 0, + "y": 0 + }, + "name": "vals@w00t: ~", + "percent": 1, + "swallows": [ + { + // "class": "^URxvt$", + // "instance": "^urxvt$", + // "title": "^vals\\@w00t\\:\\ \\~$" + } + ], + "type": "con" + } + ] +} +EOT +$fh->flush; +cmd "append_layout $filename"; + +ok(workspace_exists('ws_new'), 'workspace "ws_new" exists now'); + +does_i3_live; + +close($fh); + +################################################################################ +# Append a new workspace with a name that clashes with an existing workspace. +################################################################################ + +my @old_workspaces = @{get_workspace_names()}; + +cmd "append_layout $filename"; + +my @new_workspaces = @{get_workspace_names()}; +cmp_ok(scalar @new_workspaces, '>', scalar @old_workspaces, 'more workspaces than before'); + +my %created_workspaces = map { ($_, 1) } @new_workspaces; +delete $created_workspaces{$_} for @old_workspaces; +diag('created workspaces = ' . Dumper(keys %created_workspaces)); +cmp_ok(scalar keys %created_workspaces, '>', 0, 'new workspaces appeared'); + +################################################################################ +# Append a new workspace without a name. +################################################################################ + +ok(!workspace_exists('unnamed'), 'workspace "unnamed" does not exist yet'); + +($fh, $filename) = tempfile(UNLINK => 1); +print $fh <<'EOT'; +// vim:ts=4:sw=4:et +{ + // workspace with 1 children + "border": "pixel", + "floating": "auto_off", + "layout": "splith", + "percent": null, + "type": "workspace", + "nodes": [ + { + "border": "pixel", + "floating": "auto_off", + "geometry": { + "height": 268, + "width": 484, + "x": 0, + "y": 0 + }, + "name": "vals@w00t: ~", + "percent": 1, + "swallows": [ + { + // "class": "^URxvt$", + // "instance": "^urxvt$", + // "title": "^vals\\@w00t\\:\\ \\~$" + } + ], + "type": "con" + } + ] +} +EOT +$fh->flush; +cmd "append_layout $filename"; + +ok(workspace_exists('unnamed'), 'workspace "unnamed" exists now'); + +################################################################################ +# Append a workspace with a numeric name, ensure it has ->num set. +################################################################################ + +ok(!workspace_exists('4'), 'workspace "4" does not exist yet'); + +($fh, $filename) = tempfile(UNLINK => 1); +print $fh <<'EOT'; +// vim:ts=4:sw=4:et +{ + // workspace with 1 children + "border": "pixel", + "floating": "auto_off", + "layout": "splith", + "percent": null, + "type": "workspace", + "name": "4", + "nodes": [ + { + "border": "pixel", + "floating": "auto_off", + "geometry": { + "height": 268, + "width": 484, + "x": 0, + "y": 0 + }, + "name": "vals@w00t: ~", + "percent": 1, + "swallows": [ + { + // "class": "^URxvt$", + // "instance": "^urxvt$", + // "title": "^vals\\@w00t\\:\\ \\~$" + } + ], + "type": "con" + } + ] +} +EOT +$fh->flush; +cmd "append_layout $filename"; + +ok(workspace_exists('4'), 'workspace "4" exists now'); +my $ws = get_ws("4"); +is($ws->{num}, 4, 'workspace number is 4'); + +done_testing;