Extend tiling/floating criteria with optional auto/user values (#4006)

The default `tiling` and `floating` behavior is preserved and matches
both cases.

Adds a new handler to `remanage_window` on A_I3_FLOATING_WINDOW change.

Mainly in order to `run_assignments`, this makes `for_window [floating]`
directives to work for windows which where initially opened as tiling.
Now, when floating is enabled, `for_window` will trigger correctly. Same
applies to `for_window [tiling]`.

The obvious solution of `run_assignments` after
`floating_{enable,disable}` doesn't work because `run_assignments`
modifies the parser state in src/assignments.c:51.

Fixes #3588

Co-Authored-By: Michael Stapelberg <michael@stapelberg.de>

docs/userguide

@@ -1900,8 +1900,18 @@ con_id::
 	to match only the currently focused window.
 floating::
 	Only matches floating windows. This criterion requires no value.
+floating_from::
+	Like +floating+ but this criterion takes two possible values: "auto"
+	and "user". With "auto", only windows that were automatically opened as
+	floating are matched. With "user", only windows that the user made
+	floating are matched.
 tiling::
 	Only matches tiling windows. This criterion requires no value.
+tiling_from::
+	Like +tiling+ but this criterion takes two possible values: "auto" and
+	"user". With "auto", only windows that were automatically opened as
+	tiling are matched. With "user", only windows that the user made tiling
+	are matched.
 
 The criteria +class+, +instance+, +role+, +title+, +workspace+ and +mark+ are
 actually regular expressions (PCRE). See +pcresyntax(3)+ or +perldoc perlre+ for

generate-command-parser.pl

@@ -218,6 +218,8 @@ sub slurp {
             # quote of the literal. We can do strdup(literal + 1); then :).
             $token_name =~ s/'$//;
         }
+        # Escape double quotes:
+        $token_name =~ s,",\\",g;
         my $next_state = $token->{next_state};
         if ($next_state =~ /^call /) {
             ($call_identifier) = ($next_state =~ /^call ([0-9]+)$/);

include/data.h

@@ -530,7 +530,11 @@ struct Match {
     } dock;
     xcb_window_t id;
     enum { WM_ANY = 0,
+           WM_TILING_AUTO,
+           WM_TILING_USER,
            WM_TILING,
+           WM_FLOATING_AUTO,
+           WM_FLOATING_USER,
            WM_FLOATING } window_mode;
     Con *con_id;
 

parser-specs/config.spec

@@ -181,16 +181,18 @@ state NO_FOCUS_END:
 
 # Criteria: Used by for_window and assign.
 state CRITERIA:
-  ctype = 'class'       -> CRITERION
-  ctype = 'instance'    -> CRITERION
-  ctype = 'window_role' -> CRITERION
-  ctype = 'con_id'      -> CRITERION
-  ctype = 'id'          -> CRITERION
-  ctype = 'window_type' -> CRITERION
-  ctype = 'con_mark'    -> CRITERION
-  ctype = 'title'       -> CRITERION
-  ctype = 'urgent'      -> CRITERION
-  ctype = 'workspace'   -> CRITERION
+  ctype = 'class'         -> CRITERION
+  ctype = 'instance'      -> CRITERION
+  ctype = 'window_role'   -> CRITERION
+  ctype = 'con_id'        -> CRITERION
+  ctype = 'id'            -> CRITERION
+  ctype = 'window_type'   -> CRITERION
+  ctype = 'con_mark'      -> CRITERION
+  ctype = 'title'         -> CRITERION
+  ctype = 'urgent'        -> CRITERION
+  ctype = 'workspace'     -> CRITERION
+  ctype = 'floating_from' -> CRITERION_FROM
+  ctype = 'tiling_from'   -> CRITERION_FROM
   ctype = 'tiling', 'floating'
       -> call cfg_criteria_add($ctype, NULL); CRITERIA
   ']'
@@ -199,6 +201,22 @@ state CRITERIA:
 state CRITERION:
   '=' -> CRITERION_STR
 
+state CRITERION_FROM:
+  '=' -> CRITERION_FROM_STR_START
+
+state CRITERION_FROM_STR_START:
+  '"' -> CRITERION_FROM_STR
+  kind = 'auto', 'user'
+    -> call cfg_criteria_add($ctype, $kind); CRITERIA
+
+state CRITERION_FROM_STR:
+  kind = 'auto', 'user'
+    -> CRITERION_FROM_STR_END
+
+state CRITERION_FROM_STR_END:
+  '"'
+    -> call cfg_criteria_add($ctype, $kind); CRITERIA
+
 state CRITERION_STR:
   cvalue = word
       -> call cfg_criteria_add($ctype, $cvalue); CRITERIA

src/handlers.c

@@ -842,6 +842,7 @@ static void handle_client_message(xcb_client_message_event_t *event) {
             DLOG("The window was requested to be visible on all workspaces, making it sticky and floating.\n");
 
             floating_enable(con, false);
+            con->floating = FLOATING_AUTO_ON;
 
             con->sticky = true;
             ewmh_update_sticky(con->window->id, true);
@@ -1156,6 +1157,25 @@ static bool handle_strut_partial_change(Con *con, xcb_get_property_reply_t *prop
     return true;
 }
 
+/*
+ * Handles the _I3_FLOATING_WINDOW property to properly run assignments for
+ * floating window changes.
+ *
+ * This is needed to correctly run the assignments after changes in floating
+ * windows which are triggered by user commands (floating enable | disable). In
+ * that case, we can't call run_assignments because it will modify the parser
+ * state when it needs to parse the user-specified action, breaking the parser
+ * state for the original command.
+ *
+ */
+static bool handle_i3_floating(Con *con, xcb_get_property_reply_t *prop) {
+    DLOG("floating change for con %p\n", con);
+
+    remanage_window(con);
+
+    return true;
+}
+
 /* Returns false if the event could not be processed (e.g. the window could not
  * be found), true otherwise */
 typedef bool (*cb_property_handler_t)(Con *con, xcb_get_property_reply_t *property);
@@ -1177,6 +1197,7 @@ static struct property_handler_t property_handlers[] = {
     {0, 128, handle_class_change},
     {0, UINT_MAX, handle_strut_partial_change},
     {0, UINT_MAX, handle_window_type},
+    {0, UINT_MAX, handle_i3_floating},
     {0, 5 * sizeof(uint64_t), handle_motif_hints_change}};
 #define NUM_HANDLERS (sizeof(property_handlers) / sizeof(struct property_handler_t))
 
@@ -1198,7 +1219,8 @@ void property_handlers_init(void) {
     property_handlers[7].atom = XCB_ATOM_WM_CLASS;
     property_handlers[8].atom = A__NET_WM_STRUT_PARTIAL;
     property_handlers[9].atom = A__NET_WM_WINDOW_TYPE;
-    property_handlers[10].atom = A__MOTIF_WM_HINTS;
+    property_handlers[10].atom = A_I3_FLOATING_WINDOW;
+    property_handlers[11].atom = A__MOTIF_WM_HINTS;
 }
 
 static void property_notify(uint8_t state, xcb_window_t window, xcb_atom_t atom) {

src/manage.c

@@ -538,6 +538,7 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
         bool automatic_border = (motif_border_style == BS_NORMAL);
 
         floating_enable(nc, automatic_border);
+        nc->floating = FLOATING_AUTO_ON;
     }
 
     /* explicitly set the border width to the default */

src/match.c

@@ -215,15 +215,43 @@ bool match_matches_window(Match *match, i3Window *window) {
     }
 
     if (match->window_mode != WM_ANY) {
-        if ((con = con_by_window_id(window->id)) == NULL)
+        if ((con = con_by_window_id(window->id)) == NULL) {
             return false;
+        }
 
-        const bool floating = (con_inside_floating(con) != NULL);
-
-        if ((match->window_mode == WM_TILING && floating) ||
-            (match->window_mode == WM_FLOATING && !floating)) {
-            LOG("window_mode does not match\n");
-            return false;
+        switch (match->window_mode) {
+            case WM_TILING_AUTO:
+                if (con->floating != FLOATING_AUTO_OFF) {
+                    return false;
+                }
+                break;
+            case WM_TILING_USER:
+                if (con->floating != FLOATING_USER_OFF) {
+                    return false;
+                }
+                break;
+            case WM_TILING:
+                if (con_inside_floating(con) != NULL) {
+                    return false;
+                }
+                break;
+            case WM_FLOATING_AUTO:
+                if (con->floating != FLOATING_AUTO_ON) {
+                    return false;
+                }
+                break;
+            case WM_FLOATING_USER:
+                if (con->floating != FLOATING_USER_ON) {
+                    return false;
+                }
+                break;
+            case WM_FLOATING:
+                if (con_inside_floating(con) == NULL) {
+                    return false;
+                }
+                break;
+            case WM_ANY:
+                assert(false);
         }
 
         LOG("window_mode matches\n");
@@ -367,10 +395,38 @@ void match_parse_property(Match *match, const char *ctype, const char *cvalue) {
         return;
     }
 
+    if (strcmp(ctype, "tiling_from") == 0 &&
+        cvalue != NULL &&
+        strcmp(cvalue, "auto") == 0) {
+        match->window_mode = WM_TILING_AUTO;
+        return;
+    }
+
+    if (strcmp(ctype, "tiling_from") == 0 &&
+        cvalue != NULL &&
+        strcmp(cvalue, "user") == 0) {
+        match->window_mode = WM_TILING_USER;
+        return;
+    }
+
     if (strcmp(ctype, "floating") == 0) {
         match->window_mode = WM_FLOATING;
         return;
     }
 
+    if (strcmp(ctype, "floating_from") == 0 &&
+        cvalue != NULL &&
+        strcmp(cvalue, "auto") == 0) {
+        match->window_mode = WM_FLOATING_AUTO;
+        return;
+    }
+
+    if (strcmp(ctype, "floating_from") == 0 &&
+        cvalue != NULL &&
+        strcmp(cvalue, "user") == 0) {
+        match->window_mode = WM_FLOATING_USER;
+        return;
+    }
+
     ELOG("Unknown criterion: %s\n", ctype);
 }

testcases/t/201-config-parser.t

@@ -98,18 +98,53 @@ is(parser_calls($config),
 ################################################################################
 
 $config = <<'EOT';
+for_window [] nop empty
 for_window [class="^Chrome"] floating enable
+for_window [class=^Chrome] floating enable
+for_window [floating_from   = "auto" class= ==Class==  ] nop floating
+for_window [tiling_from=auto class="==Class=="]nop floating
 EOT
 
 $expected = <<'EOT';
+cfg_for_window(nop empty)
 cfg_criteria_add(class, ^Chrome)
 cfg_for_window(floating enable)
+cfg_criteria_add(class, ^Chrome)
+cfg_for_window(floating enable)
+cfg_criteria_add(floating_from, auto)
+cfg_criteria_add(class, ==Class==)
+cfg_for_window(nop floating)
+cfg_criteria_add(tiling_from, auto)
+cfg_criteria_add(class, ==Class==)
+cfg_for_window(nop floating)
 EOT
 
 is(parser_calls($config),
    $expected,
    'for_window okay');
 
+$config = <<'EOT';
+for_window [tiling_from=typo] nop typo
+for_window [tiling_from="typo"] nop typo
+EOT
+
+$expected = <<'EOT';
+ERROR: CONFIG: Expected one of these tokens: '"', 'auto', 'user'
+ERROR: CONFIG: (in file <stdin>)
+ERROR: CONFIG: Line   1: for_window [tiling_from=typo] nop typo
+ERROR: CONFIG:                                   ^^^^^^^^^^^^^^
+ERROR: CONFIG: Line   2: for_window [tiling_from="typo"] nop typo
+ERROR: CONFIG: Expected one of these tokens: 'auto', 'user'
+ERROR: CONFIG: (in file <stdin>)
+ERROR: CONFIG: Line   1: for_window [tiling_from=typo] nop typo
+ERROR: CONFIG: Line   2: for_window [tiling_from="typo"] nop typo
+ERROR: CONFIG:                                    ^^^^^^^^^^^^^^^
+EOT
+
+is(parser_calls($config),
+   $expected,
+   'for_window errors okay');
+
 ################################################################################
 # assign
 ################################################################################

testcases/t/271-for_window_tilingfloating.t

@@ -17,27 +17,69 @@
 use i3test i3_config => <<EOT;
 # i3 config file (v4)
 font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
-for_window [tiling] mark tiled
-for_window [floating] mark floated
+for_window [tiling] mark --add tiling
+for_window [floating] mark --add floating
+
+for_window [tiling_from="auto"] mark --add tiling_auto
+for_window [floating_from="auto"] mark --add floating_auto
+
+for_window [tiling_from="user"] mark --add tiling_user
+for_window [floating_from="user"] mark --add floating_user
 EOT
 use X11::XCB qw(PROP_MODE_REPLACE);
 
 ##############################################################
-# 13: check that the tiling / floating criteria work.
+# Check that the auto tiling / floating criteria work.
 ##############################################################
 
 my $tmp = fresh_workspace;
-
-open_window;
-open_floating_window;
+my $A = open_window;
+my $B = open_floating_window;
 
 my @nodes = @{get_ws($tmp)->{nodes}};
 cmp_ok(@nodes, '==', 1, 'one tiling container on this workspace');
-is_deeply($nodes[0]->{marks}, [ 'tiled' ], "mark set for 'tiling' criterion");
+is_deeply($nodes[0]->{marks}, [ 'tiling', 'tiling_auto' ], "mark set for 'tiling' criterion");
+
+@nodes = @{get_ws($tmp)->{floating_nodes}};
+cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
+is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'floating', 'floating_auto' ], "mark set for 'floating' criterion");
+
+################################################################################
+# Check that the user tiling / floating criteria work.
+# The following rules are triggered here: 'tiling', 'tiling_user', 'floating',
+# 'floating_user'. Therefore, the old marks 'tiling' and 'floating' are
+# replaced but the 'tiling_auto' and 'floating_auto' marks are preserved.
+################################################################################
+
+cmd '[id=' . $A->{id} . '] floating enable';
+cmd '[id=' . $B->{id} . '] floating disable';
+
+@nodes = @{get_ws($tmp)->{nodes}};
+cmp_ok(@nodes, '==', 1, 'one tiling container on this workspace');
+is_deeply($nodes[0]->{marks}, [ 'floating_auto', 'tiling', 'tiling_user' ], "Marks updated after 'floating_user' criterion");
+
+@nodes = @{get_ws($tmp)->{floating_nodes}};
+cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
+is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'tiling_auto', 'floating', 'floating_user' ], "Marks updated after 'tiling_user' criterion");
+
+################################################################################
+# Check that the default and auto rules do not re-trigger
+# Here, the windows are returned to their original state but since the rules
+# `tiling`, `tiling_auto`, `floating` and `floating_auto` where already
+# triggered, only the `tiling_user` and `floating_user` rules should trigger.
+################################################################################
+
+# Use 'mark' to clear old marks
+cmd '[id=' . $A->{id} . '] mark A, floating disable';
+cmd '[id=' . $B->{id} . '] mark B, floating enable';
+
+@nodes = @{get_ws($tmp)->{nodes}};
+cmp_ok(@nodes, '==', 1, 'one tiling container on this workspace');
+is_deeply($nodes[0]->{marks}, [ 'A', 'tiling_user' ], "Only 'tiling_user' rule triggered");
 
 @nodes = @{get_ws($tmp)->{floating_nodes}};
 cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
-is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'floated' ], "mark set for 'floating' criterion");
+is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'B', 'floating_user' ], "Only 'floating_user' rule triggered");
 
 ##############################################################