Merge pull request #3254 from orestisf1993/issue-555

Multiple assignments of workspaces to outputs (#555)

docs/userguide

@@ -888,7 +888,7 @@ the second screen and so on).
 
 *Syntax*:
 -------------------------------------
-workspace <workspace> output <output>
+workspace <workspace> output <output1> [output2]…
 -------------------------------------
 
 The 'output' is the name of the RandR output you attach your screen to. On a
@@ -907,12 +907,15 @@ monitor name is “Dell UP2414Q”.
 entire monitor, i3 will still use the entire area of the containing monitor
 rather than that of just the output's.)
 
+You can specify multiple outputs. The first available will be used.
+
 If you use named workspaces, they must be quoted:
 
 *Examples*:
 ---------------------------
 workspace 1 output LVDS1
-workspace 5 output VGA1
+workspace 2 output primary
+workspace 5 output VGA1 LVDS1
 workspace "2: vim" output VGA1
 ---------------------------
 

include/workspace.h

@@ -38,6 +38,13 @@ Con *get_existing_workspace_by_name(const char *name);
  */
 Con *get_existing_workspace_by_num(int num);
 
+/**
+ * Returns true if the first output assigned to a workspace with the given
+ * workspace assignment is the same as the given output.
+ *
+ */
+bool output_triggers_assignment(Output *output, struct Workspace_Assignment *assignment);
+
 /**
  * Returns a pointer to the workspace with the given number (starting at 0),
  * creating the workspace if necessary (by allocating the necessary amount of

parser-specs/config.spec

@@ -273,7 +273,7 @@ state WORKSPACE_OUTPUT:
       -> WORKSPACE_OUTPUT_STR
 
 state WORKSPACE_OUTPUT_STR:
-  output = word
+  output = string
       -> call cfg_workspace($workspace, $output)
 
 # ipc-socket <path>

src/commands.c

@@ -2019,6 +2019,9 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) {
             LOG("Could not get output named \"%s\"\n", assignment->output);
             continue;
         }
+        if (!output_triggers_assignment(target_output, assignment)) {
+            continue;
+        }
         workspace_move_to_output(workspace, target_output);
 
         break;

src/config_directives.c

@@ -322,8 +322,8 @@ CFGFUN(show_marks, const char *value) {
     config.show_marks = eval_boolstr(value);
 }
 
-CFGFUN(workspace, const char *workspace, const char *output) {
+CFGFUN(workspace, const char *workspace, const char *outputs) {
-    DLOG("Assigning workspace \"%s\" to output \"%s\"\n", workspace, output);
+    DLOG("Assigning workspace \"%s\" to outputs \"%s\"\n", workspace, outputs);
     /* Check for earlier assignments of the same workspace so that we
      * don’t have assignments of a single workspace to different
      * outputs */
@@ -336,10 +336,16 @@ CFGFUN(workspace, const char *workspace, const char *output) {
         }
     }
 
