Don’t force wrapping when focusing in a direction would work (+test)

Think of the following layout:

 -------------
 | tab |     |
 | con | win |
 |     |     |
 -------------

The tabbed container on the left has two children. Assume you have focused the
second/right child in the tabbed container. i3 used to focus the first/left
container of the tabbed container when using 'focus right' (it wrapped focus).

With this commit, the default behaviour is to instead focus the window on the
right of the screen.

The intention is to make focus switching more intuitive, especially with tabbed
containers supporting 'focus left'/'focus right' in tree. You should end up
using less 'focus parent' :).

You can force the old behaviour with 'force_focus_wrapping true' in your
config.

Code coverage is 62.5% with this commit.

include/config.h

@@ -111,6 +111,16 @@ struct Config {
          * comes with i3. Thus, you can turn it off entirely. */
         bool disable_workspace_bar;
 
+        /** Think of the following layout: Horizontal workspace with a tabbed
+         * con on the left of the screen and a terminal on the right of the
+         * screen. You are in the second container in the tabbed container and
+         * focus to the right. By default, i3 will set focus to the terminal on
+         * the right. If you are in the first container in the tabbed container
+         * however, focusing to the left will wrap. This option forces i3 to
+         * always wrap, which will result in you having to use "focus parent"
+         * more often. */
+        bool force_focus_wrapping;
+
         /** The default border style for new windows. */
         border_style_t default_border;
 

src/cfgparse.l

@@ -122,6 +122,7 @@ normal                          { return TOK_NORMAL; }
 none                            { return TOK_NONE; }
 1pixel                          { return TOK_1PIXEL; }
 focus_follows_mouse             { return TOKFOCUSFOLLOWSMOUSE; }
+force_focus_wrapping            { return TOK_FORCE_FOCUS_WRAPPING; }
 workspace_bar                   { return TOKWORKSPACEBAR; }
 popup_during_fullscreen         { return TOK_POPUP_DURING_FULLSCREEN; }
 ignore                          { return TOK_IGNORE; }

src/cfgparse.y

@@ -235,6 +235,7 @@ void parse_file(const char *f) {
 %token                  TOK_NONE                    "none"
 %token                  TOK_1PIXEL                  "1pixel"
 %token                  TOKFOCUSFOLLOWSMOUSE        "focus_follows_mouse"
+%token                  TOK_FORCE_FOCUS_WRAPPING    "force_focus_wrapping"
 %token                  TOKWORKSPACEBAR             "workspace_bar"
 %token                  TOK_DEFAULT                 "default"
 %token                  TOK_STACKING                "stacking"
@@ -285,6 +286,7 @@ line:
     | workspace_layout
     | new_window
     | focus_follows_mouse
+    | force_focus_wrapping
     | workspace_bar
     | workspace
     | assign
@@ -592,6 +594,14 @@ focus_follows_mouse:
     }
     ;
 
+force_focus_wrapping:
+    TOK_FORCE_FOCUS_WRAPPING bool
+    {
+        DLOG("force focus wrapping = %d\n", $2);
+        config.force_focus_wrapping = $2;
+    }
+    ;
+
 workspace_bar:
     TOKWORKSPACEBAR bool
     {

src/tree.c

@@ -362,49 +362,71 @@ void tree_render() {
 }
 
 /*
- * Changes focus in the given way (next/previous) and given orientation
- * (horizontal/vertical).
+ * Recursive function to walk the tree until a con can be found to focus.
  *
  */
-void tree_next(char way, orientation_t orientation) {
-    /* 1: get the first parent with the same orientation */
-    Con *parent = focused->parent;
-    while (focused->type != CT_WORKSPACE &&
-           (con_orientation(parent) != orientation ||
-            con_num_children(parent) == 1)) {
-        LOG("need to go one level further up\n");
-        /* if the current parent is an output, we are at a workspace
-         * and the orientation still does not match */
-        if (parent->type == CT_WORKSPACE)
-            return;
-        parent = parent->parent;
+static bool _tree_next(Con *con, char way, orientation_t orientation, bool wrap) {
+    /* Stop recursing at workspaces */
+    if (con->type == CT_WORKSPACE)
+        return false;
+
+    if (con->type == CT_FLOATING_CON) {
+        /* TODO: implement focus for floating windows */
+        return false;
     }
+
+    Con *parent = con->parent;
+
+    /* 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));
-    assert(current != TAILQ_END(&(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 here, move along...\n");
-        return;
+        DLOG("nothing to focus\n");
+        return false;
     }
 
-    /* 2: chose next (or previous) */
     Con *next;
-    if (way == 'n') {
+    if (way == 'n')
         next = TAILQ_NEXT(current, nodes);
-        /* if we are at the end of the list, we need to wrap */
-        if (next == TAILQ_END(&(parent->nodes_head)))
+    else next = TAILQ_PREV(current, nodes_head, nodes);
+
+    if (!next) {
+        if (!config.force_focus_wrapping) {
+            /* 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_PREV(current, nodes_head, nodes);
-        /* if we are at the end of the list, we need to wrap */
-        if (next == TAILQ_END(&(parent->nodes_head)))
-            next = TAILQ_LAST(&(parent->nodes_head), nodes_head);
+        else next = TAILQ_LAST(&(parent->nodes_head), nodes_head);
     }
 
     /* 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 */
     con_focus(con_descend_focused(next));
+    return true;
+}
+
+/*
+ * Changes focus in the given way (next/previous) and given orientation
+ * (horizontal/vertical).
+ *
+ */
+void tree_next(char way, orientation_t orientation) {
+    _tree_next(focused, way, orientation, true);
 }
 
 /*

testcases/t/70-force_focus_wrapping.t

@@ -0,0 +1,110 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+# !NO_I3_INSTANCE! will prevent complete-run.pl from starting i3
+#
+# Tests if the 'force_focus_wrapping' config directive works correctly.
+#
+use i3test;
+use Cwd qw(abs_path);
+use Proc::Background;
+use File::Temp qw(tempfile tempdir);
+use X11::XCB qw(:all);
+use X11::XCB::Connection;
+
+my $x = X11::XCB::Connection->new;
+
+# assuming we are run by complete-run.pl
+my $i3_path = abs_path("../i3");
+
+#####################################################################
+# 1: test the wrapping behaviour without force_focus_wrapping
+#####################################################################
+
+my ($fh, $tmpfile) = tempfile();
+say $fh "font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1";
+say $fh "ipc-socket /tmp/nestedcons";
+close($fh);
+
+diag("Starting i3");
+my $i3cmd = "exec " . abs_path("../i3") . " -V -d all --disable-signalhandler -c $tmpfile >/dev/null 2>/dev/null";
+my $process = Proc::Background->new($i3cmd);
+sleep 1;
+
+diag("pid = " . $process->pid);
+
+my $tmp = fresh_workspace;
+
+ok(@{get_ws_content($tmp)} == 0, 'no containers yet');
+
+my $first = open_standard_window($x);
+my $second = open_standard_window($x);
+
+cmd 'layout tabbed';
+cmd 'focus parent';
+
+my $third = open_standard_window($x);
+is($x->input_focus, $third->id, 'third window focused');
+
+cmd 'focus left';
+is($x->input_focus, $second->id, 'second window focused');
+
+cmd 'focus left';
+is($x->input_focus, $first->id, 'first window focused');
+
+# now test the wrapping
+cmd 'focus left';
+is($x->input_focus, $second->id, 'second window focused');
+
+# but focusing right should not wrap now, but instead focus the third window
+cmd 'focus right';
+is($x->input_focus, $third->id, 'third window focused');
+
+exit_gracefully($process->pid);
+
+#####################################################################
+# 2: test the wrapping behaviour with force_focus_wrapping
+#####################################################################
+
+($fh, $tmpfile) = tempfile();
+say $fh "font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1";
+say $fh "ipc-socket /tmp/nestedcons";
+say $fh "force_focus_wrapping true";
+close($fh);
+
+diag("Starting i3");
+$i3cmd = "exec " . abs_path("../i3") . " -V -d all --disable-signalhandler -c $tmpfile >/dev/null 2>/dev/null";
+$process = Proc::Background->new($i3cmd);
+sleep 1;
+
+diag("pid = " . $process->pid);
+
+$tmp = fresh_workspace;
+
+ok(@{get_ws_content($tmp)} == 0, 'no containers yet');
+
+$first = open_standard_window($x);
+$second = open_standard_window($x);
+
+cmd 'layout tabbed';
+cmd 'focus parent';
+
+$third = open_standard_window($x);
+is($x->input_focus, $third->id, 'third window focused');
+
+cmd 'focus left';
+is($x->input_focus, $second->id, 'second window focused');
+
+cmd 'focus left';
+is($x->input_focus, $first->id, 'first window focused');
+
+# now test the wrapping
+cmd 'focus left';
+is($x->input_focus, $second->id, 'second window focused');
+
+# focusing right should now be forced to wrap
+cmd 'focus right';
+is($x->input_focus, $first->id, 'first window focused');
+
+exit_gracefully($process->pid);
+
+done_testing;