Add get_child_by_id and get_widget_by_id

CHANGELOG.md

@@ -7,16 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [0.5.0] - Unreleased
 
-
 ### Added
 
+- Add get_child_by_id and get_widget_by_id, remove get_child https://github.com/Textualize/textual/pull/1146
 - Add easing parameter to Widget.scroll_* methods https://github.com/Textualize/textual/pull/1144
 
 ### Changed
 
 - Watchers are now called immediately when setting the attribute if they are synchronous. https://github.com/Textualize/textual/pull/1145
 
-
 ## [0.4.0] - 2022-11-08
 
 https://textual.textualize.io/blog/2022/11/08/version-040/#version-040

src/textual/_node_list.py

@@ -8,6 +8,10 @@
     from .widget import Widget
 
 
+class DuplicateIds(Exception):
+    pass
+
+
 @rich.repr.auto(angular=True)
 class NodeList(Sequence):
     """
@@ -21,6 +25,12 @@ def __init__(self) -> None:
         # The nodes in the list
         self._nodes: list[Widget] = []
         self._nodes_set: set[Widget] = set()
+
+        # We cache widgets by their IDs too for a quick lookup
+        # Note that only widgets with IDs are cached like this, so
+        # this cache will likely hold fewer values than self._nodes.
+        self._nodes_by_id: dict[str, Widget] = {}
+
         # Increments when list is updated (used for caching)
         self._updates = 0
 
@@ -53,6 +63,9 @@ def index(self, widget: Widget) -> int:
         """
         return self._nodes.index(widget)
 
+    def _get_by_id(self, widget_id: str) -> Widget | None:
+        return self._nodes_by_id.get(widget_id)
+
     def _append(self, widget: Widget) -> None:
         """Append a Widget.
 
@@ -62,6 +75,10 @@ def _append(self, widget: Widget) -> None:
         if widget not in self._nodes_set:
             self._nodes.append(widget)
             self._nodes_set.add(widget)
+            widget_id = widget.id
+            if widget_id is not None:
+                self._ensure_unique_id(widget_id)
+                self._nodes_by_id[widget_id] = widget
             self._updates += 1
 
     def _insert(self, index: int, widget: Widget) -> None:
@@ -73,8 +90,19 @@ def _insert(self, index: int, widget: Widget) -> None:
         if widget not in self._nodes_set:
             self._nodes.insert(index, widget)
             self._nodes_set.add(widget)
+            widget_id = widget.id
+            if widget_id is not None:
+                self._ensure_unique_id(widget_id)
+                self._nodes_by_id[widget_id] = widget
             self._updates += 1
 
+    def _ensure_unique_id(self, widget_id: str) -> None:
+        if widget_id in self._nodes_by_id:
+            raise DuplicateIds(
+                f"Tried to insert a widget with ID {widget_id!r}, but a widget {self._nodes_by_id[widget_id]!r} "
+                f"already exists with that ID. Widget IDs must be unique."
+            )
+
     def _remove(self, widget: Widget) -> None:
         """Remove a widget from the list.
 
@@ -86,13 +114,17 @@ def _remove(self, widget: Widget) -> None:
         if widget in self._nodes_set:
             del self._nodes[self._nodes.index(widget)]
             self._nodes_set.remove(widget)
+            widget_id = widget.id
+            if widget_id in self._nodes_by_id:
+                del self._nodes_by_id[widget_id]
             self._updates += 1
 
     def _clear(self) -> None:
         """Clear the node list."""
         if self._nodes:
             self._nodes.clear()
             self._nodes_set.clear()
+            self._nodes_by_id.clear()
             self._updates += 1
 
     def __iter__(self) -> Iterator[Widget]:

src/textual/app.py

@@ -60,7 +60,7 @@
 from .reactive import Reactive
 from .renderables.blank import Blank
 from .screen import Screen
-from .widget import AwaitMount, Widget
+from .widget import AwaitMount, Widget, MountError
 
 if TYPE_CHECKING:
     from .devtools.client import DevtoolsClient
@@ -892,7 +892,7 @@ async def _on_css_change(self) -> None:
     def render(self) -> RenderableType:
         return Blank(self.styles.background)
 
-    def get_child(self, id: str) -> DOMNode:
+    def get_child_by_id(self, id: str) -> Widget:
         """Shorthand for self.screen.get_child(id: str)
         Returns the first child (immediate descendent) of this DOMNode
         with the given ID.
@@ -906,7 +906,26 @@ def get_child(self, id: str) -> DOMNode:
         Raises:
             NoMatches: if no children could be found for this ID
         """
-        return self.screen.get_child(id)
+        return self.screen.get_child_by_id(id)
+
+    def get_widget_by_id(self, id: str) -> Widget:
+        """Shorthand for self.screen.get_widget_by_id(id)
+        Return the first descendant widget with the given ID.
+
+        Performs a breadth-first search rooted at the current screen.
+        It will not return the Screen if that matches the ID.
+        To get the screen, use `self.screen`.
+
+        Args:
+            id (str): The ID to search for in the subtree
+
+        Returns:
+            DOMNode: The first descendant encountered with this ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        return self.screen.get_widget_by_id(id)
 
     def update_styles(self, node: DOMNode | None = None) -> None:
         """Request update of styles.

src/textual/dom.py

