Skip to content

Commit

Permalink
Don't skip identically numbered workspaces when moving to next/prev (i…
Browse files Browse the repository at this point in the history 
…3#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 i3#4452
  • Loading branch information
@rsgowman @desmana
rsgowman authored and desmana committed Apr 3, 2024
1 parent 4d4a6a5 commit 39203b3
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 8 deletions.
1 change: 1 addition & 0 deletions release-notes/bugfixes/2-numbered-workspace-prev-next
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
do not skip identically numbered workspaces when moving to next/prev
36 changes: 36 additions & 0 deletions src/workspace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
}
}
}
}
Expand All @@ -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;
Expand All @@ -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;
}
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
}

Expand Down
52 changes: 50 additions & 2 deletions testcases/t/503-workspace.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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';
Expand All @@ -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');

Expand Down
32 changes: 29 additions & 3 deletions testcases/t/528-workspace-next-prev-reversed.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
################################################################################

Expand All @@ -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;
Expand All @@ -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';
Expand All @@ -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');
Expand All @@ -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');
Expand Down
32 changes: 29 additions & 3 deletions testcases/t/535-workspace-next-prev.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
################################################################################

Expand All @@ -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;
Expand All @@ -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';
Expand All @@ -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');
Expand All @@ -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');
Expand Down

0 comments on commit 39203b3

Please sign in to comment.