-    assignment = scalloc(1, sizeof(struct Workspace_Assignment));
+    char *buf = sstrdup(outputs);
-    assignment->name = sstrdup(workspace);
+    char *output = strtok(buf, " ");
-    assignment->output = sstrdup(output);
+    while (output != NULL) {
-    TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments);
+        assignment = scalloc(1, sizeof(struct Workspace_Assignment));
+        assignment->name = sstrdup(workspace);
+        assignment->output = sstrdup(output);
+        TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments);
+        output = strtok(NULL, " ");
+    }
+    free(buf);
 }
 
 CFGFUN(ipc_socket, const char *path) {

src/randr.c

@@ -424,9 +424,9 @@ void init_ws_for_output(Output *output, Con *content) {
     /* go through all assignments and move the existing workspaces to this output */
     struct Workspace_Assignment *assignment;
     TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
-        if (strcmp(assignment->output, output_primary_name(output)) != 0)
+        if (!output_triggers_assignment(output, assignment)) {
             continue;
-
+        }
         Con *workspace = get_existing_workspace_by_name(assignment->name);
         if (workspace == NULL)
             continue;
@@ -501,8 +501,9 @@ void init_ws_for_output(Output *output, Con *content) {
 
     /* otherwise, we create the first assigned ws for this output */
     TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
-        if (strcmp(assignment->output, output_primary_name(output)) != 0)
+        if (!output_triggers_assignment(output, assignment)) {
             continue;
+        }
 
         LOG("Initializing first assigned workspace \"%s\" for output \"%s\"\n",
             assignment->name, assignment->output);

src/workspace.c

@@ -67,6 +67,52 @@ static void _workspace_apply_default_orientation(Con *ws) {
     }
 }
 
+/*
+ * Returns the first output that is assigned to a workspace specified by the
+ * given name or number or NULL if no such output exists. If there is a
+ * workspace with a matching name and another workspace with a matching number,
+ * the output assigned to the first one is returned.
+ * The order of the 'ws_assignments' queue is respected: if multiple assignments
+ * match the specified workspace, the first one is returned.
+ * If 'name' is NULL it will be ignored.
+ * If 'parsed_num' is -1 it will be ignored.
+ *
+ */
+static Con *get_assigned_output(const char *name, long parsed_num) {
+    Con *output = NULL;
+    struct Workspace_Assignment *assignment;
+    TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
+        if (name && strcmp(assignment->name, name) == 0) {
+            DLOG("Found workspace name assignment to output \"%s\"\n", assignment->output);
+            Output *assigned_by_name = get_output_by_name(assignment->output, true);
+            if (assigned_by_name) {
+                /* When the name matches exactly, skip numbered assignments. */
+                return assigned_by_name->con;
+            }
+        } else if (!output && /* Only keep the first numbered assignment. */
+                   parsed_num != -1 &&
+                   name_is_digits(assignment->name) &&
+                   ws_name_to_number(assignment->name) == parsed_num) {
+            DLOG("Found workspace number assignment to output \"%s\"\n", assignment->output);
+            Output *assigned_by_num = get_output_by_name(assignment->output, true);
+            if (assigned_by_num) {
+                output = assigned_by_num->con;
+            }
+        }
+    }
+
+    return output;
+}
+
+/*
+ * Returns true if the first output assigned to a workspace with the given
+ * workspace assignment is the same as the given output.
+ */
+bool output_triggers_assignment(Output *output, struct Workspace_Assignment *assignment) {
+    Con *assigned = get_assigned_output(assignment->name, -1);
+    return assigned && assigned == output->con;
+}
+
 /*
  * Returns a pointer to the workspace with the given number (starting at 0),
  * creating the workspace if necessary (by allocating the necessary amount of
@@ -78,25 +124,16 @@ Con *workspace_get(const char *num, bool *created) {
 
     if (workspace == NULL) {
         LOG("Creating new workspace \"%s\"\n", num);
-        /* unless an assignment is found, we will create this workspace on the current output */
-        Con *output = con_get_output(focused);
-        /* look for assignments */
-        struct Workspace_Assignment *assignment;
 
         /* We set workspace->num to the number if this workspace’s name begins
          * with a positive number. Otherwise it’s a named ws and num will be
          * -1. */
         long parsed_num = ws_name_to_number(num);
 
-        TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
+        Con *output = get_assigned_output(num, parsed_num);
-            if (strcmp(assignment->name, num) == 0) {
+        /* if an assignment is not found, we create this workspace on the current output */
-                DLOG("Found workspace name assignment to output \"%s\"\n", assignment->output);
+        if (!output) {
-                GREP_FIRST(output, croot, !strcmp(child->name, assignment->output));
+            output = con_get_output(focused);
-                break;
-            } else if (parsed_num != -1 && name_is_digits(assignment->name) && ws_name_to_number(assignment->name) == parsed_num) {
-                DLOG("Found workspace number assignment to output \"%s\"\n", assignment->output);
-                GREP_FIRST(output, croot, !strcmp(child->name, assignment->output));
-            }
         }
 
         Con *content = output_get_content(output);
@@ -208,19 +245,10 @@ Con *create_workspace_on_output(Output *output, Con *content) {
         /* Ensure that this workspace is not assigned to a different output —
          * otherwise we would create it, then move it over to its output, then
          * find a new workspace, etc… */
-        bool assigned = false;
+        Con *assigned = get_assigned_output(target_name, -1);
-        struct Workspace_Assignment *assignment;
+        if (assigned && assigned != output->con) {
-        TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
-            if (strcmp(assignment->name, target_name) != 0 ||
-                strcmp(assignment->output, output_primary_name(output)) == 0)
-                continue;
-
-            assigned = true;
-            break;
-        }
-
-        if (assigned)
             continue;
+        }
 
         exists = (get_existing_workspace_by_name(target_name) != NULL);
         if (!exists) {
@@ -239,7 +267,9 @@ Con *create_workspace_on_output(Output *output, Con *content) {
         DLOG("Getting next unused workspace by number\n");
         int c = 0;
         while (exists) {
-            exists = (get_existing_workspace_by_num(++c) != NULL);
+            c++;
+            Con *assigned = get_assigned_output(NULL, c);
+            exists = (get_existing_workspace_by_num(c) || (assigned && assigned != output->con));
             DLOG("result for ws %d: exists = %d\n", c, exists);
         }
         ws->num = c;
@@ -946,8 +976,9 @@ bool workspace_move_to_output(Con *ws, Output *output) {
         bool used_assignment = false;
         struct Workspace_Assignment *assignment;
         TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) {
-            if (assignment->output == NULL || strcmp(assignment->output, output_primary_name(current_output)) != 0)
+            if (!output_triggers_assignment(current_output, assignment)) {
                 continue;
+            }
             /* check if this workspace is already attached to the tree */
             if (get_existing_workspace_by_name(assignment->name) != NULL) {
                 continue;

testcases/lib/i3test.pm.in

@@ -26,6 +26,7 @@ use Data::Dumper ();
 use Exporter ();
 our @EXPORT = qw(
     get_workspace_names
+    get_output_for_workspace
     get_unused_workspace
     fresh_workspace
     get_ws_content
@@ -402,6 +403,29 @@ sub get_workspace_names {
     [ map { $_->{name} } @cons ]
 }
 
+=head2 get_output_for_workspace()
+
+Returns the name of the output on which this workspace resides
+
+  cmd 'focus output fake-1';
+  cmd 'workspace 1';
+  is(get_output_for_workspace('1', 'fake-0', 'Workspace 1 in output fake-0');
+
+=cut
+sub get_output_for_workspace {
+    my $ws_name = shift @_;
+    my $i3 = i3(get_socket_path());
+    my $tree = $i3->get_tree->recv;
+    my @outputs = @{$tree->{nodes}};
+
+    foreach (grep { not $_->{name} =~ /^__/ } @outputs) {
+        my $output = $_->{name};
+        foreach (grep { $_->{name} =~ "content" } @{$_->{nodes}}) {
+            return $output if $_->{nodes}[0]->{name} =~ $ws_name;
+        }
+    }
+}
+
 =head2 get_unused_workspace
 
 Returns a workspace name which has not yet been used. See also

testcases/t/297-assign-workspace-to-output.t

@@ -0,0 +1,102 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • https://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • https://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • https://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)
+#
+# Test assignments of workspaces to outputs.
+use i3test i3_autostart => 0;
+
+################################################################################
+# Test initial workspaces.
+################################################################################
+
+my $config = <<EOT;
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1024x768+0+0,1024x768+1024+0,1024x768+1024+768,1024x768+0+768
+
+bindsym Mod1+x workspace bindingname
+
+workspace 9 output doesnotexist
+workspace special output fake-0
+workspace 1 output doesnotexist
+workspace dontusethisname output doesnotexist
+workspace donotoverride output fake-0
+workspace 2 output fake-0
+workspace 3 output fake-0
+EOT
+
+my $pid = launch_with_config($config);
+
+sub check_output {
+    my ($workspace, $output, $msg) = @_;
+    is(get_output_for_workspace($workspace), $output, $msg);
+}
+
+check_output('9', '', 'Numbered workspace with a big number that is assigned to output that does not exist is not used');
+check_output('special', 'fake-0', 'Output gets special workspace because of assignment');
+check_output('bindingname', 'fake-1', 'Bindings give workspace names');
+check_output('1', 'fake-2', 'Numbered workspace that is assigned to output that does not exist is used');
+check_output('2', '', 'Numbered workspace assigned to output with existing workspace is not used');
+check_output('3', '', 'Numbered workspace assigned to output with existing workspace is not used');
+check_output('4', 'fake-3', 'First available number that is not assigned to existing output is used');
+check_output('dontusethisname', '', 'Named workspace that is assigned to output that does not exist is not used');
+check_output('donotoverride', '', 'Named workspace assigned to already occupied output is not used');
+
+exit_gracefully($pid);
+
+################################################################################
+# Test multiple assignments
+################################################################################
+
+sub do_test {
+    my ($workspace, $output, $msg) = @_;
+    cmd 'focus output ' . ($output eq 'fake-3' ? 'fake-0' : 'fake-3');
+
+    cmd "workspace $workspace";
+    check_output($workspace, $output, $msg)
+}
+
+$config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1024x768+0+0P,1024x768+1024+0,1024x768+1024+768,1024x768+0+768
+
+workspace 1 output fake-0
+workspace 1 output fake-1 fake-2
+workspace 2 output fake-3 fake-4 fake-0 fake-1
+workspace 3 output these outputs do not exist but these do: fake-0 fake-3
+workspace 4 output whitespace                    fake-0
+workspace special output doesnotexist1 doesnotexist2 doesnotexist3
+EOT
+
+$pid = launch_with_config($config);
+
+do_test('1', 'fake-0', 'Multiple assignments do not override a single one');
+do_test('2', 'fake-3', 'First output out of multiple assignments is used');
+do_test('3', 'fake-0', 'First existing output is used');
+do_test('4', 'fake-0', 'Excessive whitespace is ok');
+do_test('5', 'fake-1', 'Numbered initialization for fake-1');
+do_test('6', 'fake-2', 'Numbered initialization for fake-2');
+
+cmd 'focus output fake-0, workspace special';
+check_output('special', 'fake-0', 'Workspace with only non-existing assigned outputs opened in current output.');
+
+# Moving assigned workspaces.
+cmd 'workspace 2, move workspace to output left';
+check_output('2', 'fake-2', 'Moved assigned workspace up');
+
+exit_gracefully($pid);
+done_testing;

testcases/t/518-interpret-workspace-numbers.t

@@ -26,25 +26,12 @@ workspace 1:override output fake-0
 workspace 2 output fake-0
 workspace 1 output fake-1
 workspace 2:override output fake-1
+workspace 3 output fake-0
+workspace 3:override output doesnotexist fake-1
 
 fake-outputs 1024x768+0+0,1024x768+1024+0
 EOT
 
-my $i3 = i3(get_socket_path());
-$i3->connect->recv;
-
-# Returns the name of the output on which this workspace resides
-sub get_output_for_workspace {
-    my $ws_name = shift @_;
-
-    foreach (grep { not $_->{name} =~ /^__/ } @{$i3->get_tree->recv->{nodes}}) {
-        my $output = $_->{name};
-        foreach (grep { $_->{name} =~ "content" } @{$_->{nodes}}) {
-            return $output if $_->{nodes}[0]->{name} =~ $ws_name;
-        }
-    }
-}
-
 ################################################################################
 # Workspace assignments with bare numbers should be interpreted as `workspace
 # number` config directives. Any workspace beginning with that number should be
@@ -69,4 +56,10 @@ is(get_output_for_workspace('1:override'), 'fake-0',
     'Assignment rules should not be affected by the order assignments are declared')
     or diag 'Since workspace "1:override" is assigned by name to fake-0, it should open on fake-0';
 
+cmd 'focus output fake-1';
+cmd 'workspace "3:override"';
+is(get_output_for_workspace('3:override'), 'fake-1',
+   'Assignment rules should not be affected by multiple output assignments')
+    or diag 'Since workspace "3:override" is assigned by name to fake-1, it should open on fake-1';
+
 done_testing;

testcases/t/522-rename-assigned-workspace.t

@@ -29,23 +29,10 @@ workspace 2 output fake-1
 workspace 3:foo output fake-1
 workspace baz output fake-1
 workspace 5 output left
+workspace 6 output doesnotexist fake-0
+workspace 7 output fake-1 fake-0
 EOT
 
-my $i3 = i3(get_socket_path());
-$i3->connect->recv;
-
-# Returns the name of the output on which this workspace resides
-sub get_output_for_workspace {
-    my $ws_name = shift @_;
-
-    foreach (grep { not $_->{name} =~ /^__/ } @{$i3->get_tree->recv->{nodes}}) {
-        my $output = $_->{name};
-        foreach (grep { $_->{name} =~ "content" } @{$_->{nodes}}) {
-            return $output if $_->{nodes}[0]->{name} =~ $ws_name;
-        }
-    }
-}
-
 ##########################################################################
 # Renaming the workspace to an unassigned name does not move the workspace
 # (regression test)
@@ -106,4 +93,24 @@ cmd 'rename workspace 5 to 2';
 is(get_output_for_workspace('2'), 'fake-1',
     'Renaming a workspace so that it moves to the focused output which contains only an empty workspace should replace the empty workspace');
 
+##########################################################################
+# Renaming a workspace with multiple assignments, where the first output
+# doesn't exist.
+##########################################################################
+
+cmd 'focus output fake-1';
+cmd 'rename workspace to 6';
+is(get_output_for_workspace('6'), 'fake-0',
+   'Renaming the workspace while first target output doesn\'t exist moves it to the second assigned output');
+
+##########################################################################
+# Renaming a workspace with multiple assignments, where both outputs exist
+# moves it to the first output.
+##########################################################################
+
+cmd 'focus output fake-0';
+cmd 'rename workspace to 7';
+is(get_output_for_workspace('7'), 'fake-1',
+   'Renaming a workspace with multiple assignments, where both outputs exist moves it to the first output.');
+
 done_testing;