address feedback

sway/commands/kill.c

@@ -10,19 +10,7 @@ struct cmd_results *cmd_kill(int argc, char **argv) {
 	struct sway_container *con =
 		config->handler_context.current_container;
 
-	switch (con->type) {
+	container_close(con);
-	case C_ROOT:
-	case C_OUTPUT:
-	case C_WORKSPACE:
-	case C_TYPES:
-		return cmd_results_new(CMD_INVALID, NULL,
-				"Can only kill views and containers with this command");
-		break;
-	case C_CONTAINER:
-	case C_VIEW:
-		container_close(con);
-		break;
-	}
 
 	return cmd_results_new(CMD_SUCCESS, NULL, NULL);
 }

sway/desktop/output.c

@@ -25,7 +25,7 @@
 struct sway_container *output_by_name(const char *name) {
 	for (int i = 0; i < root_container.children->length; ++i) {
 		struct sway_container *output = root_container.children->items[i];
-		if (strcasecmp(output->name, name) == 0){
+		if (strcasecmp(output->name, name) == 0) {
 			return output;
 		}
 	}

sway/tree/container.c

@@ -186,45 +186,44 @@ static struct sway_container *container_workspace_destroy(
 	return parent;
 }
 
+static void container_root_finish(struct sway_container *con) {
+	wlr_log(L_ERROR, "TODO: destroy the root container");
+}
 
-static void reap_empty_func(struct sway_container *con, void *data) {
+static bool container_reap_empty(struct sway_container *con) {
 	switch (con->type) {
-		case C_TYPES:
+	case C_ROOT:
-		case C_ROOT:
+	case C_OUTPUT:
-		case C_OUTPUT:
+		// dont reap these
-			// dont reap these
+		break;
-			break;
+	case C_WORKSPACE:
-		case C_WORKSPACE:
+		if (!workspace_is_visible(con) && con->children->length == 0) {
-			if (!workspace_is_visible(con) && con->children->length == 0) {
+			container_workspace_destroy(con);
-				container_workspace_destroy(con);
+			return true;
-			}
+		}
-			break;
+		break;
-		case C_CONTAINER:
+	case C_CONTAINER:
-			if (con->children->length == 0) {
+		if (con->children->length == 0) {
+			container_finish(con);
+			return true;
+		} else if (con->children->length == 1) {
+			struct sway_container *child = con->children->items[0];
+			if (child->type == C_CONTAINER) {
+				container_remove_child(child);
+				container_replace_child(con, child);
 				container_finish(con);
-			} else if (con->children->length == 1) {
+				return true;
-				struct sway_container *only_child = con->children->items[0];
-				if (only_child->type == C_CONTAINER) {
-					container_remove_child(only_child);
-					container_replace_child(con, only_child);
-					container_finish(con);
-				}
 			}
-		case C_VIEW:
+		}
-			break;
+	case C_VIEW:
+		break;
+	case C_TYPES:
+		sway_assert(false, "container_reap_empty called on an invalid "
+			"container");
+		break;
 	}
-}
-
-struct sway_container *container_reap_empty(struct sway_container *container) {
-	struct sway_container *parent = container->parent;
-
-	container_for_each_descendant_dfs(container, reap_empty_func, NULL);
 
-	return parent;
+	return false;
-}
-
-static void container_root_finish(struct sway_container *con) {
-	wlr_log(L_ERROR, "TODO: destroy the root container");
 }
 
 struct sway_container *container_destroy(struct sway_container *con) {
@@ -232,38 +231,52 @@ struct sway_container *container_destroy(struct sway_container *con) {
 		return NULL;
 	}
 
-	struct sway_container *anscestor = NULL;
+	struct sway_container *parent = con->parent;
 
 	switch (con->type) {
 		case C_ROOT:
 			container_root_finish(con);
 			break;
 		case C_OUTPUT:
-			anscestor = container_output_destroy(con);
+			// dont try to reap the root after this
+			container_output_destroy(con);
 			break;
 		case C_WORKSPACE:
-			anscestor = container_workspace_destroy(con);
+			// dont try to reap the output after this
+			container_workspace_destroy(con);
 			break;
 		case C_CONTAINER:
-			if (con->children != NULL && con->children->length) {
+			if (con->children->length) {
-				assert(false &&
+				for (int i = 0; i < con->children->length; ++i) {
-					"dont destroy container containers with children");
+					struct sway_container *child = con->children->items[0];
+					container_remove_child(child);
+					container_add_child(parent, child);
+				}
+				container_finish(con);
 			}
 			container_finish(con);
-			// TODO return parent to arrange maybe?
 			break;
 		case C_VIEW:
 			container_finish(con);
-			// TODO return parent to arrange maybe?
 			break;
 		case C_TYPES:
-			wlr_log(L_ERROR, "tried to destroy an invalid container");
+			wlr_log(L_ERROR, "container_destroy called on an invalid "
+				"container");
 			break;
 	}
 
-	container_reap_empty(&root_container);
+	struct sway_container *tmp = parent;
+	while (parent) {
+		tmp = parent->parent;
+
+		if (!container_reap_empty(parent)) {
+			break;
+		}
+
+		parent = tmp;
+	}
 
-	return anscestor;
+	return tmp;
 }
 
 static void container_close_func(struct sway_container *container, void *data) {
@@ -274,23 +287,23 @@ static void container_close_func(struct sway_container *container, void *data) {
 
 struct sway_container *container_close(struct sway_container *con) {
 	if (!sway_assert(con != NULL,
-				"container_close called with a NULL container")) {
+			"container_close called with a NULL container")) {
 		return NULL;
 	}
 
 	struct sway_container *parent = con->parent;
 
 	switch (con->type) {
-		case C_TYPES:
+	case C_TYPES:
-		case C_ROOT:
+	case C_ROOT:
-		case C_OUTPUT:
+	case C_OUTPUT:
-		case C_WORKSPACE:
+	case C_WORKSPACE:
-		case C_CONTAINER:
+	case C_CONTAINER:
-			container_for_each_descendant_dfs(con, container_close_func, NULL);
+		container_for_each_descendant_dfs(con, container_close_func, NULL);
-			break;
+		break;
-		case C_VIEW:
+	case C_VIEW:
-			view_close(con->sway_view);
+		view_close(con->sway_view);
-			break;
+		break;
 
 	}
 
@@ -365,8 +378,8 @@ struct sway_container *container_output_create(
 static struct sway_container *get_workspace_initial_output(const char *name) {
 	struct sway_container *parent;
 	// Search for workspace<->output pair
-	int i, e = config->workspace_outputs->length;
+	int e = config->workspace_outputs->length;
-	for (i = 0; i < e; ++i) {
+	for (int i = 0; i < config->workspace_outputs->length; ++i) {
 		struct workspace_output *wso = config->workspace_outputs->items[i];
 		if (strcasecmp(wso->workspace, name) == 0) {
 			// Find output to use if it exists

sway/tree/layout.c

@@ -174,7 +174,7 @@ enum sway_container_layout container_get_default_layout(
 	}
 
 	if (!sway_assert(con != NULL,
-			"container_get_default_layout must be called on an attached "
+			"container_get_default_layout must be called on an attached"
 			" container below the root container")) {
 		return 0;
 	}

sway/tree/view.c

@@ -149,12 +149,12 @@ void view_unmap(struct sway_view *view) {
 
 	view_damage_whole(view);
 
-	container_destroy(view->swayc);
+	struct sway_container *parent = container_destroy(view->swayc);
 
 	view->swayc = NULL;
 	view->surface = NULL;
 
-	arrange_windows(&root_container, -1, -1);
+	arrange_windows(parent, -1, -1);
 }
 
 void view_update_position(struct sway_view *view, double ox, double oy) {