@@ -674,7 +674,7 @@ def walk_depth_first() -> Iterable[DOMNode]:
                         push(iter(node.children))
 
         def walk_breadth_first() -> Iterable[DOMNode]:
-            """Walk the tree breadth first (children first)."""
+            """Walk the tree breadth first (children first) (level order traversal)"""
             queue: deque[DOMNode] = deque()
             popleft = queue.popleft
             extend = queue.extend
@@ -700,23 +700,6 @@ def walk_breadth_first() -> Iterable[DOMNode]:
             nodes.reverse()
         return nodes
 
-    def get_child(self, id: str) -> DOMNode:
-        """Return the first child (immediate descendent) of this node with the given ID.
-
-        Args:
-            id (str): The ID of the child.
-
-        Returns:
-            DOMNode: The first child of this node with the ID.
-
-        Raises:
-            NoMatches: if no children could be found for this ID
-        """
-        for child in self.children:
-            if child.id == id:
-                return child
-        raise NoMatches(f"No child found with id={id!r}")
-
     ExpectType = TypeVar("ExpectType", bound="Widget")
 
     @overload

src/textual/widget.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from asyncio import Lock, wait, create_task
+from collections import Counter
 from fractions import Fraction
 from itertools import islice
 from operator import attrgetter
@@ -41,6 +42,7 @@
 from ._types import Lines
 from .binding import NoBinding
 from .box_model import BoxModel, get_box_model
+from .css.query import NoMatches
 from .css.scalar import ScalarOffset
 from .dom import DOMNode, NoScreen
 from .geometry import Offset, Region, Size, Spacing, clamp
@@ -333,6 +335,44 @@ def offset(self) -> Offset:
     def offset(self, offset: Offset) -> None:
         self.styles.offset = ScalarOffset.from_offset(offset)
 
+    def get_child_by_id(self, id: str) -> Widget:
+        """Return the first child (immediate descendent) of this node with the given ID.
+
+        Args:
+            id (str): The ID of the child.
+
+        Returns:
+            DOMNode: The first child of this node with the ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        child = self.children._get_by_id(id)
+        if child is not None:
+            return child
+        raise NoMatches(f"No child found with id={id!r}")
+
+    def get_widget_by_id(self, id: str) -> Widget:
+        """Return the first descendant widget with the given ID.
+        Performs a breadth-first search rooted at this node, but this node won't match
+        if it has a matching id.
+
+        Args:
+            id (str): The ID to search for in the subtree
+
+        Returns:
+            DOMNode: The first descendant encountered with this ID.
+
+        Raises:
+            NoMatches: if no children could be found for this ID
+        """
+        for child in self.walk_children(method="depth"):
+            try:
+                return child.get_child_by_id(id)
+            except NoMatches:
+                pass
+        raise NoMatches(f"No descendant found with id={id!r}")
+
     def get_component_rich_style(self, name: str) -> Style:
         """Get a *Rich* style for a component.
 
@@ -460,6 +500,20 @@ def mount(
             provided a ``MountError`` will be raised.
         """
 
+        # Check for duplicate IDs in the incoming widgets
+        ids_to_mount = [widget.id for widget in widgets if widget.id is not None]
+        unique_ids = set(ids_to_mount)
+        num_unique_ids = len(unique_ids)
+        num_widgets_with_ids = len(ids_to_mount)
+        if num_unique_ids != num_widgets_with_ids:
+            counter = Counter(widget.id for widget in widgets)
+            for widget_id, count in counter.items():
+                if count > 1:
+                    raise MountError(
+                        f"Tried to insert {count!r} widgets with the same ID {widget_id!r}. "
+                        f"Widget IDs must be unique."
+                    )
+
         # Saying you want to mount before *and* after something is an error.
         if before is not None and after is not None:
             raise MountError(

tests/test_dom.py

@@ -1,7 +1,6 @@
 import pytest
 
 from textual.css.errors import StyleValueError
-from textual.css.query import NoMatches
 from textual.dom import DOMNode, BadIdentifier
 
 
@@ -26,37 +25,6 @@ def test_display_set_invalid_value():
         node.display = "blah"
 
 
-@pytest.fixture
-def parent():
-    parent = DOMNode(id="parent")
-    child1 = DOMNode(id="child1")
-    child2 = DOMNode(id="child2")
-    grandchild1 = DOMNode(id="grandchild1")
-    child1._add_child(grandchild1)
-
-    parent._add_child(child1)
-    parent._add_child(child2)
-
-    yield parent
-
-
-def test_get_child_gets_first_child(parent):
-    child = parent.get_child(id="child1")
-    assert child.id == "child1"
-    assert child.get_child(id="grandchild1").id == "grandchild1"
-    assert parent.get_child(id="child2").id == "child2"
-
-
-def test_get_child_no_matching_child(parent):
-    with pytest.raises(NoMatches):
-        parent.get_child(id="doesnt-exist")
-
-
-def test_get_child_only_immediate_descendents(parent):
-    with pytest.raises(NoMatches):
-        parent.get_child(id="grandchild1")
-
-
 def test_validate():
     with pytest.raises(BadIdentifier):
         DOMNode(id="23")

tests/test_unmount.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from textual.app import App, ComposeResult
 from textual import events
 from textual.containers import Container