From 5a8e492294458b4a2f3992c9e795d13a97164bb5 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 13 Oct 2022 16:43:59 +0100 Subject: [PATCH 1/5] depth first search --- sandbox/will/screens_focus.css | 9 ++++ sandbox/will/screens_focus.py | 20 +++++++++ src/textual/app.py | 4 +- src/textual/dom.py | 80 +++++++++++++++++++++++++++------- src/textual/widget.py | 4 -- tests/test_dom.py | 26 +++++++++++ 6 files changed, 122 insertions(+), 21 deletions(-) create mode 100644 sandbox/will/screens_focus.css create mode 100644 sandbox/will/screens_focus.py diff --git a/sandbox/will/screens_focus.css b/sandbox/will/screens_focus.css new file mode 100644 index 0000000000..dd2a3ab260 --- /dev/null +++ b/sandbox/will/screens_focus.css @@ -0,0 +1,9 @@ + Focusable { + padding: 3 6; + background: blue 20%; + } + + Focusable :focus { + border: solid red; + } + diff --git a/sandbox/will/screens_focus.py b/sandbox/will/screens_focus.py new file mode 100644 index 0000000000..2d35f54705 --- /dev/null +++ b/sandbox/will/screens_focus.py @@ -0,0 +1,20 @@ +from textual.app import App, ComposeResult +from textual.widgets import Static, Footer + + +class Focusable(Static, can_focus=True): + pass + + +class ScreensFocusApp(App): + def compose(self) -> ComposeResult: + yield Focusable("App - one") + yield Focusable("App - two") + yield Focusable("App - three") + yield Focusable("App - four") + yield Footer() + + +app = ScreensFocusApp(css_path="screens_focus.css") +if __name__ == "__main__": + app.run() diff --git a/src/textual/app.py b/src/textual/app.py index d286dcffe5..0b3789623b 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -1394,7 +1394,9 @@ async def _on_remove(self, event: events.Remove) -> None: if parent is not None: parent.refresh(layout=True) - remove_widgets = list(widget.walk_children(Widget, with_self=True)) + remove_widgets = list( + widget.walk_children(Widget, with_self=True, method="depth") + ) for child in remove_widgets: self._unregister(child) for child in remove_widgets: diff --git a/src/textual/dom.py b/src/textual/dom.py index f3234ac866..1d3712e38b 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -1,7 +1,9 @@ from __future__ import annotations +from collections import deque from inspect import getfile import re +import sys from typing import ( cast, ClassVar, @@ -40,10 +42,23 @@ from .screen import Screen from .widget import Widget +if sys.version_info >= (3, 8): + from typing import Literal, Iterable, Sequence +else: + from typing_extensions import Literal + +if sys.version_info >= (3, 10): + from typing import TypeAlias +else: # pragma: no cover + from typing_extensions import TypeAlias + _re_identifier = re.compile(IDENTIFIER) +WalkMethod: TypeAlias = Literal["depth", "breadth"] + + class BadIdentifier(Exception): """raised by check_identifiers.""" @@ -617,11 +632,14 @@ def walk_children( filter_type: type[WalkType], *, with_self: bool = True, + method: WalkMethod = "breadth", ) -> Iterable[WalkType]: ... @overload - def walk_children(self, *, with_self: bool = True) -> Iterable[DOMNode]: + def walk_children( + self, *, with_self: bool = True, method: WalkMethod = "breadth" + ) -> Iterable[DOMNode]: ... def walk_children( @@ -629,6 +647,7 @@ def walk_children( filter_type: type[WalkType] | None = None, *, with_self: bool = True, + method: WalkMethod = "breadth", ) -> Iterable[DOMNode | WalkType]: """Generate descendant nodes. @@ -636,29 +655,58 @@ def walk_children( 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. + method (Literal["breadth", "depth"], optional): One of "depth" or "breadth". Defaults to "breadth". Returns: Iterable[DOMNode | WalkType]: An iterable of nodes. """ - stack: list[Iterator[DOMNode]] = [iter(self.children)] - pop = stack.pop - push = stack.append - check_type = filter_type or DOMNode + def walk_breadth_first() -> Iterable[DOMNode]: + """Walk the tree breadth first (parent's first).""" + stack: list[Iterator[DOMNode]] = [iter(self.children)] + pop = stack.pop + push = stack.append + check_type = filter_type or DOMNode - if with_self and isinstance(self, check_type): - yield self + if with_self and isinstance(self, check_type): + yield self - while stack: - node = next(stack[-1], None) - if node is None: - pop() - else: - if isinstance(node, check_type): - yield node - if node.children: - push(iter(node.children)) + while stack: + node = next(stack[-1], None) + if node is None: + pop() + else: + if isinstance(node, check_type): + yield node + if node.children: + push(iter(node.children)) + + def walk_depth_first() -> Iterable[DOMNode]: + """Walk the tree depth first (children first).""" + depth_stack: list[tuple[DOMNode, Iterator[DOMNode]]] = ( + [(self, iter(self.children))] + if with_self + else [(node, iter(node.children)) for node in reversed(self.children)] + ) + pop = depth_stack.pop + push = depth_stack.append + check_type = filter_type or DOMNode + + while depth_stack: + node, iter_nodes = pop() + child_widget = next(iter_nodes, None) + if child_widget is None: + if isinstance(node, check_type): + yield node + else: + push((node, iter_nodes)) + push((child_widget, iter(child_widget.children))) + + if method == "depth": + yield from walk_depth_first() + else: + yield from walk_breadth_first() def get_child(self, id: str) -> DOMNode: """Return the first child (immediate descendent) of this node with the given ID. diff --git a/src/textual/widget.py b/src/textual/widget.py index 6f190ec09a..23858b64e5 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -1857,10 +1857,6 @@ async def handle_key(self, event: events.Key) -> bool: await self.action(binding.action) return True - def _on_compose(self, event: events.Compose) -> None: - widgets = self.compose() - self.app.mount_all(widgets) - def _on_mount(self, event: events.Mount) -> None: widgets = self.compose() self.mount(*widgets) diff --git a/tests/test_dom.py b/tests/test_dom.py index e4254f6e55..3a35768d34 100644 --- a/tests/test_dom.py +++ b/tests/test_dom.py @@ -75,3 +75,29 @@ def test_validate(): node.remove_class("1") with pytest.raises(BadIdentifier): node.toggle_class("1") + + +def test_walk_children(parent): + children = [node.id for node in parent.walk_children(with_self=False)] + assert children == ["child1", "grandchild1", "child2"] + + +def test_walk_children_with_self(parent): + children = [node.id for node in parent.walk_children(with_self=True)] + assert children == ["parent", "child1", "grandchild1", "child2"] + + +def test_walk_children_depth(parent): + children = [ + node.id for node in parent.walk_children(with_self=False, method="depth") + ] + print(children) + assert children == ["grandchild1", "child1", "child2"] + + +def test_walk_children_with_self_depth(parent): + children = [ + node.id for node in parent.walk_children(with_self=True, method="depth") + ] + print(children) + assert children == ["grandchild1", "child1", "child2", "parent"] From 7eb5119fe0a6e05e7478d14fa3a96e630acd6a9b Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 13 Oct 2022 20:51:03 +0100 Subject: [PATCH 2/5] real breadth first, and tests --- src/textual/app.py | 11 ++++--- src/textual/css/styles.py | 2 +- src/textual/dom.py | 57 ++++++++++++++++++++----------------- tests/test_dom.py | 60 +++++++++++++++++++++++++++++++-------- 4 files changed, 87 insertions(+), 43 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 0b3789623b..7cf010f3c1 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -37,7 +37,7 @@ from .design import ColorSystem from .devtools.client import DevtoolsClient, DevtoolsConnectionError, DevtoolsLog from .devtools.redirect_output import StdoutRedirector -from .dom import DOMNode +from .dom import DOMNode, NoScreen from .driver import Driver from .drivers.headless_driver import HeadlessDriver from .features import FeatureFlag, parse_features @@ -1142,7 +1142,10 @@ def _unregister(self, widget: Widget) -> None: Args: widget (Widget): A Widget to unregister """ - widget.screen._reset_focus(widget) + try: + widget.screen._reset_focus(widget) + except NoScreen: + pass if isinstance(widget._parent, Widget): widget._parent.children._remove(widget) @@ -1394,8 +1397,8 @@ async def _on_remove(self, event: events.Remove) -> None: if parent is not None: parent.refresh(layout=True) - remove_widgets = list( - widget.walk_children(Widget, with_self=True, method="depth") + remove_widgets = widget.walk_children( + Widget, with_self=True, method="depth", reverse=True ) for child in remove_widgets: self._unregister(child) diff --git a/src/textual/css/styles.py b/src/textual/css/styles.py index d59bea2f66..dd4b8d4b63 100644 --- a/src/textual/css/styles.py +++ b/src/textual/css/styles.py @@ -573,7 +573,7 @@ def refresh(self, *, layout: bool = False, children: bool = False) -> None: if self.node is not None: self.node.refresh(layout=layout) if children: - for child in self.node.walk_children(with_self=False): + for child in self.node.walk_children(with_self=False, reverse=True): child.refresh(layout=layout) def reset(self) -> None: diff --git a/src/textual/dom.py b/src/textual/dom.py index 1d3712e38b..caea2c1f00 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -632,13 +632,18 @@ def walk_children( filter_type: type[WalkType], *, with_self: bool = True, - method: WalkMethod = "breadth", + method: WalkMethod = "depth", + reverse: bool = False, ) -> Iterable[WalkType]: ... @overload def walk_children( - self, *, with_self: bool = True, method: WalkMethod = "breadth" + self, + *, + with_self: bool = True, + method: WalkMethod = "depth", + reverse: bool = False, ) -> Iterable[DOMNode]: ... @@ -647,7 +652,8 @@ def walk_children( filter_type: type[WalkType] | None = None, *, with_self: bool = True, - method: WalkMethod = "breadth", + method: WalkMethod = "depth", + reverse: bool = False, ) -> Iterable[DOMNode | WalkType]: """Generate descendant nodes. @@ -655,14 +661,15 @@ def walk_children( 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. - method (Literal["breadth", "depth"], optional): One of "depth" or "breadth". Defaults to "breadth". + method (Literal["breadth", "depth"], optional): One of "depth" or "breadth". Defaults to "depth". + reverse (bool, optional): Reverse the order (bottom up). Defaults to False Returns: Iterable[DOMNode | WalkType]: An iterable of nodes. """ - def walk_breadth_first() -> Iterable[DOMNode]: + def walk_depth_first() -> Iterable[DOMNode]: """Walk the tree breadth first (parent's first).""" stack: list[Iterator[DOMNode]] = [iter(self.children)] pop = stack.pop @@ -682,31 +689,29 @@ def walk_breadth_first() -> Iterable[DOMNode]: if node.children: push(iter(node.children)) - def walk_depth_first() -> Iterable[DOMNode]: + def walk_breadth_first() -> Iterable[DOMNode]: """Walk the tree depth first (children first).""" - depth_stack: list[tuple[DOMNode, Iterator[DOMNode]]] = ( - [(self, iter(self.children))] - if with_self - else [(node, iter(node.children)) for node in reversed(self.children)] - ) - pop = depth_stack.pop - push = depth_stack.append - check_type = filter_type or DOMNode + queue: deque[DOMNode] = deque() + popleft = queue.popleft + extend = queue.extend - while depth_stack: - node, iter_nodes = pop() - child_widget = next(iter_nodes, None) - if child_widget is None: - if isinstance(node, check_type): - yield node - else: - push((node, iter_nodes)) - push((child_widget, iter(child_widget.children))) + if with_self: + yield self + queue.extend(self.children) + + while queue: + node = popleft() + yield node + extend(node.children) + + node_generator = ( + walk_depth_first() if method == "depth" else walk_breadth_first() + ) - if method == "depth": - yield from walk_depth_first() + if reverse: + yield from reversed(list(node_generator)) else: - yield from walk_breadth_first() + yield from node_generator def get_child(self, id: str) -> DOMNode: """Return the first child (immediate descendent) of this node with the given ID. diff --git a/tests/test_dom.py b/tests/test_dom.py index 3a35768d34..5a713193a7 100644 --- a/tests/test_dom.py +++ b/tests/test_dom.py @@ -77,27 +77,63 @@ def test_validate(): node.toggle_class("1") -def test_walk_children(parent): - children = [node.id for node in parent.walk_children(with_self=False)] - assert children == ["child1", "grandchild1", "child2"] +@pytest.fixture +def search(): + """ + a + / \ + b c + / / \ + d e f + """ + a = DOMNode(id="a") + b = DOMNode(id="b") + c = DOMNode(id="c") + d = DOMNode(id="d") + e = DOMNode(id="e") + f = DOMNode(id="f") + + a._add_child(b) + a._add_child(c) + b._add_child(d) + c._add_child(e) + c._add_child(f) + + yield a + + +def test_walk_children_depth(search): + children = [ + node.id for node in search.walk_children(method="depth", with_self=False) + ] + assert children == ["b", "d", "c", "e", "f"] -def test_walk_children_with_self(parent): - children = [node.id for node in parent.walk_children(with_self=True)] - assert children == ["parent", "child1", "grandchild1", "child2"] +def test_walk_children_with_self_depth(search): + children = [ + node.id for node in search.walk_children(method="depth", with_self=True) + ] + assert children == ["a", "b", "d", "c", "e", "f"] -def test_walk_children_depth(parent): +def test_walk_children_breadth(search): children = [ - node.id for node in parent.walk_children(with_self=False, method="depth") + node.id for node in search.walk_children(with_self=False, method="breadth") ] print(children) - assert children == ["grandchild1", "child1", "child2"] + assert children == ["b", "c", "d", "e", "f"] -def test_walk_children_with_self_depth(parent): +def test_walk_children_with_self_breadth(search): children = [ - node.id for node in parent.walk_children(with_self=True, method="depth") + node.id for node in search.walk_children(with_self=True, method="breadth") ] print(children) - assert children == ["grandchild1", "child1", "child2", "parent"] + assert children == ["a", "b", "c", "d", "e", "f"] + + children = [ + node.id + for node in search.walk_children(with_self=True, method="breadth", reverse=True) + ] + + assert children == ["f", "e", "d", "c", "b", "a"] From de4ca1509e85de880655a600016c8b17fac8df12 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 13 Oct 2022 21:01:04 +0100 Subject: [PATCH 3/5] typing improvements --- src/textual/dom.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index caea2c1f00..e6d86180f3 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -670,7 +670,7 @@ def walk_children( """ def walk_depth_first() -> Iterable[DOMNode]: - """Walk the tree breadth first (parent's first).""" + """Walk the tree depth first (parents first).""" stack: list[Iterator[DOMNode]] = [iter(self.children)] pop = stack.pop push = stack.append @@ -678,7 +678,6 @@ def walk_depth_first() -> Iterable[DOMNode]: if with_self and isinstance(self, check_type): yield self - while stack: node = next(stack[-1], None) if node is None: @@ -690,28 +689,29 @@ def walk_depth_first() -> Iterable[DOMNode]: push(iter(node.children)) def walk_breadth_first() -> Iterable[DOMNode]: - """Walk the tree depth first (children first).""" + """Walk the tree breadth first (children first).""" queue: deque[DOMNode] = deque() popleft = queue.popleft extend = queue.extend + check_type = filter_type or DOMNode - if with_self: + if with_self and isinstance(self, check_type): yield self - queue.extend(self.children) - + extend(self.children) while queue: node = popleft() - yield node + if isinstance(node, check_type): + yield node extend(node.children) node_generator = ( walk_depth_first() if method == "depth" else walk_breadth_first() ) + nodes = list(node_generator) if reverse: - yield from reversed(list(node_generator)) - else: - yield from node_generator + nodes.reverse() + yield from nodes def get_child(self, id: str) -> DOMNode: """Return the first child (immediate descendent) of this node with the given ID. From 9392fbfae49b12b8e98dae60227610d65591812e Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 13 Oct 2022 21:04:10 +0100 Subject: [PATCH 4/5] imports --- src/textual/dom.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index e6d86180f3..314a5e879c 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -1,18 +1,18 @@ from __future__ import annotations -from collections import deque -from inspect import getfile import re import sys +from collections import deque +from inspect import getfile from typing import ( - cast, + TYPE_CHECKING, ClassVar, Iterable, Iterator, Type, - overload, TypeVar, - TYPE_CHECKING, + cast, + overload, ) import rich.repr @@ -25,14 +25,14 @@ from ._context import NoActiveAppError from ._node_list import NodeList from .binding import Bindings, BindingType -from .color import Color, WHITE, BLACK +from .color import BLACK, WHITE, Color from .css._error_tools import friendly_list from .css.constants import VALID_DISPLAY, VALID_VISIBILITY -from .css.errors import StyleValueError, DeclarationError +from .css.errors import DeclarationError, StyleValueError from .css.parse import parse_declarations -from .css.styles import Styles, RenderStyles -from .css.tokenize import IDENTIFIER from .css.query import NoMatches +from .css.styles import RenderStyles, Styles +from .css.tokenize import IDENTIFIER from .message_pump import MessagePump from .timer import Timer @@ -43,7 +43,7 @@ from .widget import Widget if sys.version_info >= (3, 8): - from typing import Literal, Iterable, Sequence + from typing import Literal else: from typing_extensions import Literal From 14316fdbca7e1ba0e7d0c642b5c90d593d154ea4 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 13 Oct 2022 21:41:22 +0100 Subject: [PATCH 5/5] comment --- src/textual/dom.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index 314a5e879c..cc3f0866e2 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -708,10 +708,13 @@ def walk_breadth_first() -> Iterable[DOMNode]: walk_depth_first() if method == "depth" else walk_breadth_first() ) + # We want a snapshot of the DOM at this point + # So that is doesn't change mid-walk nodes = list(node_generator) if reverse: - nodes.reverse() - yield from nodes + yield from reversed(nodes) + else: + yield from nodes def get_child(self, id: str) -> DOMNode: """Return the first child (immediate descendent) of this node with the given ID.