From ae757c6848b49d7a0423973a39791ba6eca29308 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Sun, 12 Apr 2020 13:49:08 +0200 Subject: [PATCH] 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 --- docs/userguide | 10 +++ generate-command-parser.pl | 2 + include/data.h | 4 ++ parser-specs/config.spec | 38 ++++++++--- src/handlers.c | 24 ++++++- src/manage.c | 1 + src/match.c | 70 ++++++++++++++++++--- testcases/t/201-config-parser.t | 35 +++++++++++ testcases/t/271-for_window_tilingfloating.t | 58 ++++++++++++++--- 9 files changed, 216 insertions(+), 26 deletions(-) diff --git a/docs/userguide b/docs/userguide index 4cda35f8d..a886d73be 100644 --- a/docs/userguide +++ b/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 diff --git a/generate-command-parser.pl b/generate-command-parser.pl index 052e4c663..a017edab2 100755 --- a/generate-command-parser.pl +++ b/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]+)$/); diff --git a/include/data.h b/include/data.h index c0e34b413..e686d2c72 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 93901fd99..bb9e226e0 100644 --- a/parser-specs/config.spec +++ b/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 diff --git a/src/handlers.c b/src/handlers.c index 22a974ac8..ed3f6c3ed 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 726c714cc..eecba95c5 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 a8850af4e..6ac312e5d 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,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); } diff --git a/testcases/t/201-config-parser.t b/testcases/t/201-config-parser.t index a8d45325b..e2d885bad 100644 --- a/testcases/t/201-config-parser.t +++ b/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 ) +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 ) +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 ################################################################################ diff --git a/testcases/t/271-for_window_tilingfloating.t b/testcases/t/271-for_window_tilingfloating.t index caee328df..f3947d6e8 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 => <{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"); ##############################################################