From 089dce1b41c641b111e68a3dcb5bdfa6a0ea3dc1 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:39:12 +0100 Subject: [PATCH 01/10] Add a unit test for #2502 Currently marked as xfail, but this gets it down to the most basic level. --- tests/test_resolve.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index c36e1a406..e6799d20c 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -1,4 +1,5 @@ from fractions import Fraction +from itertools import chain import pytest @@ -122,3 +123,30 @@ def test_resolve_fraction_unit(): Fraction(32), resolve_dimension="width", ) == Fraction(2) + + +@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2502") +def test_resolve_issue_2502(): + """Test https://github.com/Textualize/textual/issues/2502""" + + widgets_fr = [Widget() for _ in range(50)] + widgets_min_width = [Widget() for _ in range(50)] + + for widget in widgets_fr: + widget.styles.width = "1fr" + widget.styles.min_width = 3 + + for widget in widgets_min_width: + widget.styles.width = 0 + + styles = ( + widget.styles + for widget in chain.from_iterable(zip(widgets_fr, widgets_min_width)) + ) + + assert isinstance( + resolve_fraction_unit( + styles, Size(80, 24), Size(80, 24), Fraction(10), resolve_dimension="width" + ), + Fraction, + ) From 052ec83b7aa25160a5ae8b70d3b765d7f32ae6bf Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:44:30 +0100 Subject: [PATCH 02/10] Make the test as small as possible --- tests/test_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index e6799d20c..a0fde4cd4 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -134,7 +134,7 @@ def test_resolve_issue_2502(): for widget in widgets_fr: widget.styles.width = "1fr" - widget.styles.min_width = 3 + widget.styles.min_width = 1 for widget in widgets_min_width: widget.styles.width = 0 From a5cc96cbc7750a6d41aeaad641b76bc25beadd35 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:44:51 +0100 Subject: [PATCH 03/10] Make a pass of the #2502 test a fail If/when I get this actually passing, I want the test to appear to fail so I know things have changed for the better. This makes sense, trust me. --- tests/test_resolve.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index a0fde4cd4..c7cdd485e 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -125,7 +125,9 @@ def test_resolve_fraction_unit(): ) == Fraction(2) -@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2502") +@pytest.mark.xfail( + strict=True, reason="https://github.com/Textualize/textual/issues/2502" +) def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" From a77dbf4beefc2d0279729af28084c3b77c77fec5 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 10:38:09 +0100 Subject: [PATCH 04/10] Tentative fix for resolve_fraction_unit ZeroDivision error I'll admit to not really following what the code does, so will really need someone with a better understanding of the aim of this code to look over the proposed fix; but based on a bunch of runs and hand-debugging, this seems to do the job. This passes all existing tests and also removes the reported error. On the other hand I'm not confident that I'm *not* just masking an underlying issue with this function. --- src/textual/_resolve.py | 4 ++-- tests/test_resolve.py | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index ddbdd1d5c..1ee40af9e 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -143,7 +143,7 @@ def resolve_fraction_unit( resolved: list[Fraction | None] = [None] * len(resolve) remaining_fraction = Fraction(sum(scalar.value for scalar, _, _ in resolve)) - while True: + while remaining_fraction: remaining_space_changed = False resolve_fraction = Fraction(remaining_space, remaining_fraction) for index, (scalar, min_value, max_value) in enumerate(resolve): @@ -166,7 +166,7 @@ def resolve_fraction_unit( return ( Fraction(remaining_space, remaining_fraction) - if remaining_space + if remaining_space > 0 else Fraction(1) ) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index c7cdd485e..b73db6c55 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -125,9 +125,6 @@ def test_resolve_fraction_unit(): ) == Fraction(2) -@pytest.mark.xfail( - strict=True, reason="https://github.com/Textualize/textual/issues/2502" -) def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" From ee707130023207b8b1e969c633228d9481d7dbce Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 13:35:55 +0100 Subject: [PATCH 05/10] Simplify the resolver zero division bug unit test --- tests/test_resolve.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index b73db6c55..202299747 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -128,24 +128,17 @@ def test_resolve_fraction_unit(): def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" - widgets_fr = [Widget() for _ in range(50)] - widgets_min_width = [Widget() for _ in range(50)] - - for widget in widgets_fr: - widget.styles.width = "1fr" - widget.styles.min_width = 1 - - for widget in widgets_min_width: - widget.styles.width = 0 - - styles = ( - widget.styles - for widget in chain.from_iterable(zip(widgets_fr, widgets_min_width)) - ) + widget = Widget() + widget.styles.width = "1fr" + widget.styles.min_width = 11 assert isinstance( resolve_fraction_unit( - styles, Size(80, 24), Size(80, 24), Fraction(10), resolve_dimension="width" + (widget.styles,), + Size(80, 24), + Size(80, 24), + Fraction(10), + resolve_dimension="width", ), Fraction, ) From 8d7ae4d1fbd6dc74562819036d4b3f0b3d822835 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 13:36:32 +0100 Subject: [PATCH 06/10] Ensure that remaining fraction is always above zero --- src/textual/_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index 1ee40af9e..79a1a1e3e 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -143,7 +143,7 @@ def resolve_fraction_unit( resolved: list[Fraction | None] = [None] * len(resolve) remaining_fraction = Fraction(sum(scalar.value for scalar, _, _ in resolve)) - while remaining_fraction: + while remaining_fraction > 0: remaining_space_changed = False resolve_fraction = Fraction(remaining_space, remaining_fraction) for index, (scalar, min_value, max_value) in enumerate(resolve): From e8baf52bddb95c37839b6653d8047bdd9743df14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 9 May 2023 16:55:43 +0100 Subject: [PATCH 07/10] Allow setting (sub)title of any type. Related issues: #2521. --- CHANGELOG.md | 8 ++++++++ src/textual/app.py | 8 ++++++++ tests/test_app.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa2923fac..542d137f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). + +## Unrealeased + +### Changed + +- App `title` and `sub_title` attributes can be set to any type https://github.com/Textualize/textual/issues/2521 + + ## [0.24.1] - 2023-05-08 ### Fixed diff --git a/src/textual/app.py b/src/textual/app.py index 113a7a220..e5987f857 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -316,6 +316,7 @@ class App(Generic[ReturnType], DOMNode): the name of the app if it doesn't. Assign a new value to this attribute to change the title. + The new value is always converted to string. """ self.sub_title = self.SUB_TITLE if self.SUB_TITLE is not None else "" @@ -328,6 +329,7 @@ class App(Generic[ReturnType], DOMNode): the file being worker on. Assign a new value to this attribute to change the sub-title. + The new value is always converted to string. """ self._logger = Logger(self._log) @@ -406,6 +408,12 @@ class App(Generic[ReturnType], DOMNode): self.set_class(self.dark, "-dark-mode") self.set_class(not self.dark, "-light-mode") + def validate_title(self, title: Any) -> str: + return str(title) + + def validate_sub_title(self, sub_title: Any) -> str: + return str(sub_title) + @property def workers(self) -> WorkerManager: """The [worker](guide/workers/) manager. diff --git a/tests/test_app.py b/tests/test_app.py index e529cfbd7..54bde8221 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -36,3 +36,33 @@ async def test_hover_update_styles(): # We've hovered, so ensure the pseudoclass is present and background changed assert button.pseudo_classes == {"enabled", "hover"} assert button.styles.background != initial_background + + +def test_setting_title(): + app = MyApp() + app.title = None + assert app.title == "None" + + app.title = "" + assert app.title == "" + + app.title = 0.125 + assert app.title == "0.125" + + app.title = [True, False, 2] + assert app.title == "[True, False, 2]" + + +def test_setting_sub_title(): + app = MyApp() + app.sub_title = None + assert app.sub_title == "None" + + app.sub_title = "" + assert app.sub_title == "" + + app.sub_title = 0.125 + assert app.sub_title == "0.125" + + app.sub_title = [True, False, 2] + assert app.sub_title == "[True, False, 2]" From 41466be0079c85f004dd5ee40080a95014e4a23e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?= <5621605+rodrigogiraoserrao@users.noreply.github.com> Date: Tue, 9 May 2023 17:10:08 +0100 Subject: [PATCH 08/10] Add docstrings. --- src/textual/app.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/textual/app.py b/src/textual/app.py index e5987f857..12f66cf04 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -409,9 +409,11 @@ class App(Generic[ReturnType], DOMNode): self.set_class(not self.dark, "-light-mode") def validate_title(self, title: Any) -> str: + """Make sure the title is set to a string.""" return str(title) def validate_sub_title(self, sub_title: Any) -> str: + """Make sure the sub-title is set to a string.""" return str(sub_title) @property From 0eeadf9ae9136eedb830a17bbe6103b717e89495 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 21:01:31 +0100 Subject: [PATCH 09/10] Update the CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa2923fac..2e65e54e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Fixed + +- Fixed `ZeroDivisionError` in `resolve_fraction_unit` https://github.com/Textualize/textual/issues/2502 + ## [0.24.1] - 2023-05-08 ### Fixed From c7fc66fa6c665d688bb945646114b2491cf92571 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Wed, 10 May 2023 09:24:07 +0100 Subject: [PATCH 10/10] Ensure that TreeNode messages are posted when via-API changes are made Until now the Tree.NodeExpanded and Tree.NodeCollapsed messages were only sent out when changes were made to the tree by user interaction. This meant that if any changes were made with the TreeNode expand, expand_all, collapse, collapse_all, toggle or toggle_all API calls no messages would be sent. This PR corrects this. The work here is, in part, required for #2456 (DirectoryTree lazy-loads directory information on node expansion so if someone is expanding nodes under code control the DirectoryTree never gets to know that it should load a directory's content) and will build on #1644, essentially adding a missing aspect to the latter PR. --- CHANGELOG.md | 8 +++++ src/textual/widgets/_tree.py | 4 +-- tests/tree/test_tree_messages.py | 54 ++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa2923fac..3664be152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Fixed + +- Fixed `TreeNode.expand` and `TreeNode.expand_all` not posting a `Tree.NodeExpanded` message https://github.com/Textualize/textual/issues/2535 +- Fixed `TreeNode.collapse` and `TreeNode.collapse_all` not posting a `Tree.NodeCollapsed` message https://github.com/Textualize/textual/issues/2535 +- Fixed `TreeNode.toggle` and `TreeNode.toggle_all` not posting a `Tree.NodeExpanded` or `Tree.NodeCollapsed` message https://github.com/Textualize/textual/issues/2535 + ## [0.24.1] - 2023-05-08 ### Fixed diff --git a/src/textual/widgets/_tree.py b/src/textual/widgets/_tree.py index aab6e9cc5..01cf2f180 100644 --- a/src/textual/widgets/_tree.py +++ b/src/textual/widgets/_tree.py @@ -207,6 +207,7 @@ class TreeNode(Generic[TreeDataType]): """ self._expanded = True self._updates += 1 + self._tree.post_message(Tree.NodeExpanded(self._tree, self)) if expand_all: for child in self.children: child._expand(expand_all) @@ -239,6 +240,7 @@ class TreeNode(Generic[TreeDataType]): """ self._expanded = False self._updates += 1 + self._tree.post_message(Tree.NodeCollapsed(self._tree, self)) if collapse_all: for child in self.children: child._collapse(collapse_all) @@ -1157,10 +1159,8 @@ class Tree(Generic[TreeDataType], ScrollView, can_focus=True): return if node.is_expanded: node.collapse() - self.post_message(self.NodeCollapsed(self, node)) else: node.expand() - self.post_message(self.NodeExpanded(self, node)) async def _on_click(self, event: events.Click) -> None: meta = event.style.meta diff --git a/tests/tree/test_tree_messages.py b/tests/tree/test_tree_messages.py index ef742319a..bc55a7d81 100644 --- a/tests/tree/test_tree_messages.py +++ b/tests/tree/test_tree_messages.py @@ -78,6 +78,20 @@ async def test_tree_node_expanded_message() -> None: assert pilot.app.messages == [("NodeExpanded", "test-tree")] +async def tree_node_expanded_by_code_message() -> None: + """Expanding a node via the API should result in an expanded message being posted.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].expand() + assert pilot.app.messages == [("NodeExpanded", "test-tree")] + + +async def tree_node_all_expanded_by_code_message() -> None: + """Expanding all nodes via the API should result in expanded messages being posted.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].expand_all() + assert pilot.app.messages == [("NodeExpanded", "test-tree")] + + async def test_tree_node_collapsed_message() -> None: """Collapsing a node should result in a collapsed message being emitted.""" async with TreeApp().run_test() as pilot: @@ -89,6 +103,46 @@ async def test_tree_node_collapsed_message() -> None: ] +async def tree_node_collapsed_by_code_message() -> None: + """Collapsing a node via the API should result in a collapsed message being posted.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].expand().collapse() + assert pilot.app.messages == [ + ("NodeExpanded", "test-tree"), + ("NodeCollapsed", "test-tree"), + ] + + +async def tree_node_all_collapsed_by_code_message() -> None: + """Collapsing all nodes via the API should result in collapsed messages being posted.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].expand_all().collapse_all() + assert pilot.app.messages == [ + ("NodeExpanded", "test-tree"), + ("NodeCollapsed", "test-tree"), + ] + + +async def tree_node_toggled_by_code_message() -> None: + """Toggling a node twice via the API should result in expanded and collapsed messages.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].toggle().toggle() + assert pilot.app.messages == [ + ("NodeExpanded", "test-tree"), + ("NodeCollapsed", "test-tree"), + ] + + +async def tree_node_all_toggled_by_code_message() -> None: + """Toggling all nodes twice via the API should result in expanded and collapsed messages.""" + async with TreeApp().run_test() as pilot: + pilot.app.query_one(Tree).root.children[0].toggle_all().toggle_all() + assert pilot.app.messages == [ + ("NodeExpanded", "test-tree"), + ("NodeCollapsed", "test-tree"), + ] + + async def test_tree_node_highlighted_message() -> None: """Highlighting a node should result in a highlighted message being emitted.""" async with TreeApp().run_test() as pilot: