Make workspace_layout handle all cons at workspace level, not only the first one (+test)

This makes opening new windows on workspace level and moving windows to the
right/left more like in the old i3.

include/con.h

@@ -7,7 +7,7 @@
  * X11 IDs using x_con_init().
  *
  */
-Con *con_new(Con *parent);
+Con *con_new(Con *parent, i3Window *window);
 
 /**
  * Sets input focus to the given container. Will be updated in X11 in the next

include/tree.h

@@ -24,7 +24,7 @@ void tree_init();
  * Opens an empty container in the current container
  *
  */
-Con *tree_open_con(Con *con);
+Con *tree_open_con(Con *con, i3Window *window);
 
 /**
  * Splits (horizontally or vertically) the given container by creating a new

include/workspace.h

@@ -109,4 +109,16 @@ void workspace_update_urgent_flag(Con *ws);
  */
 void ws_force_orientation(Con *ws, orientation_t orientation);
 
+/**
+ * Called when a new con (with a window, not an empty or split con) should be
+ * attached to the workspace (for example when managing a new window or when
+ * moving an existing window to the workspace level).
+ *
+ * Depending on the workspace_layout setting, this function either returns the
+ * workspace itself (default layout) or creates a new stacked/tabbed con and
+ * returns that.
+ *
+ */
+Con *workspace_attach_to(Con *ws);
+
 #endif

src/cmdparse.y

