Merge pull request #4622 from orestisfl/focus-output-next

Focus output next

9 files changed, 214 insertions, 108 deletions

diff --git a/docs/userguide b/docs/userguide
index a05f12d3..d43361b6 100644
--- a/docs/userguide
+++ b/docs/userguide
@@ -2212,7 +2212,7 @@ output::
 focus left|right|down|up
 focus parent|child|floating|tiling|mode_toggle
 focus next|prev [sibling]
-focus output left|right|up|down|primary|<output>
+focus output left|right|down|up|current|primary|next|<output1> [output2]…
 ----------------------------------------------
 
 *Examples*:
@@ -2232,6 +2232,9 @@ bindsym $mod+u focus parent
 # Focus last floating/tiling container
 bindsym $mod+g focus mode_toggle
 
+# Focus the next output (effectively toggles when you only have two outputs)
+bindsym $mod+x move workspace to output next
+
 # Focus the output right to the current one
 bindsym $mod+x focus output right
 
@@ -2240,6 +2243,9 @@ bindsym $mod+x focus output HDMI-2
 
 # Focus the primary output
 bindsym $mod+x focus output primary
+
+# Cycle focus between outputs VGA1 and LVDS1 but not DVI0
+bindsym $mod+x move workspace to output VGA1 LVDS1
 -------------------------------------------------
 
 Note that you might not have a primary output configured yet. To do so, run:
diff --git a/parser-specs/commands.spec b/parser-specs/commands.spec
index 7d3367ea..6efaa855 100644
--- a/parser-specs/commands.spec
+++ b/parser-specs/commands.spec
@@ -166,8 +166,10 @@ state FOCUS_AUTO:
       -> call cmd_focus_direction($direction)
 
 state FOCUS_OUTPUT:
-  output = string
-      -> call cmd_focus_output($output)
+  output = word
+      -> call cmd_focus_output($output); FOCUS_OUTPUT
+  end
+      -> call cmd_focus_output(NULL); INITIAL
 
 # kill [window|client]
 state KILL:
diff --git a/release-notes/changes/3-focus-outputs b/release-notes/changes/3-focus-outputs
new file mode 100644
index 00000000..cecd5b42
--- /dev/null
+++ b/release-notes/changes/3-focus-outputs
@@ -0,0 +1 @@
+Add support for multiple outputs in focus command
diff --git a/src/commands.c b/src/commands.c
index 798e2f6c..ac02ba6e 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -1023,6 +1023,76 @@ void cmd_mode(I3_CMD, const char *mode) {
     ysuccess(true);
 }
 
