Merge pull request #4795 from Textualize/screen-result

allow None in Screen callback

CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed exception when removing Selects https://github.com/Textualize/textual/pull/4786
 - Fixed issue with non-clickable Footer keys https://github.com/Textualize/textual/pull/4798
 
+### Changed
+
+- Calling `Screen.dismiss` with no arguments will invoke the screen callback with `None` (previously the callback wasn't invoke at all). https://github.com/Textualize/textual/pull/4795
+
 ## [0.73.0] - 2024-07-18
 
 ### Added

docs/examples/guide/screens/modal03.py

@@ -44,7 +44,7 @@ def compose(self) -> ComposeResult:
     def action_request_quit(self) -> None:
         """Action to display the quit dialog."""
 
-        def check_quit(quit: bool) -> None:
+        def check_quit(quit: bool | None) -> None:
             """Called when QuitScreen is dismissed."""
             if quit:
                 self.exit()

src/textual/_debug.py

@@ -0,0 +1,26 @@
+"""
+Functions related to debugging.
+"""
+
+from __future__ import annotations
+
+from . import constants
+
+
+def get_caller_file_and_line() -> str | None:
+    """Get the caller filename and line, if in debug mode, otherwise return `None`:
+
+    Returns:
+        Path and file if `constants.DEBUG==True`
+    """
+
+    if not constants.DEBUG:
+        return None
+    import inspect
+
+    try:
+        current_frame = inspect.currentframe()
+        caller_frame = inspect.getframeinfo(current_frame.f_back.f_back)
+        return f"{caller_frame.filename}:{caller_frame.lineno}"
+    except Exception:
+        return None

src/textual/app.py

@@ -3599,7 +3599,7 @@ def clear_notifications(self) -> None:
     def action_command_palette(self) -> None:
         """Show the Textual command palette."""
         if self.use_command_palette and not CommandPalette.is_open(self):
-            self.push_screen(CommandPalette(), callback=self.call_next)
+            self.push_screen(CommandPalette())
 
     def _suspend_signal(self) -> None:
         """Signal that the application is being suspended."""

src/textual/await_complete.py

@@ -6,6 +6,7 @@
 import rich.repr
 from typing_extensions import Self
 
+from ._debug import get_caller_file_and_line
 from .message_pump import MessagePump
 
 if TYPE_CHECKING:
@@ -27,10 +28,12 @@ def __init__(
         self._awaitables = awaitables
         self._future: Future[Any] = gather(*awaitables)
         self._pre_await: CallbackType | None = pre_await
+        self._caller = get_caller_file_and_line()
 
     def __rich_repr__(self) -> rich.repr.Result:
         yield self._awaitables
         yield "pre_await", self._pre_await, None
+        yield "caller", self._caller, None
 
     def set_pre_await_callback(self, pre_await: CallbackType | None) -> None:
         """Set a callback to run prior to awaiting.

src/textual/await_remove.py

@@ -8,10 +8,14 @@
 from asyncio import Task, gather
 from typing import Generator
 
+import rich.repr
+
 from ._callback import invoke
+from ._debug import get_caller_file_and_line
 from ._types import CallbackType
 
 
+@rich.repr.auto
 class AwaitRemove:
     """An awaitable that waits for nodes to be removed."""
 
@@ -20,6 +24,12 @@ def __init__(
     ) -> None:
         self._tasks = tasks
         self._post_remove = post_remove
+        self._caller = get_caller_file_and_line()
+
+    def __rich_repr__(self) -> rich.repr.Result:
+        yield "tasks", self._tasks
+        yield "post_remove", self._post_remove
+        yield "caller", self._caller, None
 
     async def __call__(self) -> None:
         await self

src/textual/command.py

@@ -39,7 +39,7 @@
 from .reactive import var
 from .screen import Screen, SystemModalScreen
 from .timer import Timer
-from .types import CallbackType, IgnoreReturnCallbackType
+from .types import IgnoreReturnCallbackType
 from .widget import Widget
 from .widgets import Button, Input, LoadingIndicator, OptionList, Static
 from .widgets.option_list import Option
@@ -419,7 +419,7 @@ class CommandInput(Input):
     """
 
 
-class CommandPalette(SystemModalScreen[CallbackType]):
+class CommandPalette(SystemModalScreen):
     """The Textual command palette."""
 
     AUTO_FOCUS = "CommandInput"
@@ -1079,7 +1079,8 @@ def _select_or_command(
                 # decide what to do with it (hopefully it'll run it).
                 self._cancel_gather_commands()
                 self.app.post_message(CommandPalette.Closed(option_selected=True))
-                self.dismiss(self._selected_command.command)
+                self.dismiss()
+                self.call_later(self._selected_command.command)
 
     @on(OptionList.OptionHighlighted)
     def _stop_event_leak(self, event: OptionList.OptionHighlighted) -> None:

src/textual/message_pump.py

@@ -446,6 +446,7 @@ def call_next(self, callback: Callback, *args: Any, **kwargs: Any) -> None:
             *args: Positional arguments to pass to the callable.
             **kwargs: Keyword arguments to pass to the callable.
         """
+        assert callback is not None, "Callback must not be None"
         callback_message = events.Callback(callback=partial(callback, *args, **kwargs))
         callback_message._prevent.update(self._get_prevented_messages())
         self._next_callbacks.append(callback_message)

src/textual/screen.py

@@ -20,10 +20,9 @@
     Generic,
     Iterable,
     Iterator,
-    Type,
+    Optional,
     TypeVar,
     Union,
-    cast,
 )
 
 import rich.repr
@@ -68,7 +67,8 @@
 """The result type of a screen."""
 
 ScreenResultCallbackType = Union[
-    Callable[[ScreenResultType], None], Callable[[ScreenResultType], Awaitable[None]]
+    Callable[[Optional[ScreenResultType]], None],
+    Callable[[Optional[ScreenResultType]], Awaitable[None]],
 ]
 """Type of a screen result callback function."""
 
@@ -110,6 +110,7 @@ def __call__(self, result: ScreenResultType) -> None:
             self.future.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
 
 
 @rich.repr.auto
@@ -209,7 +210,7 @@ def __init__(
         self._dirty_widgets: set[Widget] = set()
         self.__update_timer: Timer | None = None
         self._callbacks: list[tuple[CallbackType, MessagePump]] = []
-        self._result_callbacks: list[ResultCallback[ScreenResultType]] = []
+        self._result_callbacks: list[ResultCallback[ScreenResultType | None]] = []
 
         self._tooltip_widget: Widget | None = None
         self._tooltip_timer: Timer | None = None
@@ -884,7 +885,7 @@ def _push_result_callback(
         self,
         requester: MessagePump,
         callback: ScreenResultCallbackType[ScreenResultType] | None,
-        future: asyncio.Future[ScreenResultType] | None = None,
+        future: asyncio.Future[ScreenResultType | None] | None = None,
     ) -> None:
         """Add a result callback to the screen.
 
@@ -894,7 +895,7 @@ def _push_result_callback(
             future: A Future to hold the result.
         """
         self._result_callbacks.append(
-            ResultCallback[ScreenResultType](requester, callback, future)
+            ResultCallback[Optional[ScreenResultType]](requester, callback, future)
         )
 
     def _pop_result_callback(self) -> None:
@@ -1227,14 +1228,11 @@ def _forward_event(self, event: events.Event) -> None:
         else:
             self.post_message(event)
 
-    class _NoResult:
-        """Class used to mark that there is no result."""
-
-    def dismiss(
-        self, result: ScreenResultType | Type[_NoResult] = _NoResult
-    ) -> AwaitComplete:
+    def dismiss(self, result: ScreenResultType | None = None) -> AwaitComplete:
         """Dismiss the screen, optionally with a result.
 
+        Any callback provided in [push_screen][textual.app.push_screen] will be invoked with the supplied result.
+
         Only the active screen may be dismissed. This method will produce a warning in the logs if
         called on an inactive screen (but otherwise have no effect).
 
@@ -1244,9 +1242,6 @@ def dismiss(
             message handler on the Screen being dismissed. If you want to dismiss the current screen, you can
             call `self.dismiss()` _without_ awaiting.
 
-        If `result` is provided and a callback was set when the screen was [pushed][textual.app.App.push_screen], then
-        the callback will be invoked with `result`.
-
         Args:
             result: The optional result to be passed to the result callback.
 
@@ -1255,8 +1250,9 @@ def dismiss(
         if not self.is_active:
             self.log.warning("Can't dismiss inactive screen")
             return AwaitComplete()
-        if result is not self._NoResult and self._result_callbacks:
-            self._result_callbacks[-1](cast(ScreenResultType, result))
+        if self._result_callbacks:
+            callback = self._result_callbacks[-1]
+            callback(result)
         await_pop = self.app.pop_screen()
 
         def pre_await() -> None:
@@ -1273,9 +1269,7 @@ def pre_await() -> None:
 
         return await_pop
 
-    async def action_dismiss(
-        self, result: ScreenResultType | Type[_NoResult] = _NoResult
-    ) -> None:
+    async def action_dismiss(self, result: ScreenResultType | None = None) -> None:
         """A wrapper around [`dismiss`][textual.screen.Screen.dismiss] that can be called as an action.
 
         Args:

src/textual/widget.py

@@ -50,6 +50,7 @@
 from ._arrange import DockArrangeResult, arrange
 from ._compose import compose
 from ._context import NoActiveAppError, active_app
+from ._debug import get_caller_file_and_line
 from ._dispatch_key import dispatch_key
 from ._easing import DEFAULT_SCROLL_EASING
 from ._layout import Layout
@@ -114,6 +115,7 @@
 _MOUSE_EVENTS_ALLOW_IF_DISABLED = (events.MouseScrollDown, events.MouseScrollUp)
 
 
+@rich.repr.auto
 class AwaitMount:
     """An *optional* awaitable returned by [mount][textual.widget.Widget.mount] and [mount_all][textual.widget.Widget.mount_all].
 
@@ -126,6 +128,12 @@ class AwaitMount:
     def __init__(self, parent: Widget, widgets: Sequence[Widget]) -> None:
         self._parent = parent
         self._widgets = widgets
+        self._caller = get_caller_file_and_line()
+
+    def __rich_repr__(self) -> rich.repr.Result:
+        yield "parent", self._parent
+        yield "widgets", self._widgets
+        yield "caller", self._caller, None
 
     async def __call__(self) -> None:
         """Allows awaiting via a call operation."""

tests/test_modal.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from textual.app import App, ComposeResult
 from textual.containers import Grid
 from textual.screen import ModalScreen
@@ -72,7 +74,7 @@ def compose(self) -> ComposeResult:
     def action_request_quit(self) -> None:
         """Action to display the quit dialog."""
 
-        def check_quit(quit: bool) -> None:
+        def check_quit(quit: bool | None) -> None:
             """Called when QuitScreen is dismissed."""
 
             if quit:

tests/test_screens.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import asyncio
 import sys
 import threading
@@ -326,6 +328,25 @@ def callback(self, result: bool) -> None:
         assert app.bingo
 
 
+async def test_dismiss_action_no_argument():
+    class ConfirmScreen(Screen[bool]):
+        BINDINGS = [("y", "dismiss", "Dismiss")]
+
+    class MyApp(App[None]):
+        bingo = False
+
+        def on_mount(self) -> None:
+            self.push_screen(ConfirmScreen(), callback=self.callback)
+
+        def callback(self, result: bool | None) -> None:
+            self.bingo = result
+
+    app = MyApp()
+    async with app.run_test() as pilot:
+        await pilot.press("y")
+        assert app.bingo is None
+
+
 async def test_switch_screen_no_op():
     """Regression test for https://github.com/Textualize/textual/issues/2650"""
 

tests/test_unmount.py

@@ -6,7 +6,7 @@
 from textual.screen import Screen
 
 
-async def test_unmount():
+async def test_unmount() -> None:
     """Test unmount events are received in reverse DOM order."""
     unmount_ids: list[str] = []