From 0cd7250f4177ac192f4b97b9ddfab96cbf2e6366 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Mon, 14 Oct 2019 02:28:09 +0300 Subject: [PATCH 1/6] Add testcases/t/308-focus_wrapping.t These tests pass with and without the following refactoring. --- testcases/t/308-focus_wrapping.t | 255 +++++++++++++++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 testcases/t/308-focus_wrapping.t diff --git a/testcases/t/308-focus_wrapping.t b/testcases/t/308-focus_wrapping.t new file mode 100644 index 00000000..7053b5ae --- /dev/null +++ b/testcases/t/308-focus_wrapping.t @@ -0,0 +1,255 @@ +#!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) +# +# Tests focus_wrapping yes|no|force|workspace with cmp_tree +# Tickets: #2352 +use i3test i3_autostart => 0; + +my $pid = 0; +sub focus_wrapping { + my ($setting) = @_; + + print "--------------------------------------------------------------------------------\n"; + print " focus_wrapping $setting\n"; + print "--------------------------------------------------------------------------------\n"; + exit_gracefully($pid) if $pid > 0; + + my $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+1024+768,1024x768+0+768 +workspace left-top output fake-0 +workspace right-top output fake-1 +workspace right-bottom output fake-2 +workspace left-bottom output fake-3 + +focus_wrapping $setting +EOT + $pid = launch_with_config($config); +} + +############################################################################### +focus_wrapping('yes'); +############################################################################### + +cmp_tree( + msg => 'Normal focus up - should work for all options', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Normal focus right - should work for all options', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b] V[c d T[e f* g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + }); +cmp_tree( + msg => 'Focus leaves workspace vertically', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus down'; + is(focused_ws, 'left-bottom', 'Correct workspace focused'); + }); +cmp_tree( + msg => 'Focus wraps vertically', + layout_before => 'S[a* b] V[c d T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Focus wraps horizontally', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g*]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Directional focus in the orientation of the parent does not wrap', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Focus leaves workspace horizontally', + layout_before => 'S[a b] V[c d* T[e f g*]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + is(focused_ws, 'right-top', 'Correct workspace focused'); + }); + +############################################################################### +focus_wrapping('no'); +# See issue #2352 +############################################################################### + +cmp_tree( + msg => 'Normal focus up - should work for all options', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Normal focus right - should work for all options', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b] V[c d T[e f* g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + }); +cmp_tree( + msg => 'Focus leaves workspace vertically', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus down'; + is(focused_ws, 'left-bottom', 'Correct workspace focused'); + }); +cmp_tree( + msg => 'Focus does not wrap vertically', + layout_before => 'S[a* b] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Focus does not wrap horizontally', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Directional focus in the orientation of the parent does not wrap', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Focus leaves workspace horizontally', + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + is(focused_ws, 'right-top', 'Correct workspace focused'); + }); + +############################################################################### +focus_wrapping('force'); +############################################################################### + +cmp_tree( + msg => 'Normal focus up - should work for all options', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Normal focus right - should work for all options', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b] V[c d T[e f* g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + }); +cmp_tree( + msg => 'Focus does not leave workspace vertically', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus down'; + is(focused_ws, 'left-top', 'Correct workspace focused'); + }); +cmp_tree( + msg => 'Focus wraps vertically', + layout_before => 'S[a* b] V[c d T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Focus wraps horizontally (focus direction different than parent\'s orientation)', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g*]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Directional focus in the orientation of the parent wraps', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b] V[c d T[e f g*]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( # 'focus_wrapping force' exclusive test + msg => 'But leaves when selecting parent', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus parent, focus right'; + }); +cmp_tree( + msg => 'Focus does not leave workspace horizontally', + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + is(focused_ws, 'left-top', 'Correct workspace focused'); + }); +cmp_tree( # 'focus_wrapping force|workspace' exclusive test + msg => 'But leaves when selecting parent x2', + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus parent, focus parent, focus right'; + is(focused_ws, 'right-top', 'Correct workspace focused'); + }); + +exit_gracefully($pid); + + +done_testing; From e5c430e4194910cacc0d50e66b599b7187a94343 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 14 Sep 2018 19:17:47 +0300 Subject: [PATCH 2/6] tree_move: Use direction_t --- include/move.h | 5 ++--- src/move.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/move.h b/include/move.h index df644a6b..96a2cef7 100644 --- a/include/move.h +++ b/include/move.h @@ -12,11 +12,10 @@ #include /** - * Moves the given container in the given direction (TOK_LEFT, TOK_RIGHT, - * TOK_UP, TOK_DOWN from cmdparse.l) + * Moves the given container in the given direction * */ -void tree_move(Con *con, int direction); +void tree_move(Con *con, direction_t direction); typedef enum { BEFORE, AFTER } position_t; diff --git a/src/move.c b/src/move.c index e28a91c6..12277fce 100644 --- a/src/move.c +++ b/src/move.c @@ -243,11 +243,10 @@ static void move_to_output_directed(Con *con, direction_t direction) { } /* - * Moves the given container in the given direction (D_LEFT, D_RIGHT, - * D_UP, D_DOWN). + * Moves the given container in the given direction * */ -void tree_move(Con *con, int direction) { +void tree_move(Con *con, direction_t direction) { position_t position; Con *target; From 1e8e4d3e7ff0110346f43b8f32c2eca00959b501 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 14 Sep 2018 20:18:04 +0300 Subject: [PATCH 3/6] Introduce direction / orientation / position conversion functions --- include/data.h | 2 ++ include/move.h | 3 --- include/util.h | 12 ++++++++++++ src/util.c | 20 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/include/data.h b/include/data.h index 8cf38790..b8c31c52 100644 --- a/include/data.h +++ b/include/data.h @@ -59,6 +59,8 @@ typedef enum { D_LEFT, typedef enum { NO_ORIENTATION = 0, HORIZ, VERT } orientation_t; +typedef enum { BEFORE, + AFTER } position_t; typedef enum { BS_NORMAL = 0, BS_NONE = 1, BS_PIXEL = 2 } border_style_t; diff --git a/include/move.h b/include/move.h index 96a2cef7..830488b0 100644 --- a/include/move.h +++ b/include/move.h @@ -17,9 +17,6 @@ */ void tree_move(Con *con, direction_t direction); -typedef enum { BEFORE, - AFTER } position_t; - /** * This function detaches 'con' from its parent and inserts it either before or * after 'target'. diff --git a/include/util.h b/include/util.h index 7a2b3083..ad7195ee 100644 --- a/include/util.h +++ b/include/util.h @@ -181,3 +181,15 @@ ssize_t slurp(const char *path, char **buf); * */ orientation_t orientation_from_direction(direction_t direction); + +/** + * Convert a direction to its corresponding position. + * + */ +position_t position_from_direction(direction_t direction); + +/** + * Convert orientation and position to the corresponding direction. + * + */ +direction_t direction_from_orientation_position(orientation_t orientation, position_t position); diff --git a/src/util.c b/src/util.c index 812aad37..8c84a436 100644 --- a/src/util.c +++ b/src/util.c @@ -518,3 +518,23 @@ ssize_t slurp(const char *path, char **buf) { orientation_t orientation_from_direction(direction_t direction) { return (direction == D_LEFT || direction == D_RIGHT) ? HORIZ : VERT; } + +/* + * Convert a direction to its corresponding position. + * + */ +position_t position_from_direction(direction_t direction) { + return (direction == D_LEFT || direction == D_UP) ? BEFORE : AFTER; +} + +/* + * Convert orientation and position to the corresponding direction. + * + */ +direction_t direction_from_orientation_position(orientation_t orientation, position_t position) { + if (orientation == HORIZ) { + return position == BEFORE ? D_LEFT : D_RIGHT; + } else { + return position == BEFORE ? D_UP : D_DOWN; + } +} From f402f4570244203adf0574524a48a3538d5d9f67 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 14 Sep 2018 19:12:27 +0300 Subject: [PATCH 4/6] Introduce CMD_FOCUS_WARN_CHILDREN --- src/commands.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/commands.c b/src/commands.c index ff1b2720..24a5bd2f 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1213,6 +1213,21 @@ void cmd_exec(I3_CMD, const char *nosn, const char *command) { ysuccess(true); } +#define CMD_FOCUS_WARN_CHILDREN \ + do { \ + int count = 0; \ + owindow *current; \ + TAILQ_FOREACH(current, &owindows, owindows) { \ + count++; \ + } \ + \ + if (count > 1) { \ + LOG("WARNING: Your criteria for the focus command matches %d containers, " \ + "while only exactly one container can be focused at a time.\n", \ + count); \ + } \ + } while (0) + /* * Implementation of 'focus left|right|up|down'. * @@ -1315,12 +1330,15 @@ void cmd_focus(I3_CMD) { ELOG("Example: [class=\"urxvt\" title=\"irssi\"] focus\n"); yerror("You have to specify which window/container should be focused"); - + return; + } else if (TAILQ_EMPTY(&owindows)) { + yerror("No window matches given criteria"); return; } + CMD_FOCUS_WARN_CHILDREN; + Con *__i3_scratch = workspace_get("__i3_scratch", NULL); - int count = 0; owindow *current; TAILQ_FOREACH(current, &owindows, owindows) { Con *ws = con_get_workspace(current->con); @@ -1332,7 +1350,6 @@ void cmd_focus(I3_CMD) { /* In case this is a scratchpad window, call scratchpad_show(). */ if (ws == __i3_scratch) { scratchpad_show(current->con); - count++; /* While for the normal focus case we can change focus multiple * times and only a single window ends up focused, we could show * multiple scratchpad windows. So, rather break here. */ @@ -1341,16 +1358,10 @@ void cmd_focus(I3_CMD) { LOG("focusing %p / %s\n", current->con, current->con->name); con_activate_unblock(current->con); - count++; } - if (count > 1) - LOG("WARNING: Your criteria for the focus command matches %d containers, " - "while only exactly one container can be focused at a time.\n", - count); - cmd_output->needs_tree_render = true; - ysuccess(count > 0); + ysuccess(true); } /* From bbc4c99c724017c5f2b224f0137715ed43407326 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 14 Sep 2018 18:34:43 +0300 Subject: [PATCH 5/6] Refactor tree_next - Makes `tree_next` not recursive. - Adds `focus next|prev [sibling]` command. See (1.) and (2.) in https://github.com/i3/i3/issues/2587#issuecomment-378505551 (Issue also requests move command, not implemented here). - Directional focus command now supports command criteria. Wrapping is not implemented inside a floating container. This was also true before the refactor so I am not changing it here. --- docs/userguide | 7 + include/commands.h | 6 + include/tree.h | 11 +- parser-specs/commands.spec | 8 + src/click.c | 18 +-- src/commands.c | 69 +++++++-- src/tree.c | 250 +++++++++++++++--------------- testcases/t/101-focus.t | 16 +- testcases/t/307-focus-next-prev.t | 72 +++++++++ 9 files changed, 295 insertions(+), 162 deletions(-) create mode 100644 testcases/t/307-focus-next-prev.t diff --git a/docs/userguide b/docs/userguide index ea65bcd1..a8b0e00d 100644 --- a/docs/userguide +++ b/docs/userguide @@ -2053,6 +2053,12 @@ parent:: child:: The opposite of +focus parent+, sets the focus to the last focused child container. +next|prev:: + Automatically sets focus to the adjacent container. If +sibling+ is + specified, the command will focus the exact sibling container, + including non-leaf containers like split containers. Otherwise, it is + an automatic version of +focus left|right|up|down+ in the orientation + of the parent container. floating:: Sets focus to the last focused floating container. tiling:: @@ -2068,6 +2074,7 @@ output:: focus focus left|right|down|up focus parent|child|floating|tiling|mode_toggle +focus next|prev [sibling] focus output left|right|up|down|primary| ---------------------------------------------- diff --git a/include/commands.h b/include/commands.h index 0137460f..27d631a2 100644 --- a/include/commands.h +++ b/include/commands.h @@ -182,6 +182,12 @@ void cmd_exec(I3_CMD, const char *nosn, const char *command); */ void cmd_focus_direction(I3_CMD, const char *direction); +/** + * Implementation of 'focus next|prev sibling' + * + */ +void cmd_focus_sibling(I3_CMD, const char *direction); + /** * Implementation of 'focus tiling|floating|mode_toggle'. * diff --git a/include/tree.h b/include/tree.h index 12170f94..0b758d53 100644 --- a/include/tree.h +++ b/include/tree.h @@ -59,11 +59,16 @@ bool level_down(void); void tree_render(void); /** - * Changes focus in the given way (next/previous) and given orientation - * (horizontal/vertical). + * Changes focus in the given direction * */ -void tree_next(char way, orientation_t orientation); +void tree_next(Con *con, direction_t direction); + +/** + * Get the previous / next sibling + * + */ +Con *get_tree_next_sibling(Con *con, position_t direction); /** * Closes the given container including all children. diff --git a/parser-specs/commands.spec b/parser-specs/commands.spec index 6b015188..ed9cf0f2 100644 --- a/parser-specs/commands.spec +++ b/parser-specs/commands.spec @@ -146,6 +146,8 @@ state WORKSPACE_NUMBER: state FOCUS: direction = 'left', 'right', 'up', 'down' -> call cmd_focus_direction($direction) + direction = 'prev', 'next' + -> FOCUS_AUTO 'output' -> FOCUS_OUTPUT window_mode = 'tiling', 'floating', 'mode_toggle' @@ -155,6 +157,12 @@ state FOCUS: end -> call cmd_focus() +state FOCUS_AUTO: + 'sibling' + -> call cmd_focus_sibling($direction) + end + -> call cmd_focus_direction($direction) + state FOCUS_OUTPUT: output = string -> call cmd_focus_output($output) diff --git a/src/click.c b/src/click.c index 8ab5c5f0..75710a82 100644 --- a/src/click.c +++ b/src/click.c @@ -212,21 +212,13 @@ static int route_click(Con *con, xcb_button_press_event_t *event, const bool mod event->detail == XCB_BUTTON_SCROLL_LEFT || event->detail == XCB_BUTTON_SCROLL_RIGHT)) { DLOG("Scrolling on a window decoration\n"); - orientation_t orientation = con_orientation(con->parent); /* Use the focused child of the tabbed / stacked container, not the * container the user scrolled on. */ - Con *focused = con->parent; - focused = TAILQ_FIRST(&(focused->focus_head)); - con_activate(con_descend_focused(focused)); - /* To prevent scrolling from going outside the container (see ticket - * #557), we first check if scrolling is possible at all. */ - bool scroll_prev_possible = (TAILQ_PREV(focused, nodes_head, nodes) != NULL); - bool scroll_next_possible = (TAILQ_NEXT(focused, nodes) != NULL); - if ((event->detail == XCB_BUTTON_SCROLL_UP || event->detail == XCB_BUTTON_SCROLL_LEFT) && scroll_prev_possible) { - tree_next('p', orientation); - } else if ((event->detail == XCB_BUTTON_SCROLL_DOWN || event->detail == XCB_BUTTON_SCROLL_RIGHT) && scroll_next_possible) { - tree_next('n', orientation); - } + Con *current = TAILQ_FIRST(&(con->parent->focus_head)); + const position_t direction = + (event->detail == XCB_BUTTON_SCROLL_UP || event->detail == XCB_BUTTON_SCROLL_LEFT) ? BEFORE : AFTER; + Con *next = get_tree_next_sibling(current, direction); + con_activate(con_descend_focused(next ? next : current)); goto done; } diff --git a/src/commands.c b/src/commands.c index 24a5bd2f..5527860c 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1229,23 +1229,62 @@ void cmd_exec(I3_CMD, const char *nosn, const char *command) { } while (0) /* - * Implementation of 'focus left|right|up|down'. + * Implementation of 'focus left|right|up|down|next|prev'. * */ -void cmd_focus_direction(I3_CMD, const char *direction) { - switch (parse_direction(direction)) { - case D_LEFT: - tree_next('p', HORIZ); - break; - case D_RIGHT: - tree_next('n', HORIZ); - break; - case D_UP: - tree_next('p', VERT); - break; - case D_DOWN: - tree_next('n', VERT); - break; +void cmd_focus_direction(I3_CMD, const char *direction_str) { + HANDLE_EMPTY_MATCH; + CMD_FOCUS_WARN_CHILDREN; + + direction_t direction; + position_t position; + bool auto_direction = true; + if (strcmp(direction_str, "prev") == 0) { + position = BEFORE; + } else if (strcmp(direction_str, "next") == 0) { + position = AFTER; + } else { + auto_direction = false; + direction = parse_direction(direction_str); + } + + owindow *current; + TAILQ_FOREACH(current, &owindows, owindows) { + Con *ws = con_get_workspace(current->con); + if (!ws || con_is_internal(ws)) { + continue; + } + if (auto_direction) { + orientation_t o = con_orientation(current->con->parent); + direction = direction_from_orientation_position(o, position); + } + tree_next(current->con, direction); + } + + cmd_output->needs_tree_render = true; + // XXX: default reply for now, make this a better reply + ysuccess(true); +} + +/* + * Implementation of 'focus next|prev sibling' + * + */ +void cmd_focus_sibling(I3_CMD, const char *direction_str) { + HANDLE_EMPTY_MATCH; + CMD_FOCUS_WARN_CHILDREN; + + const position_t direction = (STARTS_WITH(direction_str, "prev")) ? BEFORE : AFTER; + owindow *current; + TAILQ_FOREACH(current, &owindows, owindows) { + Con *ws = con_get_workspace(current->con); + if (!ws || con_is_internal(ws)) { + continue; + } + Con *next = get_tree_next_sibling(current->con, direction); + if (next) { + con_activate(next); + } } cmd_output->needs_tree_render = true; diff --git a/src/tree.c b/src/tree.c index 4057d177..039c3a5e 100644 --- a/src/tree.c +++ b/src/tree.c @@ -462,170 +462,162 @@ void tree_render(void) { DLOG("-- END RENDERING --\n"); } +static Con *get_tree_next_workspace(Con *con, direction_t direction) { + if (con_get_fullscreen_con(con, CF_GLOBAL)) { + DLOG("Cannot change workspace while in global fullscreen mode.\n"); + return NULL; + } + + Output *current_output = get_output_containing(con->rect.x, con->rect.y); + if (!current_output) { + return NULL; + } + DLOG("Current output is %s\n", output_primary_name(current_output)); + + Output *next_output = get_output_next(direction, current_output, CLOSEST_OUTPUT); + if (!next_output) { + return NULL; + } + DLOG("Next output is %s\n", output_primary_name(next_output)); + + /* Find visible workspace on next output */ + Con *workspace = NULL; + GREP_FIRST(workspace, output_get_content(next_output->con), workspace_is_visible(child)); + return workspace; +} + /* - * Recursive function to walk the tree until a con can be found to focus. + * Returns the next / previous container to focus in the given direction. Does + * not modify focus and ensures focus restrictions for fullscreen containers + * are respected. * */ -static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap) { - /* When dealing with fullscreen containers, it's necessary to go up to the - * workspace level, because 'focus $dir' will start at the con's real - * position in the tree, and it may not be possible to get to the edge - * normally due to fullscreen focusing restrictions. */ - if (con->fullscreen_mode == CF_OUTPUT && con->type != CT_WORKSPACE) - con = con_get_workspace(con); +static Con *get_tree_next(Con *con, direction_t direction) { + const bool previous = position_from_direction(direction) == BEFORE; + const orientation_t orientation = orientation_from_direction(direction); - /* Stop recursing at workspaces after attempting to switch to next - * workspace if possible. */ - if (con->type == CT_WORKSPACE) { - if (con_get_fullscreen_con(con, CF_GLOBAL)) { - DLOG("Cannot change workspace while in global fullscreen mode.\n"); - return false; + Con *first_wrap = NULL; + while (con->type != CT_WORKSPACE) { + if (con->fullscreen_mode == CF_OUTPUT) { + /* We've reached a fullscreen container. Directional focus should + * now operate on the workspace level. */ + con = con_get_workspace(con); + break; + } else if (con->fullscreen_mode == CF_GLOBAL) { + /* Focus changes should happen only inside the children of a global + * fullscreen container. */ + return first_wrap; } - Output *current_output = get_output_containing(con->rect.x, con->rect.y); - Output *next_output; - if (!current_output) - return false; - DLOG("Current output is %s\n", output_primary_name(current_output)); + Con *const parent = con->parent; + if (con->type == CT_FLOATING_CON) { + if (orientation != HORIZ) { + /* up/down does not change floating containers */ + return NULL; + } - /* Try to find next output */ - direction_t direction; - if (way == 'n' && orientation == HORIZ) - direction = D_RIGHT; - else if (way == 'p' && orientation == HORIZ) - direction = D_LEFT; - else if (way == 'n' && orientation == VERT) - direction = D_DOWN; - else if (way == 'p' && orientation == VERT) - direction = D_UP; - else - return false; + /* left/right focuses the previous/next floating container */ + Con *next = previous ? TAILQ_PREV(con, floating_head, floating_windows) + : TAILQ_NEXT(con, floating_windows); + /* If there is no next/previous container, wrap */ + if (!next) { + next = previous ? TAILQ_LAST(&(parent->floating_head), floating_head) + : TAILQ_FIRST(&(parent->floating_head)); + } + /* Our parent does not list us in floating heads? */ + assert(next); - next_output = get_output_next(direction, current_output, CLOSEST_OUTPUT); - if (!next_output) - return false; - DLOG("Next output is %s\n", output_primary_name(next_output)); + return next; + } - /* Find visible workspace on next output */ - Con *workspace = NULL; - GREP_FIRST(workspace, output_get_content(next_output->con), workspace_is_visible(child)); + if (con_num_children(parent) > 1 && con_orientation(parent) == orientation) { + Con *const next = previous ? TAILQ_PREV(con, nodes_head, nodes) + : TAILQ_NEXT(con, nodes); + if (next && con_fullscreen_permits_focusing(next)) { + return next; + } + Con *const wrap = previous ? TAILQ_LAST(&(parent->nodes_head), nodes_head) + : TAILQ_FIRST(&(parent->nodes_head)); + switch (config.focus_wrapping) { + case FOCUS_WRAPPING_OFF: + break; + case FOCUS_WRAPPING_ON: + if (!first_wrap && con_fullscreen_permits_focusing(wrap)) { + first_wrap = wrap; + } + break; + case FOCUS_WRAPPING_FORCE: + /* 'force' should always return to ensure focus doesn't + * leave the parent. */ + if (next) { + return NULL; /* blocked by fullscreen */ + } + return con_fullscreen_permits_focusing(wrap) ? wrap : NULL; + } + } + + con = parent; + } + + assert(con->type == CT_WORKSPACE); + Con *workspace = get_tree_next_workspace(con, direction); + return workspace ? workspace : first_wrap; +} + +/* + * Changes focus in the given direction + * + */ +void tree_next(Con *con, direction_t direction) { + Con *next = get_tree_next(con, direction); + if (!next) { + return; + } + if (next->type == CT_WORKSPACE) { /* Show next workspace and focus appropriate container if possible. */ - if (!workspace) - return false; - /* Use descend_focused first to give higher priority to floating or * tiling fullscreen containers. */ - Con *focus = con_descend_focused(workspace); + Con *focus = con_descend_focused(next); if (focus->fullscreen_mode == CF_NONE) { - Con *focus_tiling = con_descend_tiling_focused(workspace); + Con *focus_tiling = con_descend_tiling_focused(next); /* If descend_tiling returned a workspace then focus is either a * floating container or the same workspace. */ - if (focus_tiling != workspace) { + if (focus_tiling != next) { focus = focus_tiling; } } - workspace_show(workspace); + workspace_show(next); con_activate(focus); x_set_warp_to(&(focus->rect)); - return true; - } - - Con *parent = con->parent; - - if (con->type == CT_FLOATING_CON) { - if (orientation != HORIZ) - return false; - - /* left/right focuses the previous/next floating container */ - Con *next; - if (way == 'n') - next = TAILQ_NEXT(con, floating_windows); - else - next = TAILQ_PREV(con, floating_head, floating_windows); - - /* If there is no next/previous container, wrap */ - if (!next) { - if (way == 'n') - next = TAILQ_FIRST(&(parent->floating_head)); - else - next = TAILQ_LAST(&(parent->floating_head), floating_head); - } - - /* Still no next/previous container? bail out */ - if (!next) - return false; - - /* Raise the floating window on top of other windows preserving - * relative stack order */ + return; + } else if (next->type == CT_FLOATING_CON) { + /* Raise the floating window on top of other windows preserving relative + * stack order */ + Con *parent = next->parent; while (TAILQ_LAST(&(parent->floating_head), floating_head) != next) { Con *last = TAILQ_LAST(&(parent->floating_head), floating_head); TAILQ_REMOVE(&(parent->floating_head), last, floating_windows); TAILQ_INSERT_HEAD(&(parent->floating_head), last, floating_windows); } - - con_activate(con_descend_focused(next)); - return true; } - /* If the orientation does not match or there is no other con to focus, we - * need to go higher in the hierarchy */ - if (con_orientation(parent) != orientation || - con_num_children(parent) == 1) - return _tree_next(parent, way, orientation, wrap); - - Con *current = TAILQ_FIRST(&(parent->focus_head)); - /* TODO: when can the following happen (except for floating windows, which - * are handled above)? */ - if (TAILQ_EMPTY(&(parent->nodes_head))) { - DLOG("nothing to focus\n"); - return false; - } - - Con *next; - if (way == 'n') - next = TAILQ_NEXT(current, nodes); - else - next = TAILQ_PREV(current, nodes_head, nodes); - - if (!next) { - if (config.focus_wrapping != FOCUS_WRAPPING_FORCE) { - /* If there is no next/previous container, we check if we can focus one - * when going higher (without wrapping, though). If so, we are done, if - * not, we wrap */ - if (_tree_next(parent, way, orientation, false)) - return true; - - if (!wrap) - return false; - } - - if (way == 'n') - next = TAILQ_FIRST(&(parent->nodes_head)); - else - next = TAILQ_LAST(&(parent->nodes_head), nodes_head); - } - - /* Don't violate fullscreen focus restrictions. */ - if (!con_fullscreen_permits_focusing(next)) - return false; - - /* 3: focus choice comes in here. at the moment we will go down - * until we find a window */ - /* TODO: check for window, atm we only go down as far as possible */ + workspace_show(con_get_workspace(next)); con_activate(con_descend_focused(next)); - return true; } /* - * Changes focus in the given way (next/previous) and given orientation - * (horizontal/vertical). + * Get the previous / next sibling * */ -void tree_next(char way, orientation_t orientation) { - _tree_next(focused, way, orientation, - config.focus_wrapping != FOCUS_WRAPPING_OFF); +Con *get_tree_next_sibling(Con *con, position_t direction) { + Con *to_focus = (direction == BEFORE ? TAILQ_PREV(con, nodes_head, nodes) + : TAILQ_NEXT(con, nodes)); + if (to_focus && con_fullscreen_permits_focusing(to_focus)) { + return to_focus; + } + return NULL; } /* diff --git a/testcases/t/101-focus.t b/testcases/t/101-focus.t index 9d42345d..1e87a544 100644 --- a/testcases/t/101-focus.t +++ b/testcases/t/101-focus.t @@ -35,9 +35,13 @@ my $bottom = open_window; # end sleeping for half a second to make sure i3 reacted # sub focus_after { - my $msg = shift; + my ($msg, $win_id) = @_; - cmd $msg; + if (defined($win_id)) { + cmd "[id=$win_id] $msg"; + } else { + cmd $msg; + } return $x->input_focus; } @@ -50,6 +54,14 @@ is($focus, $mid->id, "Middle window focused"); $focus = focus_after('focus up'); is($focus, $top->id, "Top window focused"); +# Same using command criteria +$focus = focus_after('focus up', $bottom->id); +is($focus, $mid->id, "Middle window focused"); + +cmd 'focus down'; +$focus = focus_after('focus up', $mid->id); +is($focus, $top->id, "Top window focused"); + ##################################################################### # Test focus wrapping ##################################################################### diff --git a/testcases/t/307-focus-next-prev.t b/testcases/t/307-focus-next-prev.t new file mode 100644 index 00000000..c7f06589 --- /dev/null +++ b/testcases/t/307-focus-next-prev.t @@ -0,0 +1,72 @@ +#!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 focus next|prev +# Ticket: #2587 +use i3test; + +cmp_tree( + msg => "cmd 'prev' selects leaf 1/2", + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c* d T[e f g]]', + cb => sub { + cmd 'focus prev'; + }); + +cmp_tree( + msg => "cmd 'prev' selects leaf 2/2", + layout_before => 'S[a b] V[c* d T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + cb => sub { + # c* -> V -> b* + cmd 'focus parent, focus prev'; + }); + +cmp_tree( + msg => "cmd 'prev sibling' selects leaf again", + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c* d T[e f g]]', + cb => sub { + cmd 'focus prev sibling'; + }); + +cmp_tree( + msg => "cmd 'next' selects leaf", + # Notice that g is the last to open before focus moves to d* + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g*]]', + cb => sub { + cmd 'focus next'; + }); + +cmp_tree( + msg => "cmd 'next sibling' selects parent 1/2", + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c d T*[e f g]]', + cb => sub { + cmd 'focus next sibling'; + }); + +cmp_tree( + msg => "cmd 'next sibling' selects parent 2/2", + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V*[c d T[e f g]]', + cb => sub { + # b* -> S* -> V* + cmd 'focus parent, focus next sibling'; + }); + +done_testing; From 24a58d29527cee29643bc512455de845639cdb0e Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Sat, 15 Sep 2018 11:36:48 +0300 Subject: [PATCH 6/6] Implement focus_wrapping workspace I had a dilemma about the behaviour here: 1. Prohibit focus leaving the workspace in any case unless if get_tree_next's initial argument is a workspace. This is what this commit does (also i3-cycle). 2. Leave the workspace if no warp is possible (eg workspace with single container or `focus right` with `V[a b c*]`). Fixes #2180 --- docs/userguide | 7 ++- include/data.h | 3 +- parser-specs/config.spec | 2 +- src/config_directives.c | 2 + src/tree.c | 13 +++++ testcases/t/308-focus_wrapping.t | 98 ++++++++++++++++++++++++++++++-- 6 files changed, 118 insertions(+), 7 deletions(-) diff --git a/docs/userguide b/docs/userguide index a8b0e00d..2b1dde0b 100644 --- a/docs/userguide +++ b/docs/userguide @@ -1105,9 +1105,14 @@ If you want the focus to *always* wrap and you are aware of using +focus parent+ to switch to different containers, you can instead set +focus_wrapping+ to the value +force+. +To restrict focus inside the current workspace set +focus_wrapping+ to the +value +workspace+. You will need to use +focus parent+ until a workspace is +selected to switch to a different workspace using the focus commands (the ++workspace+ command will still work as expected). + *Syntax*: --------------------------- -focus_wrapping yes|no|force +focus_wrapping yes|no|force|workspace # Legacy syntax, equivalent to "focus_wrapping force" force_focus_wrapping yes diff --git a/include/data.h b/include/data.h index b8c31c52..c0e34b41 100644 --- a/include/data.h +++ b/include/data.h @@ -141,7 +141,8 @@ typedef enum { typedef enum { FOCUS_WRAPPING_OFF = 0, FOCUS_WRAPPING_ON = 1, - FOCUS_WRAPPING_FORCE = 2 + FOCUS_WRAPPING_FORCE = 2, + FOCUS_WRAPPING_WORKSPACE = 3 } focus_wrapping_t; /** diff --git a/parser-specs/config.spec b/parser-specs/config.spec index d5b7e063..93901fd9 100644 --- a/parser-specs/config.spec +++ b/parser-specs/config.spec @@ -215,7 +215,7 @@ state MOUSE_WARPING: # focus_wrapping state FOCUS_WRAPPING: - value = '1', 'yes', 'true', 'on', 'enable', 'active', '0', 'no', 'false', 'off', 'disable', 'inactive', 'force' + value = '1', 'yes', 'true', 'on', 'enable', 'active', '0', 'no', 'false', 'off', 'disable', 'inactive', 'force', 'workspace' -> call cfg_focus_wrapping($value) # force_focus_wrapping diff --git a/src/config_directives.c b/src/config_directives.c index f647fb4d..4712296c 100644 --- a/src/config_directives.c +++ b/src/config_directives.c @@ -268,6 +268,8 @@ CFGFUN(disable_randr15, const char *value) { CFGFUN(focus_wrapping, const char *value) { if (strcmp(value, "force") == 0) { config.focus_wrapping = FOCUS_WRAPPING_FORCE; + } else if (strcmp(value, "workspace") == 0) { + config.focus_wrapping = FOCUS_WRAPPING_WORKSPACE; } else if (eval_boolstr(value)) { config.focus_wrapping = FOCUS_WRAPPING_ON; } else { diff --git a/src/tree.c b/src/tree.c index 039c3a5e..a81acce8 100644 --- a/src/tree.c +++ b/src/tree.c @@ -497,6 +497,13 @@ static Con *get_tree_next(Con *con, direction_t direction) { const orientation_t orientation = orientation_from_direction(direction); Con *first_wrap = NULL; + + if (con->type == CT_WORKSPACE) { + /* Special case for FOCUS_WRAPPING_WORKSPACE: allow the focus to leave + * the workspace only when a workspace is selected. */ + goto handle_workspace; + } + while (con->type != CT_WORKSPACE) { if (con->fullscreen_mode == CF_OUTPUT) { /* We've reached a fullscreen container. Directional focus should @@ -542,6 +549,7 @@ static Con *get_tree_next(Con *con, direction_t direction) { switch (config.focus_wrapping) { case FOCUS_WRAPPING_OFF: break; + case FOCUS_WRAPPING_WORKSPACE: case FOCUS_WRAPPING_ON: if (!first_wrap && con_fullscreen_permits_focusing(wrap)) { first_wrap = wrap; @@ -561,6 +569,11 @@ static Con *get_tree_next(Con *con, direction_t direction) { } assert(con->type == CT_WORKSPACE); + if (config.focus_wrapping == FOCUS_WRAPPING_WORKSPACE) { + return first_wrap; + } + +handle_workspace:; Con *workspace = get_tree_next_workspace(con, direction); return workspace ? workspace : first_wrap; } diff --git a/testcases/t/308-focus_wrapping.t b/testcases/t/308-focus_wrapping.t index 7053b5ae..9fa5858c 100644 --- a/testcases/t/308-focus_wrapping.t +++ b/testcases/t/308-focus_wrapping.t @@ -15,10 +15,11 @@ # (unless you are already familiar with Perl) # # Tests focus_wrapping yes|no|force|workspace with cmp_tree -# Tickets: #2352 +# Tickets: #2180 #2352 use i3test i3_autostart => 0; my $pid = 0; + sub focus_wrapping { my ($setting) = @_; @@ -222,7 +223,7 @@ cmp_tree( cb => sub { cmd 'focus left'; }); -cmp_tree( # 'focus_wrapping force' exclusive test +cmp_tree( # 'focus_wrapping force' exclusive test msg => 'But leaves when selecting parent', layout_before => 'S[a b] V[c d T[e* f g]]', layout_after => 'S[a b*] V[c d T[e f g]]', @@ -239,7 +240,7 @@ cmp_tree( cmd 'focus right'; is(focused_ws, 'left-top', 'Correct workspace focused'); }); -cmp_tree( # 'focus_wrapping force|workspace' exclusive test +cmp_tree( # 'focus_wrapping force|workspace' exclusive test msg => 'But leaves when selecting parent x2', layout_before => 'S[a b] V[c d* T[e f g]]', layout_after => 'S[a b] V[c d T[e f g]]', @@ -249,7 +250,96 @@ cmp_tree( # 'focus_wrapping force|workspace' exclusive test is(focused_ws, 'right-top', 'Correct workspace focused'); }); +############################################################################### +focus_wrapping('workspace'); +# See issue #2180 +############################################################################### + +cmp_tree( + msg => 'Normal focus up - should work for all options', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Normal focus right - should work for all options', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b] V[c d T[e f* g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + }); +cmp_tree( + msg => 'Focus does not leave workspace vertically', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a* b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus down'; + is(focused_ws, 'left-top', 'Correct workspace focused'); + }); +cmp_tree( + msg => 'Focus wraps vertically', + layout_before => 'S[a* b] V[c d T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus up'; + }); +cmp_tree( + msg => 'Focus wraps horizontally', + layout_before => 'S[a b*] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g*]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Directional focus in the orientation of the parent does not wrap', + layout_before => 'S[a b] V[c d T[e* f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus left'; + }); +cmp_tree( + msg => 'Focus does not leave workspace horizontally', + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b*] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus right'; + is(focused_ws, 'left-top', 'Correct workspace focused'); + }); +cmp_tree( # 'focus_wrapping force|workspace' exclusive test + msg => 'But leaves when selecting parent x2', + layout_before => 'S[a b] V[c d* T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + cmd 'focus parent, focus parent, focus right'; + is(focused_ws, 'right-top', 'Correct workspace focused'); + }); + +cmp_tree( # 'focus_wrapping workspace' exclusive test + msg => 'x', + layout_before => 'S[a* b] V[c d T[e f g]]', + layout_after => 'S[a b] V[c d T[e f g]]', + ws => 'left-top', + cb => sub { + subtest 'random tests' => sub { + my @directions = qw(left right top down); + for my $i (1 .. 50) { + my $direction = $directions[rand @directions]; + cmd "focus $direction"; + + return unless is(focused_ws, 'left-top', "'focus $direction' did not change workspace"); + } + }; + }); + exit_gracefully($pid); - done_testing;