@@ -443,7 +443,7 @@ open:
     TOK_OPEN
     {
         printf("opening new container\n");
-        Con *con = tree_open_con(NULL);
+        Con *con = tree_open_con(NULL, NULL);
         con_focus(con);
         asprintf(&json_output, "{\"success\":true, \"id\":%ld}", (long int)con);
 

src/con.c

@@ -32,11 +32,12 @@ static void con_on_remove_child(Con *con);
  * X11 IDs using x_con_init().
  *
  */
-Con *con_new(Con *parent) {
+Con *con_new(Con *parent, i3Window *window) {
     Con *new = scalloc(sizeof(Con));
     new->on_remove_child = con_on_remove_child;
     TAILQ_INSERT_TAIL(&all_cons, new, all_cons);
     new->type = CT_CON;
+    new->window = window;
     new->border_style = config.default_border;
     static int cnt = 0;
     DLOG("opening window %d\n", cnt);
@@ -59,19 +60,8 @@ Con *con_new(Con *parent) {
     TAILQ_INIT(&(new->focus_head));
     TAILQ_INIT(&(new->swallow_head));
 
-    if (parent != NULL) {
+    if (parent != NULL)
-        /* Set layout of ws if this is the first child of the ws and the user
+        con_attach(new, parent, false);
-         * wanted something different than the default layout. */
-        if (parent->type == CT_WORKSPACE &&
-            con_is_leaf(parent) &&
-            config.default_layout != L_DEFAULT) {
-            con_set_layout(new, config.default_layout);
-            con_attach(new, parent, false);
-            con_set_layout(parent, config.default_layout);
-        } else {
-            con_attach(new, parent, false);
-        }
-    }
 
     return new;
 }
@@ -91,6 +81,7 @@ void con_attach(Con *con, Con *parent, bool ignore_focus) {
     Con *loop;
     Con *current = NULL;
     struct nodes_head *nodes_head = &(parent->nodes_head);
+    struct focus_head *focus_head = &(parent->focus_head);
 
     /* Workspaces are handled differently: they need to be inserted at the
      * right position. */
@@ -134,6 +125,26 @@ void con_attach(Con *con, Con *parent, bool ignore_focus) {
             }
         }
 
+        /* When the container is not a split container (but contains a window)
+         * and is attached to a workspace, we check if the user configured a
+         * workspace_layout. This is done in workspace_attach_to, which will
+         * provide us with the container to which we should attach (either the
+         * workspace or a new split container with the configured
+         * workspace_layout).
+         */
+        if (con->window != NULL && parent->type == CT_WORKSPACE) {
+            DLOG("Parent is a workspace. Applying default layout...\n");
+            Con *target = workspace_attach_to(parent);
+
+            /* Attach the original con to this new split con instead */
+            nodes_head = &(target->nodes_head);
+            focus_head = &(target->focus_head);
+            con->parent = target;
+            current = NULL;
+
+            DLOG("done\n");
+        }
+
         /* Insert the container after the tiling container, if found.
          * When adding to a CT_OUTPUT, just append one after another. */
         if (current && parent->type != CT_OUTPUT) {
@@ -147,7 +158,7 @@ add_to_focus_head:
     /* We insert to the TAIL because con_focus() will correct this.
      * This way, we have the option to insert Cons without having
      * to focus them. */
-    TAILQ_INSERT_TAIL(&(parent->focus_head), con, focused);
+    TAILQ_INSERT_TAIL(focus_head, con, focused);
 }
 
 /*
@@ -818,7 +829,7 @@ void con_set_layout(Con *con, int layout) {
     if (con->type == CT_WORKSPACE) {
         DLOG("Creating new split container\n");
         /* 1: create a new split container */
-        Con *new = con_new(NULL);
+        Con *new = con_new(NULL, NULL);
         new->parent = con;
 
         /* 2: set the requested layout on the split con */

src/floating.c

@@ -35,7 +35,7 @@ void floating_enable(Con *con, bool automatic) {
             return;
         }
         /* TODO: refactor this with src/con.c:con_set_layout */
-        Con *new = con_new(NULL);
+        Con *new = con_new(NULL, NULL);
         new->parent = con;
         new->orientation = con->orientation;
 
@@ -76,7 +76,7 @@ void floating_enable(Con *con, bool automatic) {
 
     /* 2: create a new container to render the decoration on, add
      * it as a floating window to the workspace */
-    Con *nc = con_new(NULL);
+    Con *nc = con_new(NULL, NULL);
     /* we need to set the parent afterwards instead of passing it as an
      * argument to con_new() because nc would be inserted into the tiling layer
      * otherwise. */

src/load_layout.c

@@ -32,12 +32,12 @@ static int json_start_map(void *ctx) {
             if (last_key && strcasecmp(last_key, "floating_nodes") == 0) {
                 DLOG("New floating_node\n");
                 Con *ws = con_get_workspace(json_node);
-                json_node = con_new(NULL);
+                json_node = con_new(NULL, NULL);
                 json_node->parent = ws;
                 DLOG("Parent is workspace = %p\n", ws);
             } else {
                 Con *parent = json_node;
-                json_node = con_new(NULL);
+                json_node = con_new(NULL, NULL);
                 json_node->parent = parent;
             }
         }

src/manage.c

@@ -219,8 +219,8 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
                 nc = con_descend_focused(workspace_get(assignment->dest.workspace, NULL));
                 DLOG("focused on ws %s: %p / %s\n", assignment->dest.workspace, nc, nc->name);
                 if (nc->type == CT_WORKSPACE)
-                    nc = tree_open_con(nc);
+                    nc = tree_open_con(nc, cwindow);
-                else nc = tree_open_con(nc->parent);
+                else nc = tree_open_con(nc->parent, cwindow);
             }
         /* TODO: handle assignments with type == A_TO_OUTPUT */
         } else {
@@ -229,13 +229,13 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
                 LOG("using current container, focused = %p, focused->name = %s\n",
                                 focused, focused->name);
                 nc = focused;
-            } else nc = tree_open_con(NULL);
+            } else nc = tree_open_con(NULL, cwindow);
         }
     } else {
         /* M_BELOW inserts the new window as a child of the one which was
          * matched (e.g. dock areas) */
         if (match != NULL && match->insert_where == M_BELOW) {
-            nc = tree_open_con(nc);
+            nc = tree_open_con(nc, cwindow);
         }
     }
 

