From 063916718505162b5ebe22268e413787cba32642 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 6 Feb 2024 14:07:21 -0500 Subject: 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 --- .../bugfixes/2-numbered-workspace-prev-next | 1 + src/workspace.c | 36 +++++++++++++++ testcases/t/503-workspace.t | 52 +++++++++++++++++++++- testcases/t/528-workspace-next-prev-reversed.t | 32 +++++++++++-- testcases/t/535-workspace-next-prev.t | 32 +++++++++++-- 5 files changed, 145 insertions(+), 8 deletions(-) create mode 100644 release-notes/bugfixes/2-numbered-workspace-prev-next 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,8 +118,16 @@ 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'; @@ -94,6 +139,9 @@ cmd 'workspace 2'; # later in time and mess up our subsequent tests. 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'); 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'); -- cgit v1.2.3-54-g00ecf