+typedef struct user_output_name {
+    char *name;
+    TAILQ_ENTRY(user_output_name) user_output_names;
+} user_output_name;
+typedef TAILQ_HEAD(user_output_names_head, user_output_name) user_output_names_head;
+
+static void user_output_names_add(user_output_names_head *list, const char *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(list, co, user_output_names);
+        }
+        return;
+    }
+
+    user_output_name *co = scalloc(sizeof(user_output_name), 1);
+    co->name = sstrdup(name);
+    TAILQ_INSERT_TAIL(list, co, user_output_names);
+    return;
+}
+
+static Output *user_output_names_find_next(user_output_names_head *names, Output *current_output) {
+    Output *target_output = NULL;
+    user_output_name *uo;
+    TAILQ_FOREACH (uo, 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) {
+                    return out;
+                }
+            }
+            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`. */
+            target_output = get_output_from_string(current_output, uo->name);
+        }
+    }
+    return target_output;
+}
+
+static void user_output_names_free(user_output_names_head *names) {
+    user_output_name *uo;
+    while (!TAILQ_EMPTY(names)) {
+        uo = TAILQ_FIRST(names);
+        free(uo->name);
+        TAILQ_REMOVE(names, uo, user_output_names);
+        free(uo);
+    }
+}
+
 /*
  * Implementation of 'move [window|container|workspace] [to] output <strings>'.
  *
@@ -1031,40 +1101,21 @@ 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);
+    static user_output_names_head names = TAILQ_HEAD_INITIALIZER(names);
 
     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;
-        }
-
-        user_output_name *co = scalloc(sizeof(user_output_name), 1);
-        co->name = sstrdup(name);
-        TAILQ_INSERT_TAIL(&user_output_names, co, user_output_names);
+        user_output_names_add(&names, name);
         return;
     }
 
     HANDLE_EMPTY_MATCH;
 
-    if (TAILQ_EMPTY(&user_output_names)) {
+    if (TAILQ_EMPTY(&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);
@@ -1073,41 +1124,7 @@ void cmd_move_con_to_output(I3_CMD, const char *name, bool move_workspace) {
         }
 
         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);
-            }
-        }
+        Output *target_output = user_output_names_find_next(&names, current_output);
         if (target_output) {
             if (move_workspace) {
                 workspace_move_to_output(ws, target_output);
@@ -1117,13 +1134,7 @@ void cmd_move_con_to_output(I3_CMD, const char *name, bool move_workspace) {
             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);
-    }
+    user_output_names_free(&names);
 
     cmd_output->needs_tree_render = success;
     if (success) {
@@ -1757,6 +1768,17 @@ void cmd_open(I3_CMD) {
  *
  */
 void cmd_focus_output(I3_CMD, const char *name) {
+    static user_output_names_head names = TAILQ_HEAD_INITIALIZER(names);
+    if (name) {
+        user_output_names_add(&names, name);
+        return;
+    }
+
+    if (TAILQ_EMPTY(&names)) {
+        yerror("At least one output must be specified");
+        return;
+    }
+
     HANDLE_EMPTY_MATCH;
 
     if (TAILQ_EMPTY(&owindows)) {
@@ -1765,25 +1787,29 @@ void cmd_focus_output(I3_CMD, const char *name) {
     }
 
     Output *current_output = get_output_for_con(TAILQ_FIRST(&owindows)->con);
-    Output *output = get_output_from_string(current_output, name);
+    Output *target_output = user_output_names_find_next(&names, current_output);
+    user_output_names_free(&names);
+    bool success = false;
+    if (target_output) {
+        success = true;
 
-    if (!output) {
-        yerror("Output %s not found.", name);
-        return;
-    }
+        /* get visible workspace on output */
+        Con *ws = NULL;
+        GREP_FIRST(ws, output_get_content(target_output->con), workspace_is_visible(child));
+        if (!ws) {
+            yerror("BUG: No workspace found on output.");
+            return;
+        }
 
-    /* get visible workspace on output */
-    Con *ws = NULL;
-    GREP_FIRST(ws, output_get_content(output->con), workspace_is_visible(child));
-    if (!ws) {
-        yerror("BUG: No workspace found on output.");
-        return;
+        workspace_show(ws);
     }
 
-    workspace_show(ws);
-
-    cmd_output->needs_tree_render = true;
-    ysuccess(true);
+    cmd_output->needs_tree_render = success;
+    if (success) {
+        ysuccess(true);
+    } else {
+        yerror("No output matched");
+    }
 }
 
 /*
diff --git a/testcases/lib/i3test.pm.in b/testcases/lib/i3test.pm.in
index 16a0a75c..c64d5b82 100644
--- a/testcases/lib/i3test.pm.in
+++ b/testcases/lib/i3test.pm.in
@@ -42,6 +42,7 @@ our @EXPORT = qw(
     exit_forcefully
     workspace_exists
     focused_ws
+    focused_output
     get_socket_path
     launch_with_config
     get_i3_log
@@ -664,6 +665,25 @@ sub workspace_exists {
     (scalar grep { $_ eq $name } @{get_workspace_names()}) > 0;
 }
 
+=head2 focused_output
+
+Returns the name of the currently focused output.
+
+  is(focused_output, 'fake-0', 'i3 starts on output 0');
+
+=cut
+sub _focused_output {
+    my $i3 = i3(get_socket_path());
+    my $tree = $i3->get_tree->recv;
+    my $focused = $tree->{focus}->[0];
+    my $output = first { $_->{id} == $focused } @{$tree->{nodes}};
+    return $output;
+}
+
+sub focused_output {
+    return _focused_output->{name}
+}
+
 =head2 focused_ws
 
 Returns the name of the currently focused workspace.
@@ -672,11 +692,9 @@ Returns the name of the currently focused workspace.
   is($ws, '1', 'i3 starts on workspace 1');
 
 =cut
+
 sub focused_ws {
-    my $i3 = i3(get_socket_path());
-    my $tree = $i3->get_tree->recv;
-    my $focused = $tree->{focus}->[0];
-    my $output = first { $_->{id} == $focused } @{$tree->{nodes}};
+    my $output = _focused_output;
     my $content = first { $_->{type} eq 'con' } @{$output->{nodes}};
     my $first = first { $_->{fullscreen_mode} == 1 } @{$content->{nodes}};
     return $first->{name}
diff --git a/testcases/t/502-focus-output.t b/testcases/t/502-focus-output.t
index 118aba16..5b051de4 100644
--- a/testcases/t/502-focus-output.t
+++ b/testcases/t/502-focus-output.t
@@ -31,13 +31,6 @@ my $i3 = i3(get_socket_path());
 # use 'focus output' and verify that focus gets changed appropriately
 ################################################################################
 
-sub focused_output {
-    my $tree = $i3->get_tree->recv;
-    my $focused = $tree->{focus}->[0];
-    my $output = first { $_->{id} == $focused } @{$tree->{nodes}};
-    return $output->{name};
-}
-
 sync_with_i3;
 $x->root->warp_pointer(0, 0);
 sync_with_i3;
diff --git a/testcases/t/506-focus-right.t b/testcases/t/506-focus-right.t
index 871bc1c8..2d8ed205 100644
--- a/testcases/t/506-focus-right.t
+++ b/testcases/t/506-focus-right.t
@@ -149,32 +149,25 @@ sync_with_i3;
 # Ensure that focusing right/left works in the expected order.
 ############################################################################
 
-sub get_focused_output {
-    my $tree = i3(get_socket_path())->get_tree->recv;
-    my ($focused_id) = @{$tree->{focus}};
-    my ($output) = grep { $_->{id} == $focused_id } @{$tree->{nodes}};
-    return $output->{name};
-}
-
-is(get_focused_output(), 'fake-0', 'focus on fake-0');
+is(focused_output, 'fake-0', 'focus on fake-0');
 
 cmd 'focus output right';
-is(get_focused_output(), 'fake-1', 'focus on fake-1');
+is(focused_output, 'fake-1', 'focus on fake-1');
 
 cmd 'focus output right';
-is(get_focused_output(), 'fake-2', 'focus on fake-2');
+is(focused_output, 'fake-2', 'focus on fake-2');
 
 cmd 'focus output left';
-is(get_focused_output(), 'fake-1', 'focus on fake-1');
+is(focused_output, 'fake-1', 'focus on fake-1');
 
 cmd 'focus output left';
-is(get_focused_output(), 'fake-0', 'focus on fake-0');
+is(focused_output, 'fake-0', 'focus on fake-0');
 
 cmd 'focus output left';
-is(get_focused_output(), 'fake-2', 'focus on fake-2 (wrapping)');
+is(focused_output, 'fake-2', 'focus on fake-2 (wrapping)');
 
 cmd 'focus output right';
-is(get_focused_output(), 'fake-0', 'focus on fake-0 (wrapping)');
+is(focused_output, 'fake-0', 'focus on fake-0 (wrapping)');
 
 exit_gracefully($pid);
 
diff --git a/testcases/t/543-move-workspace-to-multiple-outputs.t b/testcases/t/543-move-workspace-to-multiple-outputs.t
index b9bfabec..72614fe5 100644
--- a/testcases/t/543-move-workspace-to-multiple-outputs.t
+++ b/testcases/t/543-move-workspace-to-multiple-outputs.t
@@ -14,7 +14,7 @@
 # • 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 …'
+# Test using multiple outputs for 'move workspace to output …'
 # Ticket: #4337
 use i3test i3_config => <<EOT;
 # i3 config file (v4)
diff --git a/testcases/t/544-focus-multiple-outputs.t b/testcases/t/544-focus-multiple-outputs.t
new file mode 100644
index 00000000..eda1f832
--- /dev/null
+++ b/testcases/t/544-focus-multiple-outputs.t
@@ -0,0 +1,67 @@
+#!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 output for 'focus output …'
+# Ticket: #4619
+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 using "next" special keyword
+###############################################################################
+
+is(focused_output, "fake-0", 'sanity check');
+
+for (my $i = 1; $i < 9; $i++) {
+    cmd 'focus output next';
+
+    my $out = $i % 4;
+    is(focused_output, "fake-$out", 'focus output next cycle');
+}
+
+###############################################################################
+# Same as above but explicitely type all the outputs
+###############################################################################
+
+is(focused_output, "fake-0", 'sanity check');
+
+for (my $i = 1; $i < 10; $i++) {
+    cmd 'focus output fake-0 fake-1 fake-2 fake-3';
+
+    my $out = $i % 4;
+    is(focused_output, "fake-$out", 'focus output next cycle');
+}
+
+###############################################################################
+# Use a subset of the outputs plus some non-existing outputs
+###############################################################################
+
+cmd 'focus output fake-1';
+is(focused_output, "fake-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 'focus output doesnotexist fake-0 alsodoesnotexist fake-3 fake-2';
+
+    my $out = $order[$i % 3];
+    is(focused_output, "fake-$out", 'focus output next cycle');
+}
+
+done_testing;