From b4121723732a5c285f244a7b4053b45524d1afa2 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 24 Apr 2025 11:19:23 +0100 Subject: [PATCH 1/8] avoid exposing asyncio.Future directly to api consumers --- src/textual/app.py | 14 ++++---------- src/textual/screen.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index 6861012863..6b70fa8fa5 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -120,6 +120,7 @@ ScreenResultCallbackType, ScreenResultType, SystemModalScreen, + AwaitScreen, ) from textual.signal import Signal from textual.theme import BUILTIN_THEMES, Theme, ThemeProvider @@ -2650,14 +2651,14 @@ def push_screen( screen: Screen[ScreenResultType] | str, callback: ScreenResultCallbackType[ScreenResultType] | None = None, wait_for_dismiss: Literal[True] = True, - ) -> asyncio.Future[ScreenResultType]: ... + ) -> AwaitScreen[ScreenResultType]: ... def push_screen( self, screen: Screen[ScreenResultType] | str, callback: ScreenResultCallbackType[ScreenResultType] | None = None, wait_for_dismiss: bool = False, - ) -> AwaitMount | asyncio.Future[ScreenResultType]: + ) -> AwaitMount | AwaitScreen[ScreenResultType]: """Push a new [screen](/guide/screens) on the screen stack, making it the current screen. Args: @@ -2678,14 +2679,6 @@ def push_screen( f"push_screen requires a Screen instance or str; not {screen!r}" ) - try: - loop = asyncio.get_running_loop() - except RuntimeError: - # Mainly for testing, when push_screen isn't called in an async context - future: asyncio.Future[ScreenResultType] = asyncio.Future() - else: - future = loop.create_future() - if self._screen_stack: self.screen.post_message(events.ScreenSuspend()) self.screen.refresh() @@ -2695,6 +2688,7 @@ def push_screen( except LookupError: message_pump = self.app + future = AwaitScreen() next_screen._push_result_callback(message_pump, callback, future) self._load_screen_css(next_screen) self._screen_stack.append(next_screen) diff --git a/src/textual/screen.py b/src/textual/screen.py index 1268858e22..ff57173bb9 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -8,6 +8,7 @@ from __future__ import annotations +import enum import asyncio from functools import partial from operator import attrgetter @@ -23,6 +24,8 @@ Optional, TypeVar, Union, + Literal, + Generator, ) import rich.repr @@ -82,6 +85,27 @@ ] """Type of a screen result callback function.""" +class _Unset(enum.Enum): + UNSET = enum.auto() + +class AwaitScreen(Generic[ScreenResultType]): + def __init__(self) -> None: + self._event = asyncio.Event() + self._result: ScreenResultType | Literal[_Unset.UNSET] = _Unset.UNSET + + async def wait(self) -> ScreenResultType: + await self._event.wait() + assert self._result is not _Unset.UNSET + return self._result + + def __await__(self) -> Generator[Any, Any, ScreenResultType]: + return self.wait().__await__() + + def set_result(self, result): + assert self._result is _Unset.UNSET + self._result = result + self._event.set() + @rich.repr.auto class ResultCallback(Generic[ScreenResultType]): @@ -91,7 +115,7 @@ def __init__( self, requester: MessagePump, callback: ScreenResultCallbackType[ScreenResultType] | None, - future: asyncio.Future[ScreenResultType] | None = None, + future: AwaitScreen[ScreenResultType] | None = None, ) -> None: """Initialise the result callback object. @@ -1161,7 +1185,7 @@ def _push_result_callback( self, requester: MessagePump, callback: ScreenResultCallbackType[ScreenResultType] | None, - future: asyncio.Future[ScreenResultType | None] | None = None, + future: AwaitScreen[ScreenResultType] | None = None, ) -> None: """Add a result callback to the screen. From 8b3a3efed44686fcc297e62a90309383b65e45c7 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 24 Apr 2025 11:21:55 +0100 Subject: [PATCH 2/8] update docstrings --- src/textual/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/app.py b/src/textual/app.py index 6b70fa8fa5..f8e5308eb5 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2671,7 +2671,7 @@ def push_screen( NoActiveWorker: If using `wait_for_dismiss` outside of a worker. Returns: - An optional awaitable that awaits the mounting of the screen and its children, or an asyncio Future + An optional awaitable that awaits the mounting of the screen and its children, or an awaitable to await the result of the screen. """ if not isinstance(screen, (Screen, str)): From a75c042201ca9b64f9b38972521ee469ef60ab8f Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 24 Apr 2025 11:24:08 +0100 Subject: [PATCH 3/8] address a mypy issue --- src/textual/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/app.py b/src/textual/app.py index f8e5308eb5..cbb29046ed 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2688,7 +2688,7 @@ def push_screen( except LookupError: message_pump = self.app - future = AwaitScreen() + future: AwaitScreen[ScreenResultType] = AwaitScreen() next_screen._push_result_callback(message_pump, callback, future) self._load_screen_css(next_screen) self._screen_stack.append(next_screen) From 837c1a87cae94ff8bea09c1f971cc011bc8e608e Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 11 May 2025 08:43:44 +0100 Subject: [PATCH 4/8] Apply suggestions from code review --- src/textual/app.py | 4 ++-- src/textual/screen.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/textual/app.py b/src/textual/app.py index cbb29046ed..6bc3416463 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2688,8 +2688,8 @@ def push_screen( except LookupError: message_pump = self.app - future: AwaitScreen[ScreenResultType] = AwaitScreen() - next_screen._push_result_callback(message_pump, callback, future) + await_screen: AwaitScreen[ScreenResultType] = AwaitScreen() + next_screen._push_result_callback(message_pump, callback, await_screen) self._load_screen_css(next_screen) self._screen_stack.append(next_screen) next_screen.post_message(events.ScreenResume()) diff --git a/src/textual/screen.py b/src/textual/screen.py index ff57173bb9..04913c7825 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -89,6 +89,9 @@ class _Unset(enum.Enum): UNSET = enum.auto() class AwaitScreen(Generic[ScreenResultType]): + """ + An Awaitable object that resumes with the result of a Screen. + """ def __init__(self) -> None: self._event = asyncio.Event() self._result: ScreenResultType | Literal[_Unset.UNSET] = _Unset.UNSET From bf718d9a529dfc497a6d3400d75147cfae4932fe Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 11 May 2025 08:46:52 +0100 Subject: [PATCH 5/8] more future -> AwaitScreen --- src/textual/screen.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/textual/screen.py b/src/textual/screen.py index 04913c7825..18f7cba59a 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -118,21 +118,21 @@ def __init__( self, requester: MessagePump, callback: ScreenResultCallbackType[ScreenResultType] | None, - future: AwaitScreen[ScreenResultType] | None = None, + await_screen: AwaitScreen[ScreenResultType] | None = None, ) -> None: """Initialise the result callback object. Args: requester: The object making a request for the callback. callback: The callback function. - future: A Future to hold the result. + await_screen: An AwaitScreen to hold the result. """ self.requester = requester """The object in the DOM that requested the callback.""" self.callback: ScreenResultCallbackType | None = callback """The callback function.""" - self.future = future - """A future for the result""" + self.await_screen = await_screen + """An AwaitScreen for the result""" def __call__(self, result: ScreenResultType) -> None: """Call the callback, passing the given result. @@ -1188,17 +1188,17 @@ def _push_result_callback( self, requester: MessagePump, callback: ScreenResultCallbackType[ScreenResultType] | None, - future: AwaitScreen[ScreenResultType] | None = None, + await_screen: AwaitScreen[ScreenResultType] | None = None, ) -> None: """Add a result callback to the screen. Args: requester: The object requesting the callback. callback: The callback. - future: A Future to hold the result. + await_screen: An AwaitScreen to hold the result. """ self._result_callbacks.append( - ResultCallback[Optional[ScreenResultType]](requester, callback, future) + ResultCallback[Optional[ScreenResultType]](requester, callback, await_screen) ) async def _message_loop_exit(self) -> None: From 7a17d3fecb30a7ad7c5556e984689bafb97ab839 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sat, 31 May 2025 17:47:52 +0100 Subject: [PATCH 6/8] apply black --- src/textual/screen.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/textual/screen.py b/src/textual/screen.py index 44d68d16a8..6b2ccc01fa 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -85,13 +85,16 @@ ] """Type of a screen result callback function.""" + class _Unset(enum.Enum): UNSET = enum.auto() + class AwaitScreen(Generic[ScreenResultType]): """ An Awaitable object that resumes with the result of a Screen. """ + def __init__(self) -> None: self._event = asyncio.Event() self._result: ScreenResultType | Literal[_Unset.UNSET] = _Unset.UNSET @@ -1203,7 +1206,9 @@ def _push_result_callback( await_screen: An AwaitScreen to hold the result. """ self._result_callbacks.append( - ResultCallback[Optional[ScreenResultType]](requester, callback, await_screen) + ResultCallback[Optional[ScreenResultType]]( + requester, callback, await_screen + ) ) async def _message_loop_exit(self) -> None: From 6c92f7e9bd4bffb4629a555c5f4b435755f93c2e Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 1 Jun 2025 08:48:59 +0100 Subject: [PATCH 7/8] fix a missing rename to await_screen --- src/textual/screen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/screen.py b/src/textual/screen.py index 6b2ccc01fa..5536ba1c9a 100644 --- a/src/textual/screen.py +++ b/src/textual/screen.py @@ -146,8 +146,8 @@ def __call__(self, result: ScreenResultType) -> None: Note: If the requested or the callback are `None` this will be a no-op. """ - if self.future is not None: - self.future.set_result(result) + if self.await_screen is not None: + self.await_screen.set_result(result) if self.requester is not None and self.callback is not None: self.requester.call_next(self.callback, result) self.callback = None From a0127f15c5c8a58240a84f6d2e15a72efd1088de Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 1 Jun 2025 09:17:38 +0100 Subject: [PATCH 8/8] fix another un-renamed future --- src/textual/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/app.py b/src/textual/app.py index f4d05d0fbc..88a5e6301f 100644 --- a/src/textual/app.py +++ b/src/textual/app.py @@ -2734,7 +2734,7 @@ def push_screen( raise NoActiveWorker( "push_screen must be run from a worker when `wait_for_dismiss` is True" ) from None - return future + return await_screen else: return await_mount