Don't skip identically numbered workspaces when moving to next/prev (#4578)

eg if you have workspaces: { 1, 2:a, 2:b, 3 } and are on workspace 1,
then 'workspace next' should traverse 1 -> 2:a -> 2:b -> 3 -> 1 instead
of 1 -> 2:a -> 3 -> 1.

Fixes #4452

5 files changed, 145 insertions, 8 deletions

diff --git a/release-notes/bugfixes/2-numbered-workspace-prev-next b/release-notes/bugfixes/2-numbered-workspace-prev-next
new file mode 100644
index 00000000..bd0704fa
--- /dev/null
+++ b/release-notes/bugfixes/2-numbered-workspace-prev-next
@@ -0,0 +1 @@
+do not skip identically numbered workspaces when moving to next/prev
diff --git a/src/workspace.c b/src/workspace.c
index 1cacebf3..8c58b9a8 100644
--- a/src/workspace.c
+++ b/src/workspace.c
@@ -614,6 +614,7 @@ Con *workspace_next(void) {
         }
     } else {
         /* If currently a numbered workspace, find next numbered workspace. */
+        bool found_current = false;
         TAILQ_FOREACH (output, &(croot->nodes_head), nodes) {
             /* Skip outputs starting with __, they are internal. */
             if (con_is_internal(output)) {
@@ -638,6 +639,14 @@ Con *workspace_next(void) {
                 if (current->num < child->num && (!next || child->num < next->num)) {
                     next = child;
                 }
+
+                /* If two workspaces have the same number, but different names
+                 * (eg '5:a', '5:b') then just take the next one. */
+                if (child == current) {
+                    found_current = true;
+                } else if (found_current && current->num == child->num) {
+                    return child;
+                }
             }
         }
     }
@@ -692,6 +701,7 @@ Con *workspace_prev(void) {
         }
     } else {
         /* If numbered workspace, find previous numbered workspace. */
+        bool found_current = false;
         TAILQ_FOREACH_REVERSE (output, &(croot->nodes_head), nodes_head, nodes) {
             /* Skip outputs starting with __, they are internal. */
             if (con_is_internal(output)) {
@@ -716,6 +726,14 @@ Con *workspace_prev(void) {
                 if (current->num > child->num && (!prev || child->num > prev->num)) {
                     prev = child;
                 }
+
+                /* If two workspaces have the same number, but different names
+                 * (eg '5:a', '5:b') then just take the previous one. */
+                if (child == current) {
+                    found_current = true;
+                } else if (found_current && current->num == child->num) {
+                    return child;
+                }
             }
         }
     }
@@ -741,6 +759,7 @@ Con *workspace_next_on_output(void) {
         next = TAILQ_NEXT(current, nodes);
     } else {
         /* If currently a numbered workspace, find next numbered workspace. */
+        bool found_current = false;
         NODES_FOREACH (output_get_content(output)) {
             if (child->type != CT_WORKSPACE) {
                 continue;
@@ -754,6 +773,14 @@ Con *workspace_next_on_output(void) {
             if (current->num < child->num && (!next || child->num < next->num)) {
                 next = child;
             }
+
+            /* If two workspaces have the same number, but different names
+             * (eg '5:a', '5:b') then just take the next one. */
+            if (child == current) {
+                found_current = true;
+            } else if (found_current && current->num == child->num) {
+                return child;
+            }
         }
     }
 
@@ -806,6 +833,7 @@ Con *workspace_prev_on_output(void) {
         }
     } else {
         /* If numbered workspace, find previous numbered workspace. */
+        bool found_current = false;
         NODES_FOREACH_REVERSE (output_get_content(output)) {
             if (child->type != CT_WORKSPACE || child->num == -1) {
                 continue;
@@ -816,6 +844,14 @@ Con *workspace_prev_on_output(void) {
             if (current->num > child->num && (!prev || child->num > prev->num)) {
                 prev = child;
             }
+
+            /* If two workspaces have the same number, but different names
+             * (eg '5:a', '5:b') then just take the previous one. */
+            if (child == current) {
+                found_current = true;
+            } else if (found_current && current->num == child->num) {
+                return child;
+            }
         }
     }
 
diff --git a/testcases/t/503-workspace.t b/testcases/t/503-workspace.t
index 04ac8a59..8a4aab9f 100644
--- a/testcases/t/503-workspace.t
+++ b/testcases/t/503-workspace.t
@@ -41,6 +41,10 @@ is(focused_ws, '2', 'workspace 2 on second output');
 # ensure workspace 2 stays open
 open_window;
 
+cmd 'workspace 6:c';
+# ensure workspace 6:c stays open
+open_window;
+
 cmd 'focus output right';
 is(focused_ws, '1', 'back on workspace 1');
 
@@ -50,13 +54,25 @@ cmd 'workspace 5';
 # ensure workspace 5 stays open
 open_window;
 
+# numbered w/ name workspaces must be created in reverse order compared to
+# other workspace types (because a new numbered w/ name workspace is prepended
+# to the list of similarly numbered workspaces).
+
+cmd 'workspace 6:b';
+# ensure workspace 5 stays open
+open_window;
+
+cmd 'workspace 6:a';
+# ensure workspace 5 stays open
+open_window;
+
 ################################################################################
 # Use workspace next and verify the correct order.
 ################################################################################
 
 # The current order should be:
-# output 1: 1, 5
-# output 2: 2
+# output 1: 1, 5, 6:a, 6:b
+# output 2: 2, 6:c
 cmd 'workspace 1';
 cmd 'workspace next';
 # We need to sync after changing focus to a different output to wait for the
@@ -72,6 +88,27 @@ cmd 'workspace next';
 sync_with_i3;
 
 is(focused_ws, '5', 'workspace 5 focused');
+cmd 'workspace next';
+# We need to sync after changing focus to a different output to wait for the
+# EnterNotify to be processed, otherwise it will be processed at some point
+# later in time and mess up our subsequent tests.
+sync_with_i3;
+
+is(focused_ws, '6:a', 'workspace 6:a focused');
+cmd 'workspace next';
+# We need to sync after changing focus to a different output to wait for the
+# EnterNotify to be processed, otherwise it will be processed at some point
+# later in time and mess up our subsequent tests.
+sync_with_i3;
+
+is(focused_ws, '6:b', 'workspace 6:b focused');
+cmd 'workspace next';
+# We need to sync after changing focus to a different output to wait for the
+# EnterNotify to be processed, otherwise it will be processed at some point
+# later in time and mess up our subsequent tests.
+sync_with_i3;
+
+is(focused_ws, '6:c', 'workspace 6:b focused');
 
 ################################################################################
 # Now try the same with workspace next_on_output.
@@ -81,9 +118,17 @@ cmd 'workspace 1';
 cmd 'workspace next_on_output';
 is(focused_ws, '5', 'workspace 5 focused');
 cmd 'workspace next_on_output';
+is(focused_ws, '6:a', 'workspace 6:a focused');
+cmd 'workspace next_on_output';
+is(focused_ws, '6:b', 'workspace 6:b focused');
+cmd 'workspace next_on_output';
 is(focused_ws, '1', 'workspace 1 focused');
 
 cmd 'workspace prev_on_output';
+is(focused_ws, '6:b', 'workspace 6:b focused');
+cmd 'workspace prev_on_output';
+is(focused_ws, '6:a', 'workspace 6:a focused');
+cmd 'workspace prev_on_output';
 is(focused_ws, '5', 'workspace 5 focused');
 cmd 'workspace prev_on_output';
 is(focused_ws, '1', 'workspace 1 focused');
@@ -95,6 +140,9 @@ cmd 'workspace 2';
 sync_with_i3;
 
 cmd 'workspace prev_on_output';
+is(focused_ws, '6:c', 'workspace 6:c focused');
+
+cmd 'workspace prev_on_output';
 is(focused_ws, '2', 'workspace 2 focused');
 
 done_testing;
diff --git a/testcases/t/528-workspace-next-prev-reversed.t b/testcases/t/528-workspace-next-prev-reversed.t
index b58457b8..f5e11466 100644
--- a/testcases/t/528-workspace-next-prev-reversed.t
+++ b/testcases/t/528-workspace-next-prev-reversed.t
@@ -56,9 +56,9 @@ sync_with_i3;
 # Setup workspaces so that they stay open (with an empty container).
 # open_window ensures, this
 #
-#                   numbered       named
-# output 1 (left) : 1, 2, 3, 6, 7, B, F, C
-# output 2 (right): 4, 5,          A, D, E
+#                   numbered        numbered w/ names  named
+# output 1 (left):  4, 5,           8:d, 8:e,          A, D, E
+# output 2 (right): 1, 2, 3, 6, 7,  8:a, 8:b, 8:c      B, F, C
 #
 ################################################################################
 
@@ -68,6 +68,11 @@ cmd 'workspace D'; open_window;
 cmd 'workspace 4'; open_window;
 cmd 'workspace 5'; open_window;
 cmd 'workspace E'; open_window;
+# numbered w/ name workspaces must be created in reverse order compared to
+# other workspace types (because a new numbered w/ name workspace is prepended
+# to the list of similarly numbered workspaces).
+cmd 'workspace 8:e'; open_window;
+cmd 'workspace 8:d'; open_window;
 
 cmd 'focus output right';
 cmd 'workspace 1'; open_window;
@@ -78,10 +83,19 @@ cmd 'workspace F'; open_window;
 cmd 'workspace 6'; open_window;
 cmd 'workspace C'; open_window;
 cmd 'workspace 7'; open_window;
+# numbered w/ name workspaces must be created in reverse order compared to
+# other workspace types (because a new numbered w/ name workspace is prepended
+# to the list of similarly numbered workspaces).
+cmd 'workspace 8:c'; open_window;
+cmd 'workspace 8:b'; open_window;
+cmd 'workspace 8:a'; open_window;
 
 ################################################################################
 # Use workspace next and verify the correct order.
 # numbered -> numerical sort
+# numbered w/ names -> numerical sort. Workspaces with the same number but
+#     different names sort by output, followed by reverse creation time on each
+#     output.
 # named -> sort by creation time
 ################################################################################
 cmd 'workspace 1';
@@ -94,6 +108,12 @@ assert_next('5');
 assert_next('6');
 assert_next('7');
 
+assert_next('8:a');
+assert_next('8:b');
+assert_next('8:c');
+assert_next('8:d');
+assert_next('8:e');
+
 assert_next('B');
 assert_next('F');
 assert_next('C');
@@ -112,6 +132,12 @@ assert_prev('C');
 assert_prev('F');
 assert_prev('B');
 
+assert_prev('8:e');
+assert_prev('8:d');
+assert_prev('8:c');
+assert_prev('8:b');
+assert_prev('8:a');
+
 assert_prev('7');
 assert_prev('6');
 assert_prev('5');
diff --git a/testcases/t/535-workspace-next-prev.t b/testcases/t/535-workspace-next-prev.t
index 0020586e..8510370c 100644
--- a/testcases/t/535-workspace-next-prev.t
+++ b/testcases/t/535-workspace-next-prev.t
@@ -56,9 +56,9 @@ sync_with_i3;
 # Setup workspaces so that they stay open (with an empty container).
 # open_window ensures, this
 #
-#                   numbered       named
-# output 1 (left) : 1, 2, 3, 6, 7, B, F, C
-# output 2 (right): 4, 5,          A, D, E
+#                   numbered        numbered w/ names  named
+# output 1 (left) : 1, 2, 3, 6, 7,  8:a, 8:b, 8:c      B, F, C
+# output 2 (right): 4, 5,           8:d, 8:e,          A, D, E
 #
 ################################################################################
 
@@ -68,6 +68,11 @@ cmd 'workspace D'; open_window;
 cmd 'workspace 4'; open_window;
 cmd 'workspace 5'; open_window;
 cmd 'workspace E'; open_window;
+# numbered w/ name workspaces must be created in reverse order compared to
+# other workspace types (because a new numbered w/ name workspace is prepended
+# to the list of similarly numbered workspaces).
+cmd 'workspace 8:e'; open_window;
+cmd 'workspace 8:d'; open_window;
 
 cmd 'focus output left';
 cmd 'workspace 1'; open_window;
@@ -78,10 +83,19 @@ cmd 'workspace F'; open_window;
 cmd 'workspace 6'; open_window;
 cmd 'workspace C'; open_window;
 cmd 'workspace 7'; open_window;
+# numbered w/ name workspaces must be created in reverse order compared to
+# other workspace types (because a new numbered w/ name workspace is prepended
+# to the list of similarly numbered workspaces).
+cmd 'workspace 8:c'; open_window;
+cmd 'workspace 8:b'; open_window;
+cmd 'workspace 8:a'; open_window;
 
 ################################################################################
 # Use workspace next and verify the correct order.
 # numbered -> numerical sort
+# numbered w/ names -> numerical sort. Workspaces with the same number but
+#     different names sort by output, followed by reverse creation time on each
+#     output.
 # named -> sort by creation time
 ################################################################################
 cmd 'workspace 1';
@@ -94,6 +108,12 @@ assert_next('5');
 assert_next('6');
 assert_next('7');
 
+assert_next('8:a');
+assert_next('8:b');
+assert_next('8:c');
+assert_next('8:d');
+assert_next('8:e');
+
 assert_next('B');
 assert_next('F');
 assert_next('C');
@@ -112,6 +132,12 @@ assert_prev('C');
 assert_prev('F');
 assert_prev('B');
 
+assert_prev('8:e');
+assert_prev('8:d');
+assert_prev('8:c');
+assert_prev('8:b');
+assert_prev('8:a');
+
 assert_prev('7');
 assert_prev('6');
 assert_prev('5');