Merge pull request #1416 from Textualize/query-star-exclude-self

Queries/`walk_children` no longer includes self in results by default

Author: willmcgugan

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 - Moved Ctrl+C, tab, and shift+tab to App BINDINGS
+- Queries/`walk_children` no longer includes self in results by default https://github.com/Textualize/textual/pull/1416
 
 ## [0.7.0] - 2022-12-17
 

src/textual/app.py

@@ -1498,7 +1498,7 @@ def _on_idle(self) -> None:
             nodes: set[DOMNode] = {
                 child
                 for node in self._require_stylesheet_update
-                for child in node.walk_children()
+                for child in node.walk_children(with_self=True)
             }
             self._require_stylesheet_update.clear()
             self.stylesheet.update_nodes(nodes, animate=True)

src/textual/css/match.py

@@ -25,14 +25,14 @@ def match(selector_sets: Iterable[SelectorSet], node: DOMNode) -> bool:
 
 
 def _check_selectors(selectors: list[Selector], css_path_nodes: list[DOMNode]) -> bool:
-    """Match a list of selectors against a node.
+    """Match a list of selectors against DOM nodes.
 
     Args:
         selectors (list[Selector]): A list of selectors.
-        node (DOMNode): A DOM node.
+        css_path_nodes (list[DOMNode]): The DOM nodes to check the selectors against.
 
     Returns:
-        bool: True if the node matches the selector.
+        bool: True if any node in css_path_nodes matches a selector.
     """
 
     DESCENDENT = CombinatorType.DESCENDENT

src/textual/css/stylesheet.py

@@ -490,7 +490,7 @@ def update(self, root: DOMNode, animate: bool = False) -> None:
             animate (bool, optional): Enable CSS animation. Defaults to False.
         """
 
