Accept multiple outputs in move container|workspace to output

Fixes #4337

RELEASE-NOTES-next

@@ -19,6 +19,8 @@ strongly encouraged to upgrade.
     should be more reliable and also more portable.
   • Allow for_window to match against WM_CLIENT_MACHINE
   • Add %machine placeholder (WM_CLIENT_MACHINE) to title_format
+  • Allow multiple output names in 'move container|workspace to output'
+  • Add 'move container|workspace to output next'
 
  ┌────────────────────────────┐
  │ Bugfixes                   │

docs/userguide

@@ -2406,15 +2406,18 @@ To move a container to another RandR output (addressed by names like +LVDS1+ or
 
 *Syntax*:
 ------------------------------------------------------------
-move container to output left|right|down|up|current|primary|<output>
-move workspace to output left|right|down|up|current|primary|<output>
-------------------------------------------------------------
+move container to output left|right|down|up|current|primary|next|<output1> [output2]…
+move workspace to output left|right|down|up|current|primary|next|<output1> [output2]…
+-------------------------------------------------------------------------------------
 
 *Examples*:
 --------------------------------------------------------
 # Move the current workspace to the next output
 # (effectively toggles when you only have two outputs)
-bindsym $mod+x move workspace to output right
+bindsym $mod+x move workspace to output next
+
+# Cycle this workspace between outputs VGA1 and LVDS1 but not DVI0
+bindsym $mod+x move workspace to output VGA1 LVDS1
 
 # Put this window on the presentation output.
 bindsym $mod+x move container to output VGA1
@@ -2423,6 +2426,11 @@ bindsym $mod+x move container to output VGA1
 bindsym $mod+x move container to output primary
 --------------------------------------------------------
 
+If you specify more than one output, the container/workspace is cycled through
+them: If it is already in one of the outputs of the list, it will move to the
+next output in the list. If it is in an output not in the list, it will move to
+the first specified output. Non-existing outputs are skipped.
+
 Note that you might not have a primary output configured yet. To do so, run:
 -------------------------
 xrandr --output <output> --primary

include/commands.h

@@ -138,7 +138,7 @@ void cmd_mode(I3_CMD, const char *mode);
  * Implementation of 'move [window|container] [to] output <str>'.
  *
  */
-void cmd_move_con_to_output(I3_CMD, const char *name);
+void cmd_move_con_to_output(I3_CMD, const char *name, bool move_workspace);
 
 /**
  * Implementation of 'move [window|container] [to] mark <str>'.
@@ -152,12 +152,6 @@ void cmd_move_con_to_mark(I3_CMD, const char *mark);
  */
 void cmd_floating(I3_CMD, const char *floating_mode);
 
-/**
- * Implementation of 'move workspace to [output] <str>'.
- *
- */
-void cmd_move_workspace_to_output(I3_CMD, const char *name);
-
 /**
  * Implementation of 'split v|h|t|vertical|horizontal|toggle'.
  *

parser-specs/commands.spec

@@ -385,8 +385,10 @@ state MOVE_WORKSPACE_NUMBER:
       -> call cmd_move_con_to_workspace_number($number, $no_auto_back_and_forth)
 
 state MOVE_TO_OUTPUT:
-  output = string
-      -> call cmd_move_con_to_output($output)
+  output = word
+      -> call cmd_move_con_to_output($output, 0); MOVE_TO_OUTPUT
+  end
+      -> call cmd_move_con_to_output(NULL, 0); INITIAL
 
 state MOVE_TO_MARK:
   mark = string
@@ -394,9 +396,13 @@ state MOVE_TO_MARK:
 
 state MOVE_WORKSPACE_TO_OUTPUT:
   'output'
-      ->
-  output = string
-      -> call cmd_move_workspace_to_output($output)
+      -> MOVE_WORKSPACE_TO_OUTPUT_WORD
+
+state MOVE_WORKSPACE_TO_OUTPUT_WORD:
+  output = word
+      -> call cmd_move_con_to_output($output, 1); MOVE_WORKSPACE_TO_OUTPUT_WORD
+  end
+      -> call cmd_move_con_to_output(NULL, 1); INITIAL
 
 state MOVE_TO_ABSOLUTE_POSITION:
   'position'

src/commands.c

@@ -1024,23 +1024,113 @@ void cmd_mode(I3_CMD, const char *mode) {
 }
 
 /*
- * Implementation of 'move [window|container] [to] output <str>'.
+ * Implementation of 'move [window|container|workspace] [to] output <strings>'.
  *
  */
-void cmd_move_con_to_output(I3_CMD, const char *name) {
-    DLOG("Should move window to output \"%s\".\n", name);
-    HANDLE_EMPTY_MATCH;
+void cmd_move_con_to_output(I3_CMD, const char *name, bool move_workspace) {
+    /* Initialize a data structure that is used to save multiple user-specified
+     * output names since this function is called multiple types for each
+     * command call. */
+    typedef struct user_output_name {
+        char *name;
+        TAILQ_ENTRY(user_output_name) user_output_names;
+    } user_output_name;
+    static TAILQ_HEAD(user_output_names_head, user_output_name) user_output_names = TAILQ_HEAD_INITIALIZER(user_output_names);
 
-    owindow *current;
-    bool had_error = false;
-    TAILQ_FOREACH (current, &owindows, owindows) {
-        DLOG("matching: %p / %s\n", current->con, current->con->name);
-
-        had_error |= !con_move_to_output_name(current->con, name, true);
+    if (name) {
+        if (strcmp(name, "next") == 0) {
+            /* "next" here works like a wildcard: It "expands" to all available
+             * outputs. */
+            Output *output;
+            TAILQ_FOREACH (output, &outputs, outputs) {
+                user_output_name *co = scalloc(sizeof(user_output_name), 1);
+                co->name = sstrdup(output_primary_name(output));
+                TAILQ_INSERT_TAIL(&user_output_names, co, user_output_names);
+            }
+            return;
         }
 
-    cmd_output->needs_tree_render = true;
-    ysuccess(!had_error);
+        user_output_name *co = scalloc(sizeof(user_output_name), 1);
+        co->name = sstrdup(name);
+        TAILQ_INSERT_TAIL(&user_output_names, co, user_output_names);
+        return;
+    }
+
+    HANDLE_EMPTY_MATCH;
+
+    if (TAILQ_EMPTY(&user_output_names)) {
+        yerror("At least one output must be specified");
+        return;
+    }
+
+    bool success = false;
+    user_output_name *uo;
+    owindow *current;
+    TAILQ_FOREACH (current, &owindows, owindows) {
+        Con *ws = con_get_workspace(current->con);
+        if (con_is_internal(ws)) {
+            continue;
+        }
+
+        Output *current_output = get_output_for_con(ws);
+
+        Output *target_output = NULL;
+        TAILQ_FOREACH (uo, &user_output_names, user_output_names) {
+            if (strcasecmp(output_primary_name(current_output), uo->name) == 0) {
+                /* The current output is in the user list */
+                while (true) {
+                    /* This corrupts the outer loop but it is ok since we are
+                     * going to break anyway. */
+                    uo = TAILQ_NEXT(uo, user_output_names);
+                    if (!uo) {
+                        /* We reached the end of the list. We should use the
+                         * first available output that, if it exists, is
+                         * already saved in target_output. */
+                        break;
+                    }
+                    Output *out = get_output_from_string(current_output, uo->name);
+                    if (out) {
+                        DLOG("Found next target for workspace %s from user list: %s\n", ws->name, uo->name);
+                        target_output = out;
+                        break;
+                    }
+                }
+                break;
+            }
+            if (!target_output) {
+                /* The first available output from the list is used in 2 cases:
+                 * 1. When we must wrap around the user list. For example, if
+                 * user specifies outputs A B C and C is `current_output`.
+                 * 2. When the current output is not in the user list. For
+                 * example, user specifies A B C and D is `current_output`.
+                 */
+                DLOG("Found first target for workspace %s from user list: %s\n", ws->name, uo->name);
+                target_output = get_output_from_string(current_output, uo->name);
+            }
+        }
+        if (target_output) {
+            if (move_workspace) {
+                workspace_move_to_output(ws, target_output);
+            } else {
+                con_move_to_output(current->con, target_output, true);
+            }
+            success = true;
+        }
+    }
+
+    while (!TAILQ_EMPTY(&user_output_names)) {
+        uo = TAILQ_FIRST(&user_output_names);
+        free(uo->name);
+        TAILQ_REMOVE(&user_output_names, uo, user_output_names);
+        free(uo);
+    }
+
+    cmd_output->needs_tree_render = success;
+    if (success) {
+        ysuccess(true);
+    } else {
+        yerror("No output matched");
+    }
 }
 
 /*
@@ -1094,36 +1184,6 @@ void cmd_floating(I3_CMD, const char *floating_mode) {
     ysuccess(true);
 }
 
-/*
- * Implementation of 'move workspace to [output] <str>'.
- *
- */
-void cmd_move_workspace_to_output(I3_CMD, const char *name) {
-    DLOG("should move workspace to output %s\n", name);
-
-    HANDLE_EMPTY_MATCH;
-
-    owindow *current;
-    TAILQ_FOREACH (current, &owindows, owindows) {
-        Con *ws = con_get_workspace(current->con);
-        if (con_is_internal(ws)) {
-            continue;
-        }
-
-        Output *current_output = get_output_for_con(ws);
-        Output *target_output = get_output_from_string(current_output, name);
-        if (!target_output) {
-            yerror("Could not get output from string \"%s\"", name);
-            return;
-        }
-
-        workspace_move_to_output(ws, target_output);
-    }
-
-    cmd_output->needs_tree_render = true;
-    ysuccess(true);
-}
-
 /*
  * Implementation of 'split v|h|t|vertical|horizontal|toggle'.
  *

testcases/t/187-commands-parser.t

@@ -54,6 +54,7 @@ is(parser_calls(
    'move workspace foobar; ' .
    'move workspace torrent; ' .
    'move workspace to output LVDS1; ' .
+   'move to output LVDS1 DVI1; ' .
    'move workspace 3: foobar; ' .
    'move workspace "3: foobar"; ' .
    'move workspace "3: foobar, baz"; '),
@@ -62,7 +63,11 @@ is(parser_calls(
    "cmd_move_con_to_workspace_name(3, (null))\n" .
    "cmd_move_con_to_workspace_name(foobar, (null))\n" .
    "cmd_move_con_to_workspace_name(torrent, (null))\n" .
-   "cmd_move_workspace_to_output(LVDS1)\n" .
+   "cmd_move_con_to_output(LVDS1, 1)\n" .
+   "cmd_move_con_to_output(NULL, 1)\n" .
+   "cmd_move_con_to_output(LVDS1, 0)\n" .
+   "cmd_move_con_to_output(DVI1, 0)\n" .
+   "cmd_move_con_to_output(NULL, 0)\n" .
    "cmd_move_con_to_workspace_name(3: foobar, (null))\n" .
    "cmd_move_con_to_workspace_name(3: foobar, (null))\n" .
    "cmd_move_con_to_workspace_name(3: foobar, baz, (null))",

testcases/t/254-move-to-output-with-criteria.t

@@ -41,4 +41,12 @@ is_num_children($ws_top_right, 1, 'one container on the upper right workspace');
 is_num_children($ws_bottom_left, 0, 'no containers on the lower left workspace');
 is_num_children($ws_bottom_right, 1, 'one container on the lower right workspace');
 
+# Also test with multiple explicit outputs
+cmd '[class="moveme"] move window to output fake-1 fake-2';
+
+is_num_children($ws_top_left, 0, 'no containers on the upper left workspace');
+is_num_children($ws_top_right, 1, 'one container on the upper right workspace');
+is_num_children($ws_bottom_left, 1, 'one container on the lower left workspace');
+is_num_children($ws_bottom_right, 0, 'no containers on the lower right workspace');
+
 done_testing;

testcases/t/543-move-workspace-to-multiple-outputs.t

@@ -0,0 +1,93 @@
+#!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 using multiple workspaces for 'move workspace to output …'
+# Ticket: #4337
+use i3test i3_config => <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+fake-outputs 1024x768+0+0,1024x768+1024+0,1024x768+0+768,1024x768+1024+768
+EOT
+
+# Test setup: 4 outputs 2 marked windows
+
+open_window;
+cmd 'mark aa, move to workspace 1, workspace 1';
+open_window;
+cmd 'mark ab, move to workspace 3';
+
+sub is_ws {
+    my $ws_num = shift;
+    my $out_num = shift;
+    my $msg = shift;
+
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+    is(get_output_for_workspace("$ws_num"), "fake-$out_num", "Workspace $ws_num -> $out_num: $msg");
+}
+
+###############################################################################
+# Test using "next" special keyword
+###############################################################################
+
+is_ws(1, 0, 'sanity check');
+is_ws(3, 2, 'sanity check');
+
+for (my $i = 1; $i < 9; $i++) {
+    cmd '[con_mark=a] move workspace to output next';
+    my $out1 = $i % 4;
+    my $out3 = ($i + 2) % 4;
+
+    is_ws(1, $out1, 'move workspace to next');
+    is_ws(3, $out3, 'move workspace to next');
+}
+
+###############################################################################
+# Same as above but explicitely type all the outputs
+###############################################################################
+
+is_ws(1, 0, 'sanity check');
+is_ws(3, 2, 'sanity check');
+
+for (my $i = 1; $i < 10; $i++) {
+    cmd '[con_mark=a] move workspace to output fake-0 fake-1 fake-2 fake-3';
+    my $out1 = $i % 4;
+    my $out3 = ($i + 2) % 4;
+
+    is_ws(1, $out1, 'cycle through explicit outputs');
+    is_ws(3, $out3, 'cycle through explicit outputs');
+}
+
+###############################################################################
+# Use a subset of the outputs plus some non-existing outputs
+###############################################################################
+
+cmd '[con_mark=aa] move workspace to output fake-1';
+cmd '[con_mark=ab] move workspace to output fake-1';
+is_ws(1, 1, 'start from fake-1 which is not included in output list');
+is_ws(3, 1, 'start from fake-1 which is not included in output list');
+
+my @order = (0, 3, 2);
+for (my $i = 0; $i < 10; $i++) {
+    cmd '[con_mark=a] move workspace to output doesnotexist fake-0 alsodoesnotexist fake-3 fake-2';
+
+    my $out = $order[$i % 3];
+    is_ws(1, $out, 'cycle through shuffled outputs');
+    is_ws(3, $out, 'cycle through shuffled outputs');
+
+}
+
+done_testing;