From dd400ff74bbf709445473dd99b3c0b8314e63b41 Mon Sep 17 00:00:00 2001 From: hwangcc23 Date: Fri, 16 Oct 2015 23:17:41 +0800 Subject: [PATCH] Revise workspace next/prev See the issue #1798 (http://github.com/i3/i3/issues/1798). +workspace_next+ as-is cycles through either numbered or named workspaces, but when it reaches the last numbered/named workspace, it only looks for named workspaces. This commit changes it: look for named workspaces after exhausting numbered ones, but also for numbered ones after exhausting named ones. Also add a test case 528-workspace-next-prev.t (numbered workspaces and named workspaces on 2 outputs) for testing this. --- docs/userguide | 4 + src/workspace.c | 135 ++++++++++++-------------- testcases/t/528-workspace-next-prev.t | 127 ++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 73 deletions(-) create mode 100644 testcases/t/528-workspace-next-prev.t diff --git a/docs/userguide b/docs/userguide index 007ef69f..c98c25f9 100644 --- a/docs/userguide +++ b/docs/userguide @@ -1849,6 +1849,10 @@ container to workspace next+, +move container to workspace prev+ to move a container to the next/previous workspace and +move container to workspace current+ (the last one makes sense only when used with criteria). ++workspace next+ cycles through either numbered or named workspaces. But when it +reaches the last numbered/named workspace, it looks for named workspaces after +exhausting numbered ones and looks for numbered ones after exhausting named ones. + See <> for how to move a container/workspace to a different RandR output. diff --git a/src/workspace.c b/src/workspace.c index e7a09c70..dc0a596d 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -506,12 +506,33 @@ void workspace_show_by_name(const char *num) { */ Con *workspace_next(void) { Con *current = con_get_workspace(focused); - Con *next = NULL; + Con *next = NULL, *first = NULL, *first_opposite = NULL; Con *output; if (current->num == -1) { /* If currently a named workspace, find next named workspace. */ - next = TAILQ_NEXT(current, nodes); + if ((next = TAILQ_NEXT(current, nodes)) != NULL) + return next; + bool found_current = false; + TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { + /* Skip outputs starting with __, they are internal. */ + if (con_is_internal(output)) + continue; + NODES_FOREACH(output_get_content(output)) { + if (child->type != CT_WORKSPACE) + continue; + if (!first) + first = child; + if (!first_opposite && child->num != -1) + first_opposite = child; + if (child == current) { + found_current = true; + } else if (child->num == -1 && found_current) { + next = child; + return next; + } + } + } } else { /* If currently a numbered workspace, find next numbered workspace. */ TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { @@ -521,6 +542,10 @@ Con *workspace_next(void) { NODES_FOREACH(output_get_content(output)) { if (child->type != CT_WORKSPACE) continue; + if (!first) + first = child; + if (!first_opposite && child->num == -1) + first_opposite = child; if (child->num == -1) break; /* Need to check child against current and next because we are @@ -532,41 +557,9 @@ Con *workspace_next(void) { } } - /* Find next named workspace. */ - if (!next) { - bool found_current = false; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - /* Skip outputs starting with __, they are internal. */ - if (con_is_internal(output)) - continue; - NODES_FOREACH(output_get_content(output)) { - if (child->type != CT_WORKSPACE) - continue; - if (child == current) { - found_current = 1; - } else if (child->num == -1 && (current->num != -1 || found_current)) { - next = child; - goto workspace_next_end; - } - } - } - } + if (!next) + next = first_opposite ? first_opposite : first; - /* Find first workspace. */ - if (!next) { - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - /* Skip outputs starting with __, they are internal. */ - if (con_is_internal(output)) - continue; - NODES_FOREACH(output_get_content(output)) { - if (child->type != CT_WORKSPACE) - continue; - if (!next || (child->num != -1 && child->num < next->num)) - next = child; - } - } - } -workspace_next_end: return next; } @@ -576,7 +569,7 @@ workspace_next_end: */ Con *workspace_prev(void) { Con *current = con_get_workspace(focused); - Con *prev = NULL; + Con *prev = NULL, *first_opposite = NULL, *last = NULL; Con *output; if (current->num == -1) { @@ -584,6 +577,28 @@ Con *workspace_prev(void) { prev = TAILQ_PREV(current, nodes_head, nodes); if (prev && prev->num != -1) prev = NULL; + if (!prev) { + 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)) + continue; + NODES_FOREACH_REVERSE(output_get_content(output)) { + if (child->type != CT_WORKSPACE) + continue; + if (!last) + last = child; + if (!first_opposite && child->num != -1) + first_opposite = child; + if (child == current) { + found_current = true; + } else if (child->num == -1 && found_current) { + prev = child; + goto workspace_prev_end; + } + } + } + } } else { /* If numbered workspace, find previous numbered workspace. */ TAILQ_FOREACH_REVERSE(output, &(croot->nodes_head), nodes_head, nodes) { @@ -591,7 +606,13 @@ Con *workspace_prev(void) { if (con_is_internal(output)) continue; NODES_FOREACH_REVERSE(output_get_content(output)) { - if (child->type != CT_WORKSPACE || child->num == -1) + if (child->type != CT_WORKSPACE) + continue; + if (!last) + last = child; + if (!first_opposite && child->num == -1) + first_opposite = child; + if (child->num == -1) continue; /* Need to check child against current and previous because we * are traversing multiple lists and thus are not guaranteed @@ -602,40 +623,8 @@ Con *workspace_prev(void) { } } - /* Find previous named workspace. */ - if (!prev) { - 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)) - continue; - NODES_FOREACH_REVERSE(output_get_content(output)) { - if (child->type != CT_WORKSPACE) - continue; - if (child == current) { - found_current = true; - } else if (child->num == -1 && (current->num != -1 || found_current)) { - prev = child; - goto workspace_prev_end; - } - } - } - } - - /* Find last workspace. */ - if (!prev) { - TAILQ_FOREACH_REVERSE(output, &(croot->nodes_head), nodes_head, nodes) { - /* Skip outputs starting with __, they are internal. */ - if (con_is_internal(output)) - continue; - NODES_FOREACH_REVERSE(output_get_content(output)) { - if (child->type != CT_WORKSPACE) - continue; - if (!prev || child->num > prev->num) - prev = child; - } - } - } + if (!prev) + prev = first_opposite ? first_opposite : last; workspace_prev_end: return prev; @@ -675,7 +664,7 @@ Con *workspace_next_on_output(void) { if (child->type != CT_WORKSPACE) continue; if (child == current) { - found_current = 1; + found_current = true; } else if (child->num == -1 && (current->num != -1 || found_current)) { next = child; goto workspace_next_on_output_end; diff --git a/testcases/t/528-workspace-next-prev.t b/testcases/t/528-workspace-next-prev.t new file mode 100644 index 00000000..79e83a07 --- /dev/null +++ b/testcases/t/528-workspace-next-prev.t @@ -0,0 +1,127 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • http://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • http://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • http://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) +# +# Tests whether 'workspace next' works correctly. +# +use List::Util qw(first); +use i3test i3_autostart => 0; + +sub assert_next { + my ($expected) = @_; + + 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, $expected, "workspace $expected focused"); +} + + +my $config = <root->warp_pointer(0, 0); +sync_with_i3; + +cmd 'workspace A'; +# ensure workspace A stays open +open_window; + +cmd 'workspace B'; +# ensure workspace B stays open +open_window; + +cmd 'workspace D'; +# ensure workspace D stays open +open_window; + +cmd 'workspace E'; +# ensure workspace E stays open +open_window; + +cmd 'focus output right'; + +cmd 'workspace 1'; +# ensure workspace 1 stays open +open_window; + +cmd 'workspace 2'; +# ensure workspace 2 stays open +open_window; + +cmd 'workspace 3'; +# ensure workspace 3 stays open +open_window; + +cmd 'workspace 4'; +# ensure workspace 4 stays open +open_window; + +cmd 'workspace 5'; +# ensure workspace 5 stays open +open_window; + +cmd 'workspace C'; +# ensure workspace C stays open +open_window; + +cmd 'workspace F'; +# ensure workspace F stays open +open_window; + +cmd 'focus output right'; + +################################################################################ +# Use workspace next and verify the correct order. +################################################################################ + +# The current order should be: +# output 1: A, B, D, E +# output 2: 1, 2, 3, 4, 5, C, F + +cmd 'workspace A'; +is(focused_ws, 'A', 'back on workspace A'); + +assert_next('B'); +assert_next('D'); +assert_next('E'); +assert_next('C'); +assert_next('F'); +assert_next('1'); +assert_next('2'); +assert_next('3'); +assert_next('4'); +assert_next('5'); +assert_next('A'); +assert_next('B'); + +exit_gracefully($pid); + +done_testing;