con_set_layout: always use the parent container, handle workspaces properly

Previously, in case 'layout stacked' (for example) had been called
interactively, con_set_layout would be called with focused->parent,
while with for_window, it’d be called on the actual matching container.

This difference in behavior was the cause for the inability to use
'for_window [class="XTerm"] layout tabbed', which now works \o/, but
more on that below.

The change also allows us to handle the case of the user selecting a
CT_WORKSPACE container properly, that is, by using the special case and
creating a new split container on the workspace which gets all the
contents, but a new layout.

Now, before you are enthusiastic about the change and try to use
for_window magic in your config file, keep in mind: The 'layout' command
acts on the parent split container. That is, when using a line such as
this one:

    for_window [class="XTerm"] layout tabbed

…and opening an XTerm when on a workspace with one single other window,
the whole workspace will be set tabbed (just as previously when you
opened an XTerm and sent 'layout tabbed' manually).

Therefore, to open XTerm in its own tabbed split container, you need to
split before:

    for_window [class="XTerm"] split v, layout tabbed

The comma here is important! It says that the second command should not
be treated as an entirely unrelated command, but it should also relate
the matching window (while it does work with a ';', that is prone to
race-conditions and should be avoided).

fixes #358

diff --git a/src/commands.c b/src/commands.c
index 00d8b97d5ef550b2e1ebe4db2e669a7fa036b1e8..2d8fce3cc410cf7802eb2a72c280cdbc1b238553 100644 (file)
--- a/src/commands.c
+++ b/src/commands.c
@@ -1114,9 +1114,17 @@ void cmd_move_workspace_to_output(I3_CMD, char *name) {
  *
  */
 void cmd_split(I3_CMD, char *direction) {
+    owindow *current;
     /* TODO: use matches */
     LOG("splitting in direction %c\n", direction[0]);
-    tree_split(focused, (direction[0] == 'v' ? VERT : HORIZ));
+    if (match_is_empty(current_match))
+        tree_split(focused, (direction[0] == 'v' ? VERT : HORIZ));
+    else {
+        TAILQ_FOREACH(current, &owindows, owindows) {
+            DLOG("matching: %p / %s\n", current->con, current->con->name);
+            tree_split(current->con, (direction[0] == 'v' ? VERT : HORIZ));
+        }
+    }
 
     cmd_output->needs_tree_render = true;
     // XXX: default reply for now, make this a better reply
@@ -1429,7 +1437,7 @@ void cmd_layout(I3_CMD, char *layout_str) {
 
     /* check if the match is empty, not if the result is empty */
     if (match_is_empty(current_match))
-        con_set_layout(focused->parent, layout);
+        con_set_layout(focused, layout);
     else {
         TAILQ_FOREACH(current, &owindows, owindows) {
             DLOG("matching: %p / %s\n", current->con, current->con->name);
@@ -1456,7 +1464,7 @@ void cmd_layout_toggle(I3_CMD, char *toggle_mode) {
 
     /* check if the match is empty, not if the result is empty */
     if (match_is_empty(current_match))
-        con_toggle_layout(focused->parent, toggle_mode);
+        con_toggle_layout(focused, toggle_mode);
     else {
         TAILQ_FOREACH(current, &owindows, owindows) {
             DLOG("matching: %p / %s\n", current->con, current->con->name);

diff --git a/src/con.c b/src/con.c
index 0c82163a436818b1d4c85dec72a4729711be531b..cb756b6d4a63252ad49d1996803cb866ba35a073 100644 (file)
--- a/src/con.c
+++ b/src/con.c
@@ -1082,6 +1082,15 @@ void con_set_border_style(Con *con, int border_style) {
  *
  */
 void con_set_layout(Con *con, int layout) {
+    DLOG("con_set_layout(%p, %d), con->type = %d\n",
+         con, layout, con->type);
+
+    /* Users can focus workspaces, but not any higher in the hierarchy.
+     * Focus on the workspace is a special case, since in every other case, the
+     * user means "change the layout of the parent split container". */
+    if (con->type != CT_WORKSPACE)
+        con = con->parent;
+
     /* We fill in last_split_layout when switching to a different layout
      * since there are many places in the code that don’t use
      * con_set_layout(). */
@@ -1092,7 +1101,8 @@ void con_set_layout(Con *con, int layout) {
      * whole workspace into stacked/tabbed mode. To do this and still allow
      * intuitive operations (like level-up and then opening a new window), we
      * need to create a new split container. */
-    if (con->type == CT_WORKSPACE) {
+    if (con->type == CT_WORKSPACE &&
+        (layout == L_STACKED || layout == L_TABBED)) {
         DLOG("Creating new split container\n");
         /* 1: create a new split container */
         Con *new = con_new(NULL, NULL);
@@ -1100,7 +1110,7 @@ void con_set_layout(Con *con, int layout) {
 
         /* 2: Set the requested layout on the split container and mark it as
          * split. */
-        con_set_layout(new, layout);
+        new->layout = layout;
         new->last_split_layout = con->last_split_layout;
         new->split = true;
 
@@ -1156,30 +1166,38 @@ void con_set_layout(Con *con, int layout) {
  *
  */
 void con_toggle_layout(Con *con, const char *toggle_mode) {
+    Con *parent = con;
+    /* Users can focus workspaces, but not any higher in the hierarchy.
+     * Focus on the workspace is a special case, since in every other case, the
+     * user means "change the layout of the parent split container". */
+    if (con->type != CT_WORKSPACE)
+        parent = con->parent;
+    DLOG("con_toggle_layout(%p, %s), parent = %p\n", con, toggle_mode, parent);
+
     if (strcmp(toggle_mode, "split") == 0) {
         /* Toggle between splits. When the current layout is not a split
          * layout, we just switch back to last_split_layout. Otherwise, we
          * change to the opposite split layout. */
-        if (con->layout != L_SPLITH && con->layout != L_SPLITV)
-            con_set_layout(con, con->last_split_layout);
+        if (parent->layout != L_SPLITH && parent->layout != L_SPLITV)
+            con_set_layout(con, parent->last_split_layout);
         else {
-            if (con->layout == L_SPLITH)
+            if (parent->layout == L_SPLITH)
                 con_set_layout(con, L_SPLITV);
             else con_set_layout(con, L_SPLITH);
         }
     } else {
-        if (con->layout == L_STACKED)
+        if (parent->layout == L_STACKED)
             con_set_layout(con, L_TABBED);
-        else if (con->layout == L_TABBED) {
+        else if (parent->layout == L_TABBED) {
             if (strcmp(toggle_mode, "all") == 0)
                 con_set_layout(con, L_SPLITH);
-            else con_set_layout(con, con->last_split_layout);
-        } else if (con->layout == L_SPLITH || con->layout == L_SPLITV) {
+            else con_set_layout(con, parent->last_split_layout);
+        } else if (parent->layout == L_SPLITH || parent->layout == L_SPLITV) {
             if (strcmp(toggle_mode, "all") == 0) {
                 /* When toggling through all modes, we toggle between
                  * splith/splitv, whereas normally we just directly jump to
                  * stacked. */
-                if (con->layout == L_SPLITH)
+                if (parent->layout == L_SPLITH)
                     con_set_layout(con, L_SPLITV);
                 else con_set_layout(con, L_STACKED);
             } else {

diff --git a/testcases/t/122-split.t b/testcases/t/122-split.t
index d491c37a47451057132eca9acf6ab47e773fd5d4..7f8f392d4ab74254830686bec9d943d7639006a5 100644 (file)
--- a/testcases/t/122-split.t
+++ b/testcases/t/122-split.t
@@ -4,6 +4,7 @@
 # Tests splitting
 #
 use i3test;
+use List::Util qw(first);
 
 my $tmp;
 my $ws;
@@ -119,4 +120,29 @@ cmd 'open';
 is(scalar @content, 1, 'Still one container on this ws');
 is(scalar @{$content[0]->{nodes}}, 1, 'Stacked con still has one child node');
 
+################################################################################
+# When focusing the workspace, changing the layout should have an effect on the
+# workspace, not on the parent (CT_CONTENT) container.
+################################################################################
+
+sub get_output_content {
+    my $tree = i3(get_socket_path())->get_tree->recv;
+
+    my @outputs = grep { $_->{name} !~ /^__/ } @{$tree->{nodes}};
+    is(scalar @outputs, 1, 'exactly one output (testcase not multi-monitor capable)');
+    my $output = $outputs[0];
+    # get the first (and only) CT_CON
+    return first { $_->{type} == 2 } @{$output->{nodes}};
+}
+
+$tmp = fresh_workspace;
+
+cmd 'open';
+cmd 'split v';
+cmd 'open';
+cmd 'focus parent';
+is(get_output_content()->{layout}, 'splith', 'content container layout ok');
+cmd 'layout stacked';
+is(get_output_content()->{layout}, 'splith', 'content container layout still ok');
+
 done_testing;