Extend tiling/floating criteria with optional auto/user values

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

7 files changed, 146 insertions, 17 deletions

diff --git a/docs/userguide b/docs/userguide
index 4cda35f8..cb339d39 100644
--- a/docs/userguide
+++ b/docs/userguide
@@ -1900,8 +1900,20 @@ con_id::
 	to match only the currently focused window.
 floating::
 	Only matches floating windows. This criterion requires no value.
+floating_auto::
+	Same as +floating+, but only matches windows that were automatically
+	opened as floating. This criterion requires no value.
+floating_user::
+	Same as +floating+, but only matches windows that the user made
+	floating. This criterion requires no value.
 tiling::
 	Only matches tiling windows. This criterion requires no value.
+tiling_auto::
+	Same as +tiling+, but only matches windows that were automatically
+	opened as tiling. This criterion requires no value.
+tiling_user::
+	Same as +tiling+, but only matches windows that the user made
+	tiling. This criterion requires no value.
 
 The criteria +class+, +instance+, +role+, +title+, +workspace+ and +mark+ are
 actually regular expressions (PCRE). See +pcresyntax(3)+ or +perldoc perlre+ for
diff --git a/include/data.h b/include/data.h
index c0e34b41..e686d2c7 100644
--- a/include/data.h
+++ b/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;
 
diff --git a/parser-specs/config.spec b/parser-specs/config.spec
index 93901fd9..d2421496 100644
--- a/parser-specs/config.spec
+++ b/parser-specs/config.spec
@@ -191,7 +191,10 @@ state CRITERIA:
   ctype = 'title'       -> CRITERION
   ctype = 'urgent'      -> CRITERION
   ctype = 'workspace'   -> CRITERION
-  ctype = 'tiling', 'floating'
+  # This is a hack here because the parser does not support optional arguments,
+  # in order to be able to handle for example both 'tiling' and 'tiling_auto',
+  # see #3588.
+  ctype = 'tiling_auto', 'tiling_user', 'floating_auto', 'floating_user', 'tiling', 'floating'
       -> call cfg_criteria_add($ctype, NULL); CRITERIA
   ']'
       -> call cfg_criteria_pop_state()
diff --git a/src/handlers.c b/src/handlers.c
index 22a974ac..ed3f6c3e 100644
--- a/src/handlers.c
+++ b/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) {
diff --git a/src/manage.c b/src/manage.c
index 726c714c..eecba95c 100644
--- a/src/manage.c
+++ b/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 */
diff --git a/src/match.c b/src/match.c
index a8850af4..a4ef05a3 100644
--- a/src/match.c
+++ b/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,6 +395,23 @@ void match_parse_property(Match *match, const char *ctype, const char *cvalue) {
         return;
     }
 
+    if (strcmp(ctype, "tiling_auto") == 0) {
+        match->window_mode = WM_TILING_AUTO;
+        return;
+    }
+    if (strcmp(ctype, "tiling_user") == 0) {
+        match->window_mode = WM_TILING_USER;
+        return;
+    }
+    if (strcmp(ctype, "floating_auto") == 0) {
+        match->window_mode = WM_FLOATING_AUTO;
+        return;
+    }
+    if (strcmp(ctype, "floating_user") == 0) {
+        match->window_mode = WM_FLOATING_USER;
+        return;
+    }
+
     if (strcmp(ctype, "floating") == 0) {
         match->window_mode = WM_FLOATING;
         return;
diff --git a/testcases/t/271-for_window_tilingfloating.t b/testcases/t/271-for_window_tilingfloating.t
index caee328d..544be162 100644
--- a/testcases/t/271-for_window_tilingfloating.t
+++ b/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_auto] mark --add tiling_auto
+for_window [floating_auto] mark --add floating_auto
+
+for_window [tiling_user] mark --add tiling_user
+for_window [floating_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");
 
 ##############################################################