Validate matched containers for "kill" command correctly.

We now execute the validations when "kill" is executed even if match
criteria are used. This prevents users from killing workspace containers,
which instead kills all clients (as before when not using criteria).

fixes #1761

diff --git a/include/con.h b/include/con.h
index 655fde7f7f62242c5d29412a1d9ccde1e3f836e8..db5fcfbf4fadf965f73bb7e2c067821559c58cdb 100644 (file)
--- a/include/con.h
+++ b/include/con.h
@@ -30,6 +30,12 @@ Con *con_new(Con *parent, i3Window *window);
  */
 void con_focus(Con *con);
 
+/**
+ * Closes the given container.
+ *
+ */
+void con_close(Con *con, kill_window_t kill_window);
+
 /**
  * Returns true when this node is a leaf node (has no children)
  *

diff --git a/include/tree.h b/include/tree.h
index 4a6404463deac4313592f2953eacbc6155eaae4e..c64c173d40c2584f4d2507fc95168ae38dd4ca60 100644 (file)
--- a/include/tree.h
+++ b/include/tree.h
@@ -56,12 +56,6 @@ bool level_down(void);
  */
 void tree_render(void);
 
-/**
- * Closes the current container using tree_close_internal().
- *
- */
-void tree_close_con(kill_window_t kill_window);
-
 /**
  * Changes focus in the given way (next/previous) and given orientation
  * (horizontal/vertical).

diff --git a/src/commands.c b/src/commands.c
index eb1834e769188687205aafe662ffb925c62a2fc1..579b5e0eb5072c987c79c5e1b6930ad0ae5755fc 100644 (file)
--- a/src/commands.c
+++ b/src/commands.c
@@ -1258,7 +1258,6 @@ void cmd_split(I3_CMD, const char *direction) {
 void cmd_kill(I3_CMD, const char *kill_mode_str) {
     if (kill_mode_str == NULL)
         kill_mode_str = "window";
-    owindow *current;
 
     DLOG("kill_mode=%s\n", kill_mode_str);
 
@@ -1273,16 +1272,11 @@ void cmd_kill(I3_CMD, const char *kill_mode_str) {
         return;
     }
 
-    HANDLE_INVALID_MATCH;
+    HANDLE_EMPTY_MATCH;
 
-    /* check if the match is empty, not if the result is empty */
-    if (match_is_empty(current_match))
-        tree_close_con(kill_mode);
-    else {
-        TAILQ_FOREACH(current, &owindows, owindows) {
-            DLOG("matching: %p / %s\n", current->con, current->con->name);
-            tree_close_internal(current->con, kill_mode, false, false);
-        }
+    owindow *current;
+    TAILQ_FOREACH(current, &owindows, owindows) {
+        con_close(current->con, kill_mode);
     }
 
     cmd_output->needs_tree_render = true;

diff --git a/src/con.c b/src/con.c
index f0353f972393bf6c3eb8a0f05ba06640d37c6172..591419e482b428a30901ea0d8c1c49253d1a1bac 100644 (file)
--- a/src/con.c
+++ b/src/con.c
@@ -220,6 +220,36 @@ void con_focus(Con *con) {
     }
 }
 
+/*
+ * Closes the given container.
+ *
+ */
+void con_close(Con *con, kill_window_t kill_window) {
+    assert(con != NULL);
+    DLOG("Closing con = %p.\n", con);
+
+    /* We never close output or root containers. */
+    if (con->type == CT_OUTPUT || con->type == CT_ROOT) {
+        DLOG("con = %p is of type %d, not closing anything.\n", con, con->type);
+        return;
+    }
+
+    if (con->type == CT_WORKSPACE) {
+        DLOG("con = %p is a workspace, closing all children instead.\n", con);
+        Con *child, *nextchild;
+        for (child = TAILQ_FIRST(&(con->focus_head)); child;) {
+            nextchild = TAILQ_NEXT(child, focused);
+            DLOG("killing child = %p.\n", child);
+            tree_close_internal(child, kill_window, false, false);
+            child = nextchild;
+        }
+
+        return;
+    }
+
+    tree_close_internal(con, kill_window, false, false);
+}
+
 /*
  * Returns true when this node is a leaf node (has no children)
  *

diff --git a/src/tree.c b/src/tree.c
index e296a9819bdae297e0f86a259b62680c060232a9..5642c5c78f360148612af684bc1861e236fc9ed2 100644 (file)
--- a/src/tree.c
+++ b/src/tree.c
@@ -362,34 +362,6 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par
     return true;
 }
 
-/*
- * Closes the current container using tree_close_internal().
- *
- */
-void tree_close_con(kill_window_t kill_window) {
-    assert(focused != NULL);
-
-    /* There *should* be no possibility to focus outputs / root container */
-    assert(focused->type != CT_OUTPUT);
-    assert(focused->type != CT_ROOT);
-
-    if (focused->type == CT_WORKSPACE) {
-        DLOG("Workspaces cannot be close, closing all children instead\n");
-        Con *child, *nextchild;
-        for (child = TAILQ_FIRST(&(focused->focus_head)); child;) {
-            nextchild = TAILQ_NEXT(child, focused);
-            DLOG("killing child=%p\n", child);
-            tree_close_internal(child, kill_window, false, false);
-            child = nextchild;
-        }
-
-        return;
-    }
-
-    /* Kill con */
-    tree_close_internal(focused, kill_window, false, false);
-}
-
 /*
  * Splits (horizontally or vertically) the given container by creating a new
  * container which contains the old one and the future ones.

diff --git a/testcases/t/261-match-con_id-con_mark-combinations.t b/testcases/t/261-match-con_id-con_mark-combinations.t
index b255558e3aa164f68befedef9401954f192a351f..61ff1ff0365b700816c1bc1bcf9b35c1299bf066 100644 (file)
--- a/testcases/t/261-match-con_id-con_mark-combinations.t
+++ b/testcases/t/261-match-con_id-con_mark-combinations.t
@@ -27,9 +27,11 @@ $ws = fresh_workspace;
 open_window(wm_class => 'matchme');
 
 cmd '[con_id=__focused__ class=doesnotmatch] kill';
+sync_with_i3;
 is(@{get_ws($ws)->{nodes}}, 1, 'window was not killed');
 
 cmd '[con_id=__focused__ class=matchme] kill';
+sync_with_i3;
 is(@{get_ws($ws)->{nodes}}, 0, 'window was killed');
 
 ###############################################################################
@@ -41,9 +43,11 @@ open_window(wm_class => 'matchme');
 cmd 'mark marked';
 
 cmd '[con_mark=marked class=doesnotmatch] kill';
+sync_with_i3;
 is(@{get_ws($ws)->{nodes}}, 1, 'window was not killed');
 
 cmd '[con_mark=marked class=matchme] kill';
+sync_with_i3;
 is(@{get_ws($ws)->{nodes}}, 0, 'window was killed');
 
 ###############################################################################