-        self.update_nodes(root.walk_children(), animate=animate)
+        self.update_nodes(root.walk_children(with_self=True), animate=animate)
 
     def update_nodes(self, nodes: Iterable[DOMNode], animate: bool = False) -> None:
         """Update styles for nodes.

src/textual/dom.py

@@ -615,7 +615,7 @@ def reset_styles(self) -> None:
         """Reset styles back to their initial state"""
         from .widget import Widget
 
-        for node in self.walk_children():
+        for node in self.walk_children(with_self=True):
             node._css_styles.reset()
             if isinstance(node, Widget):
                 node._set_dirty()
@@ -648,7 +648,7 @@ def walk_children(
         self,
         filter_type: type[WalkType],
         *,
-        with_self: bool = True,
+        with_self: bool = False,
         method: WalkMethod = "depth",
         reverse: bool = False,
     ) -> list[WalkType]:
@@ -658,7 +658,7 @@ def walk_children(
     def walk_children(
         self,
         *,
-        with_self: bool = True,
+        with_self: bool = False,
         method: WalkMethod = "depth",
         reverse: bool = False,
     ) -> list[DOMNode]:
@@ -668,16 +668,16 @@ def walk_children(
         self,
         filter_type: type[WalkType] | None = None,
         *,
-        with_self: bool = True,
+        with_self: bool = False,
         method: WalkMethod = "depth",
         reverse: bool = False,
     ) -> list[DOMNode] | list[WalkType]:
-        """Generate descendant nodes.
+        """Walk the subtree rooted at this node, and return every descendant encountered in a list.
 
         Args:
             filter_type (type[WalkType] | None, optional): Filter only this type, or None for no filter.
                 Defaults to None.
-            with_self (bool, optional): Also yield self in addition to descendants. Defaults to True.
+            with_self (bool, optional): Also yield self in addition to descendants. Defaults to False.
             method (Literal["breadth", "depth"], optional): One of "depth" or "breadth". Defaults to "depth".
             reverse (bool, optional): Reverse the order (bottom up). Defaults to False.
 

tests/test_query.py

@@ -1,5 +1,7 @@
 import pytest
 
+from textual.app import App, ComposeResult
+from textual.containers import Container
 from textual.widget import Widget
 from textual.css.query import InvalidQueryFormat, WrongType, NoMatches, TooManyMatches
 
@@ -50,17 +52,16 @@ class App(Widget):
         assert list(app.query(".frob")) == []
         assert list(app.query("#frob")) == []
 
-        assert app.query("App")
         assert not app.query("NotAnApp")
 
-        assert list(app.query("App")) == [app]
+        assert list(app.query("App")) == []  # Root is not included in queries
         assert list(app.query("#main")) == [main_view]
         assert list(app.query("View#main")) == [main_view]
         assert list(app.query("View2#help")) == [help_view]
         assert list(app.query("#widget1")) == [widget1]
-        assert list(app.query("#Widget1")) == [] # Note case.
+        assert list(app.query("#Widget1")) == []  # Note case.
         assert list(app.query("#widget2")) == [widget2]
-        assert list(app.query("#Widget2")) == [] # Note case.
+        assert list(app.query("#Widget2")) == []  # Note case.
 
         assert list(app.query("Widget.float")) == [sidebar, tooltip, helpbar]
         assert list(app.query(Widget).filter(".float")) == [sidebar, tooltip, helpbar]
@@ -110,10 +111,10 @@ class App(Widget):
             tooltip,
         ]
 
-        assert list(app.query("App,View")) == [app, main_view, sub_view, help_view]
+        assert list(app.query("View")) == [main_view, sub_view, help_view]
         assert list(app.query("#widget1, #widget2")) == [widget1, widget2]
         assert list(app.query("#widget1 , #widget2")) == [widget1, widget2]
-        assert list(app.query("#widget1, #widget2, App")) == [app, widget1, widget2]
+        assert list(app.query("#widget1, #widget2, App")) == [widget1, widget2]
 
         assert app.query(".float").first() == sidebar
         assert app.query(".float").last() == helpbar
@@ -130,14 +131,13 @@ class App(Widget):
 
 
 def test_query_classes():
-
     class App(Widget):
         pass
 
     class ClassTest(Widget):
         pass
 
-    CHILD_COUNT=100
+    CHILD_COUNT = 100
 
     # Create a fake app to hold everything else.
     app = App()
@@ -147,34 +147,35 @@ class ClassTest(Widget):
         app._add_child(ClassTest(id=f"child{n}"))
 
     # Let's just be 100% sure everything was created fine.
-    assert len(app.query(ClassTest))==CHILD_COUNT
+    assert len(app.query(ClassTest)) == CHILD_COUNT
 
     # Now, let's check there are *no* children with the test class.
-    assert len(app.query(".test"))==0
+    assert len(app.query(".test")) == 0
 
     # Add the test class to everything and then check again.
     app.query(ClassTest).add_class("test")
-    assert len(app.query(".test"))==CHILD_COUNT
+    assert len(app.query(".test")) == CHILD_COUNT
 
     # Remove the test class from everything then try again.
     app.query(ClassTest).remove_class("test")
-    assert len(app.query(".test"))==0
+    assert len(app.query(".test")) == 0
 
     # Add the test class to everything using set_class.
     app.query(ClassTest).set_class(True, "test")
-    assert len(app.query(".test"))==CHILD_COUNT
+    assert len(app.query(".test")) == CHILD_COUNT
 
     # Remove the test class from everything using set_class.
     app.query(ClassTest).set_class(False, "test")
-    assert len(app.query(".test"))==0
+    assert len(app.query(".test")) == 0
 
     # Add the test class to everything using toggle_class.
     app.query(ClassTest).toggle_class("test")
-    assert len(app.query(".test"))==CHILD_COUNT
+    assert len(app.query(".test")) == CHILD_COUNT
 
     # Remove the test class from everything using toggle_class.
     app.query(ClassTest).toggle_class("test")
-    assert len(app.query(".test"))==0
+    assert len(app.query(".test")) == 0
+
 
 def test_invalid_query():
     class App(Widget):
@@ -187,3 +188,26 @@ class App(Widget):
 
     with pytest.raises(InvalidQueryFormat):
         app.query("#foo").exclude("#2")
+
+
+async def test_universal_selector_doesnt_select_self():
+    class ExampleApp(App):
+        def compose(self) -> ComposeResult:
+            yield Container(
+                Widget(
+                    Widget(),
+                    Widget(
+                        Widget(),
+                    ),
+                ),
+                Widget(),
+                id="root-container",
+            )
+
+    app = ExampleApp()
+    async with app.run_test():
+        container = app.query_one("#root-container", Container)
+        query = container.query("*")
+        results = list(query.results())
+        assert len(results) == 5
+        assert not any(node.id == "root-container" for node in results)