From 4058e598e8304844e1b2edff0b5b9c56da0d9000 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 27 Nov 2023 17:09:50 +0000 Subject: [PATCH] error if no children allowed (#3758) * error if no children allowed * changelog * changelog * remove comment * quote RHS * annotations * attempt to fix 3.7 * restore experiment --- CHANGELOG.md | 1 + src/textual/_compose.py | 18 ++++++++++++++++-- src/textual/app.py | 1 + src/textual/scroll_view.py | 2 ++ src/textual/widget.py | 22 ++++++++++++++++++++++ src/textual/widgets/_button.py | 2 ++ src/textual/widgets/_static.py | 2 ++ src/textual/widgets/_toggle_button.py | 2 ++ tests/test_widget.py | 20 ++++++++++++++++++-- 9 files changed, 66 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eef73a86f..7f34d7dc59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added experimental Canvas class https://github.com/Textualize/textual/pull/3669/ - Added `keyline` rule https://github.com/Textualize/textual/pull/3669/ +- Widgets can now have an ALLOW_CHILDREN (bool) classvar to disallow adding children to a widget https://github.com/Textualize/textual/pull/3758 ### Changed diff --git a/src/textual/_compose.py b/src/textual/_compose.py index 022664d2b0..d27d09eb73 100644 --- a/src/textual/_compose.py +++ b/src/textual/_compose.py @@ -16,19 +16,33 @@ def compose(node: App | Widget) -> list[Widget]: Returns: A list of widgets. """ + _rich_traceback_omit = True app = node.app nodes: list[Widget] = [] compose_stack: list[Widget] = [] composed: list[Widget] = [] app._compose_stacks.append(compose_stack) app._composed.append(composed) + iter_compose = iter(node.compose()) try: - for child in node.compose(): + while True: + try: + child = next(iter_compose) + except StopIteration: + break if composed: nodes.extend(composed) composed.clear() if compose_stack: - compose_stack[-1].compose_add_child(child) + try: + compose_stack[-1].compose_add_child(child) + except Exception as error: + if hasattr(iter_compose, "throw"): + # So the error is raised inside the generator + # This will generate a more sensible traceback for the dev + iter_compose.throw(error) # type: ignore + else: + raise else: nodes.append(child) if composed: diff --git a/src/textual/app.py b/src/textual/app.py index 0e6111ad9b..f384aaaf3b 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2305,6 +2305,7 @@ async def take_screenshot() -> None: ) async def _on_compose(self) -> None: + _rich_traceback_omit = True try: widgets = [*self.screen._nodes, *compose(self)] except TypeError as error: diff --git a/src/textual/scroll_view.py b/src/textual/scroll_view.py index a4e3aa03d8..f62b8345d1 100644 --- a/src/textual/scroll_view.py +++ b/src/textual/scroll_view.py @@ -18,6 +18,8 @@ class ScrollView(ScrollableContainer): on the compositor to render children). """ + ALLOW_CHILDREN = False + DEFAULT_CSS = """ ScrollView { overflow-y: auto; diff --git a/src/textual/widget.py b/src/textual/widget.py index 31b3b6bf9b..f20297e6c1 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -87,6 +87,10 @@ } +class NotAContainer(Exception): + """Exception raised if you attempt to add a child to a widget which doesn't permit child nodes.""" + + _NULL_STYLE = Style() @@ -264,6 +268,9 @@ class Widget(DOMNode): BORDER_SUBTITLE: ClassVar[str] = "" """Initial value for border_subtitle attribute.""" + ALLOW_CHILDREN: ClassVar[bool] = True + """Set to `False` to prevent adding children to this widget.""" + can_focus: bool = False """Widget may receive focus.""" can_focus_children: bool = True @@ -488,6 +495,21 @@ def tooltip(self, tooltip: RenderableType | None): except NoScreen: pass + def compose_add_child(self, widget: Widget) -> None: + """Add a node to children. + + This is used by the compose process when it adds children. + There is no need to use it directly, but you may want to override it in a subclass + if you want children to be attached to a different node. + + Args: + widget: A Widget to add. + """ + _rich_traceback_omit = True + if not self.ALLOW_CHILDREN: + raise NotAContainer(f"Can't add children to {type(widget)} widgets") + self._nodes._append(widget) + def __enter__(self) -> Self: """Use as context manager when composing.""" self.app._compose_stacks[-1].append(self) diff --git a/src/textual/widgets/_button.py b/src/textual/widgets/_button.py index e1163a4737..e5a2c2d8b0 100644 --- a/src/textual/widgets/_button.py +++ b/src/textual/widgets/_button.py @@ -153,6 +153,8 @@ class Button(Widget, can_focus=True): BINDINGS = [Binding("enter", "press", "Press Button", show=False)] + ALLOW_CHILDREN = False + label: reactive[TextType] = reactive[TextType]("") """The text label that appears within the button.""" diff --git a/src/textual/widgets/_static.py b/src/textual/widgets/_static.py index a9f485345b..7e72e4bf3f 100644 --- a/src/textual/widgets/_static.py +++ b/src/textual/widgets/_static.py @@ -44,6 +44,8 @@ class Static(Widget, inherit_bindings=False): } """ + ALLOW_CHILDREN = False + _renderable: RenderableType def __init__( diff --git a/src/textual/widgets/_toggle_button.py b/src/textual/widgets/_toggle_button.py index 90828c7ac1..2d2841e672 100644 --- a/src/textual/widgets/_toggle_button.py +++ b/src/textual/widgets/_toggle_button.py @@ -51,6 +51,8 @@ class ToggleButton(Static, can_focus=True): | `toggle--label` | Targets the text label of the toggle button. | """ + ALLOW_CHILDREN = False + DEFAULT_CSS = """ ToggleButton { width: auto; diff --git a/tests/test_widget.py b/tests/test_widget.py index 43f844c430..d5c8407466 100644 --- a/tests/test_widget.py +++ b/tests/test_widget.py @@ -9,8 +9,8 @@ from textual.css.query import NoMatches from textual.geometry import Offset, Size from textual.message import Message -from textual.widget import MountError, PseudoClasses, Widget -from textual.widgets import Label, LoadingIndicator +from textual.widget import MountError, NotAContainer, PseudoClasses, Widget +from textual.widgets import Label, LoadingIndicator, Static @pytest.mark.parametrize( @@ -394,3 +394,19 @@ class TestWidgetIsMountedApp(App): assert widget.is_mounted is False await pilot.app.mount(widget) assert widget.is_mounted is True + + +async def test_not_allow_children(): + """Regression test for https://github.com/Textualize/textual/pull/3758""" + + class TestAppExpectFail(App): + def compose(self) -> ComposeResult: + # Statics don't have children, so this should error + with Static(): + yield Label("foo") + + app = TestAppExpectFail() + + with pytest.raises(NotAContainer): + async with app.run_test(): + pass