src/move.c

@@ -22,6 +22,23 @@ static void insert_con_into(Con *con, Con *target, position_t position) {
     con_detach(con);
     con_fix_percent(con->parent);
 
+    /* When moving to a workspace, we respect the user’s configured
+     * workspace_layout */
+    if (parent->type == CT_WORKSPACE) {
+        Con *split = workspace_attach_to(parent);
+        if (split != parent) {
+            DLOG("Got a new split con, using that one instead\n");
+            con->parent = split;
+            con_attach(con, split, false);
+            DLOG("attached\n");
+            con->percent = 0.0;
+            con_fix_percent(split);
+            con = split;
+            DLOG("ok, continuing with con %p instead\n", con);
+            con_detach(con);
+        }
+    }
+
     con->parent = parent;
 
     if (position == BEFORE) {
@@ -142,7 +159,8 @@ void tree_move(int direction) {
     } while (same_orientation == NULL);
 
     /* this time, we have to move to another container */
-    /* This is the container *above* 'con' which is inside 'same_orientation' */
+    /* This is the container *above* 'con' (an ancestor of con) which is inside
+     * 'same_orientation' */
     Con *above = con;
     while (above->parent != same_orientation)
         above = above->parent;

src/randr.c

@@ -193,7 +193,7 @@ void output_init_con(Output *output) {
     }
 
     if (con == NULL) {
-        con = con_new(croot);
+        con = con_new(croot, NULL);
         FREE(con->name);
         con->name = sstrdup(output->name);
         con->type = CT_OUTPUT;
@@ -213,7 +213,7 @@ void output_init_con(Output *output) {
     }
 
     DLOG("Changing layout, adding top/bottom dockarea\n");
-    Con *topdock = con_new(NULL);
+    Con *topdock = con_new(NULL, NULL);
     topdock->type = CT_DOCKAREA;
     topdock->layout = L_DOCKAREA;
     topdock->orientation = VERT;
@@ -235,7 +235,7 @@ void output_init_con(Output *output) {
     /* content container */
 
     DLOG("adding main content container\n");
-    Con *content = con_new(NULL);
+    Con *content = con_new(NULL, NULL);
     content->type = CT_CON;
     content->name = sstrdup("content");
 
@@ -245,7 +245,7 @@ void output_init_con(Output *output) {
     con_attach(content, con, false);
 
     /* bottom dock container */
-    Con *bottomdock = con_new(NULL);
+    Con *bottomdock = con_new(NULL, NULL);
     bottomdock->type = CT_DOCKAREA;
     bottomdock->layout = L_DOCKAREA;
     bottomdock->orientation = VERT;
@@ -363,7 +363,7 @@ void init_ws_for_output(Output *output, Con *content) {
     DLOG("Now adding a workspace\n");
 
     /* add a workspace to this output */
-    Con *ws = con_new(NULL);
+    Con *ws = con_new(NULL, NULL);
     ws->type = CT_WORKSPACE;
 
     /* get the next unused workspace number */

src/tree.c

@@ -23,7 +23,7 @@ bool tree_restore(const char *path) {
     }
 
     /* TODO: refactor the following */
-    croot = con_new(NULL);
+    croot = con_new(NULL, NULL);
     focused = croot;
 
     tree_append_json(globbed);
@@ -45,7 +45,7 @@ bool tree_restore(const char *path) {
  *
  */
 void tree_init() {
-    croot = con_new(NULL);
+    croot = con_new(NULL, NULL);
     FREE(croot->name);
     croot->name = "root";
     croot->type = CT_ROOT;
@@ -55,7 +55,7 @@ void tree_init() {
  * Opens an empty container in the current container
  *
  */
-Con *tree_open_con(Con *con) {
+Con *tree_open_con(Con *con, i3Window *window) {
     if (con == NULL) {
         /* every focusable Con has a parent (outputs have parent root) */
         con = focused->parent;
@@ -79,7 +79,7 @@ Con *tree_open_con(Con *con) {
     assert(con != NULL);
 
     /* 3. create the container and attach it to its parent */
-    Con *new = con_new(con);
+    Con *new = con_new(con, window);
 
     /* 4: re-calculate child->percent for each child */
     con_fix_percent(con);
@@ -265,7 +265,7 @@ void tree_split(Con *con, orientation_t orientation) {
     DLOG("Splitting in orientation %d\n", orientation);
 
     /* 2: replace it with a new Con */
-    Con *new = con_new(NULL);
+    Con *new = con_new(NULL, NULL);
     TAILQ_REPLACE(&(parent->nodes_head), con, new, nodes);
     TAILQ_REPLACE(&(parent->focus_head), con, new, focused);
     new->parent = parent;

src/workspace.c

@@ -41,7 +41,7 @@ Con *workspace_get(const char *num, bool *created) {
         LOG("got output %p with content %p\n", output, content);
         /* We need to attach this container after setting its type. con_attach
          * will handle CT_WORKSPACEs differently */
-        workspace = con_new(NULL);
+        workspace = con_new(NULL, NULL);
         char *name;
         asprintf(&name, "[i3 con] workspace %s", num);
         x_set_name(workspace, name);
@@ -266,7 +266,7 @@ void workspace_update_urgent_flag(Con *ws) {
  */
 void ws_force_orientation(Con *ws, orientation_t orientation) {
     /* 1: create a new split container */
-    Con *split = con_new(NULL);
+    Con *split = con_new(NULL, NULL);
     split->parent = ws;
 
     /* 2: copy layout and orientation from workspace */
@@ -296,3 +296,45 @@ void ws_force_orientation(Con *ws, orientation_t orientation) {
     if (old_focused)
         con_focus(old_focused);
 }
+
+/*
+ * Called when a new con (with a window, not an empty or split con) should be
+ * attached to the workspace (for example when managing a new window or when
+ * moving an existing window to the workspace level).
+ *
+ * Depending on the workspace_layout setting, this function either returns the
+ * workspace itself (default layout) or creates a new stacked/tabbed con and
+ * returns that.
+ *
+ */
+Con *workspace_attach_to(Con *ws) {
+    DLOG("Attaching a window to workspace %p / %s\n", ws, ws->name);
+
+    if (config.default_layout == L_DEFAULT) {
+        DLOG("Default layout, just attaching it to the workspace itself.\n");
+        return ws;
+    }
+
+    DLOG("Non-default layout, creating a new split container\n");
+    /* 1: create a new split container */
+    Con *new = con_new(NULL, NULL);
+    new->parent = ws;
+
+    /* 2: set the requested layout on the split con */
+    new->layout = config.default_layout;
+
+    /* 3: While the layout is irrelevant in stacked/tabbed mode, it needs
+     * to be set. Otherwise, this con will not be interpreted as a split
+     * container. */
+    if (config.default_orientation == NO_ORIENTATION) {
+        new->orientation = (ws->rect.height > ws->rect.width) ? VERT : HORIZ;
+    } else {
+        new->orientation = config.default_orientation;
+    }
+
+    /* 4: attach the new split container to the workspace */
+    DLOG("Attaching new split %p to workspace %p\n", new, ws);
+    con_attach(new, ws, false);
+
+    return new;
+}

testcases/t/67-workspace_layout.t

@@ -0,0 +1,139 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
+#
+# Tests the workspace_layout config option.
+#
+
+use i3test;
+use Cwd qw(abs_path);
+use Proc::Background;
+use File::Temp qw(tempfile tempdir);
+use X11::XCB qw(:all);
+use X11::XCB::Connection;
+
+my $x = X11::XCB::Connection->new;
+
+# assuming we are run by complete-run.pl
+my $i3_path = abs_path("../i3");
+
+#####################################################################
+# 1: check that with an empty config, cons are place next to each
+# other and no split containers are created
+#####################################################################
+
+my ($fh, $tmpfile) = tempfile();
+say $fh "font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1";
+say $fh "ipc-socket /tmp/nestedcons";
+close($fh);
+
+diag("Starting i3");
+my $i3cmd = "exec " . abs_path("../i3") . " -V -d all --disable-signalhandler -c $tmpfile >/dev/null 2>/dev/null";
+my $process = Proc::Background->new($i3cmd);
+sleep 1;
+
+diag("pid = " . $process->pid);
+
+my $tmp = fresh_workspace;
+
+ok(@{get_ws_content($tmp)} == 0, 'no containers yet');
+
+my $first = open_standard_window($x);
+my $second = open_standard_window($x);
+
+is($x->input_focus, $second->id, 'second window focused');
+ok(@{get_ws_content($tmp)} == 2, 'two containers opened');
+isnt($content[0]->{layout}, 'stacked', 'layout not stacked');
+isnt($content[1]->{layout}, 'stacked', 'layout not stacked');
+
+exit_gracefully($process->pid);
+
+#####################################################################
+# 2: set workspace_layout stacked, check that when opening two cons,
+# they end up in a stacked con
+#####################################################################
+
+($fh, $tmpfile) = tempfile();
+say $fh "font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1";
+say $fh "ipc-socket /tmp/nestedcons";
+say $fh "workspace_layout stacked";
+close($fh);
+
+diag("Starting i3");
+$i3cmd = "exec " . abs_path("../i3") . " -V -d all --disable-signalhandler -c $tmpfile >/dev/null 2>/dev/null";
+$process = Proc::Background->new($i3cmd);
+sleep 1;
+
+diag("pid = " . $process->pid);
+
+$tmp = fresh_workspace;
+
+ok(@{get_ws_content($tmp)} == 0, 'no containers yet');
+
+$first = open_standard_window($x);
+$second = open_standard_window($x);
+
+is($x->input_focus, $second->id, 'second window focused');
+my @content = @{get_ws_content($tmp)};
+ok(@content == 1, 'one con at workspace level');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+
+#####################################################################
+# 3: level up, open two new cons, check that they end up in a stacked
+# con
+#####################################################################
+
+cmd 'level up';
+my $right_top = open_standard_window($x);
+my $right_bot = open_standard_window($x);
+
+@content = @{get_ws_content($tmp)};
+is(@content, 2, 'two cons at workspace level after level up');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+is($content[1]->{layout}, 'stacked', 'layout stacked');
+
+#####################################################################
+# 4: move one of the cons to the right, check that it will end up in
+# a stacked con
+#####################################################################
+
+cmd 'move right';
+
+@content = @{get_ws_content($tmp)};
+is(@content, 3, 'three cons at workspace level after move');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+is($content[1]->{layout}, 'stacked', 'layout stacked');
+is($content[2]->{layout}, 'stacked', 'layout stacked');
+
+#####################################################################
+# 5: move it to the left again, check that the stacked con is deleted
+#####################################################################
+
+cmd 'move left';
+
+@content = @{get_ws_content($tmp)};
+is(@content, 2, 'two cons at workspace level after moving back');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+is($content[1]->{layout}, 'stacked', 'layout stacked');
+
+#####################################################################
+# 6: move it to a different workspace, check that it ends up in a
+# stacked con
+#####################################################################
+
+my $otmp = get_unused_workspace;
+
+cmd "move workspace $otmp";
+
+@content = @{get_ws_content($tmp)};
+is(@content, 2, 'still two cons on this workspace');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+is($content[1]->{layout}, 'stacked', 'layout stacked');
+
+@content = @{get_ws_content($otmp)};
+is(@content, 1, 'one con on target workspace');
+is($content[0]->{layout}, 'stacked', 'layout stacked');
+
+exit_gracefully($process->pid);
+
+done_testing;