Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow None in Screen callback #4795

Merged
merged 12 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fixed issues in Kitty terminal after exiting app https://github.com/Textualize/textual/issues/4779
- Fixed exception when removing Selects https://github.com/Textualize/textual/pull/4786

### 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
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/guide/screens/modal03.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
26 changes: 26 additions & 0 deletions src/textual/_debug.py
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
3 changes: 3 additions & 0 deletions src/textual/await_complete.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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.
Expand Down
10 changes: 10 additions & 0 deletions src/textual/await_remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/textual/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -419,7 +419,7 @@ class CommandInput(Input):
"""


class CommandPalette(SystemModalScreen[CallbackType]):
class CommandPalette(SystemModalScreen):
"""The Textual command palette."""

AUTO_FOCUS = "CommandInput"
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/textual/message_pump.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 14 additions & 20 deletions src/textual/screen.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@
Generic,
Iterable,
Iterator,
Type,
Optional,
TypeVar,
Union,
cast,
)

import rich.repr
Expand Down Expand Up @@ -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."""

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand All @@ -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:
Expand Down Expand Up @@ -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).

Expand All @@ -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.

Expand All @@ -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:
Expand All @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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].

Expand 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."""
Expand Down
4 changes: 3 additions & 1 deletion tests/test_modal.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

from textual.app import App, ComposeResult
from textual.containers import Grid
from textual.screen import ModalScreen
Expand Down Expand Up @@ -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:
Expand Down
21 changes: 21 additions & 0 deletions tests/test_screens.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import asyncio
import sys
import threading
Expand Down Expand Up @@ -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"""

Expand Down
2 changes: 1 addition & 1 deletion tests/test_unmount.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = []

Expand Down
Loading