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

[widget Audit] toga.App #2075

Merged
merged 74 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
684a45d
Add documentation and test coverage for app and document.
freakboy3742 Aug 11, 2023
2e19256
Add an example DocumentApp
freakboy3742 Aug 11, 2023
8f77008
Add changenotes.
freakboy3742 Aug 11, 2023
d1a64a4
Correct usage of exit() by main window and quit menu items.
freakboy3742 Aug 11, 2023
2dfa2be
Correct usage of exit in testbed, and add a pause to ensure logs are …
freakboy3742 Aug 11, 2023
8f92cd3
Add docs, docstrings and type annotations for Command.
freakboy3742 Aug 11, 2023
2076dd9
Complete core test coverage for Command.
freakboy3742 Aug 12, 2023
f8c19b3
Remove CommandSet, GROUP_BREAK and SECTION_BREAK from public namespace.
freakboy3742 Aug 12, 2023
e0a46dc
Corrected some coverage issues with a late change to comparisons.
freakboy3742 Aug 12, 2023
fec495d
Factored out a common app fixture.
freakboy3742 Aug 12, 2023
dde078e
Cocoa to 100% (ish) coverage.
freakboy3742 Aug 15, 2023
d27307b
100% coverage of keys.
freakboy3742 Aug 15, 2023
d505c95
Insert a pause on app exit to make sure Briefcase gets all the app logs.
freakboy3742 Aug 16, 2023
035fc8b
Merge branch 'audit-window' into audit-app
freakboy3742 Aug 16, 2023
8219173
Merge branch 'audit-window' into audit-app
freakboy3742 Aug 16, 2023
fedd4fe
GTK app tests passing.
freakboy3742 Aug 16, 2023
6c8877c
hack around segfault for Gtk WebView tests
samschott Aug 16, 2023
596149f
Key tests at 100% on GTK.
samschott Aug 16, 2023
8140de2
Tweaks to get 100% coverage on GTK.
freakboy3742 Aug 17, 2023
adcecf2
Short delay to improve test reliability.
freakboy3742 Aug 17, 2023
ada9303
Add a missing await.
freakboy3742 Aug 17, 2023
cceeb5f
Add some CI debugging help.
freakboy3742 Aug 17, 2023
ca1471d
Merge branch 'audit-window' into audit-app
freakboy3742 Aug 23, 2023
9f24361
100% coverage of app on iOS.
freakboy3742 Aug 23, 2023
a5931ed
Merge branch 'audit-window' into audit-app
freakboy3742 Aug 24, 2023
c6fe640
Corrections to object lifecycle and test probes to pass CI.
freakboy3742 Aug 25, 2023
28771cf
Merge branch 'main' into audit-app
freakboy3742 Oct 18, 2023
1334fc6
Enable 100% coverage requirement on core tests.
freakboy3742 Oct 18, 2023
5cf47cd
Enable 100% test requirement on testbed tests.
freakboy3742 Oct 18, 2023
b7fe16c
Remove backend tests.
freakboy3742 Oct 18, 2023
98ef170
Update docs to remove references to py-core and backend tests.
freakboy3742 Oct 18, 2023
eea80a1
Correct some pre-commit issues.
freakboy3742 Oct 18, 2023
7f42fce
Tweaks for GTK test coverage.
freakboy3742 Oct 18, 2023
69e760b
Ensure constraints are retained until the container is destroyed.
freakboy3742 Oct 19, 2023
5c31e88
Merge branch 'main' into audit-app
freakboy3742 Oct 20, 2023
08b5b33
Increase tolerances for testing on cocoa.
freakboy3742 Oct 20, 2023
9f09703
Merge remote-tracking branch 'origin/main' into audit-app
mhsmith Oct 23, 2023
fb25864
Remove toga_dummy/test_implementation.py
mhsmith Oct 23, 2023
7222344
Update tox and isort comments
mhsmith Oct 23, 2023
3f67033
Clean up App constructor docs
mhsmith Oct 23, 2023
ca8410d
Document App.windows, and related fixes
mhsmith Oct 25, 2023
1a2baec
Deprecate redundant "id" and "name" properties
mhsmith Oct 25, 2023
9127104
Add `App.loop` property
mhsmith Oct 25, 2023
5fdf934
Replace uses of App.name
mhsmith Oct 25, 2023
59654ad
Merge branch 'main' into audit-app
mhsmith Oct 25, 2023
f20899a
Fix WinForms menu initialization
mhsmith Oct 25, 2023
51141bb
More App documentation cleanups
mhsmith Oct 26, 2023
b7977d2
Command documentation cleanups
mhsmith Oct 26, 2023
64064b8
DocumentApp documentation cleanups
mhsmith Oct 26, 2023
a8df430
Correct GTK file dialog usage in DocumentApp.
freakboy3742 Oct 27, 2023
1029f33
examples/command working correctly on WinForms
mhsmith Oct 27, 2023
d185acf
WinForms passing all tests
mhsmith Oct 27, 2023
3938b1e
WinForms app.py at 100% coverage
mhsmith Oct 28, 2023
688da96
All WinForms files at 100% coverage
mhsmith Oct 28, 2023
8b83148
Add debugging for cursor visibility
mhsmith Oct 28, 2023
500ec7f
More CI debugging
mhsmith Oct 28, 2023
74f79fe
More CI debugging
mhsmith Oct 29, 2023
40d7120
More CI debugging
mhsmith Oct 29, 2023
387a4aa
Remove CI debugging
mhsmith Oct 29, 2023
f063b37
examples/command working correctly on Android
mhsmith Oct 29, 2023
4cde00c
Android passing all tests
mhsmith Oct 29, 2023
2e69615
Android app.py at 100% coverage
mhsmith Oct 29, 2023
1583545
All Android files at 100% coverage
mhsmith Oct 29, 2023
ff64a99
Protect against an edge case of garbage collection.
freakboy3742 Oct 30, 2023
dd9babf
Restore WindowSet `__isub__` and `__iadd__` methods
mhsmith Oct 30, 2023
de3fff6
Revert rename of app_name to distribution_name
mhsmith Oct 30, 2023
3c2fa56
Fix link between Window.toolbar and App.commands
mhsmith Oct 30, 2023
3ba85dd
Document the behavior around implicit command registration.
freakboy3742 Oct 30, 2023
7f0b952
Make windowset +=/-= operations no-ops.
freakboy3742 Oct 30, 2023
2b05ca2
Make the app a required argument to commandset if it's going to be used.
freakboy3742 Oct 30, 2023
0df6ea2
Ensure stale references to menus are removed on re-creation.
freakboy3742 Oct 30, 2023
aaca421
An extra scrollcontainer assertion to validate an inconsistent code p…
freakboy3742 Oct 30, 2023
ce9cc85
Ensure Winforms table warnings are captured.
freakboy3742 Oct 31, 2023
207b440
Add a test of adding a command to both toolbar and menus
mhsmith Oct 31, 2023
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
50 changes: 26 additions & 24 deletions core/src/toga/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from collections.abc import Iterator, MutableSet
from email.message import Message
from importlib import metadata as importlib_metadata
from typing import Any, Protocol
from typing import Any, Iterable, Protocol

from toga.command import CommandSet
from toga.documents import Document
Expand Down Expand Up @@ -74,22 +74,14 @@ class WindowSet(MutableSet):
def __init__(self, app: App):
"""A collection of windows managed by an app.

A window is automatically added to the app when it is shown. Alternatively, the
window can be explicitly added to the app (without being shown) using
``app.windows.add(toga.Window(...))`` or ``app.windows += toga.Window(...)``.
Adding a window to an App's window set automatically sets the
A window is automatically added to the app when it is created, and removed when
it is closed. Adding a window to an App's window set automatically sets the
:attr:`~toga.Window.app` property of the Window.

:param app: The app maintaining the window set.
"""
self.app = app
self.elements = set()

def add(self, window: Window) -> None:
"""Add a window to the window set.

:param window: The :class:`toga.Window` to add
"""
if not isinstance(window, Window):
raise TypeError("Can only add objects of type toga.Window")
# Silently not add if duplicate
Expand All @@ -98,24 +90,12 @@ def add(self, window: Window) -> None:
window.app = self.app

def discard(self, window: Window) -> None:
"""Remove a window from the Window set.

:param window: The :class:`toga.Window` to remove.
"""
if not isinstance(window, Window):
raise TypeError("Can only discard objects of type toga.Window")
if window not in self.elements:
raise ValueError(f"{window!r} is not part of this app")
self.elements.remove(window)

def __iadd__(self, window: Window) -> None:
self.add(window)
return self

def __isub__(self, other: Window) -> None:
self.discard(other)
return self

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although they're no longer required, there will be enough code in existence that uses these methods that I don't think we can delete them completely. Toga's own examples contain a bunch of uses of this.

At the very least, they should exist, but be no-ops that log DeprecationWarnings.

Copy link
Member

@mhsmith mhsmith Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've restored them and added warnings, since they were never consistent with the standard set interface anyway.

I could only find one remaining usage in the examples, which I see you've already removed in #2160.

def __iter__(self) -> Iterator:
return iter(self.elements)

Expand Down Expand Up @@ -244,6 +224,7 @@ def __init__(
description: str | None = None,
startup: AppStartupMethod | None = None,
on_exit: OnExitHandler | None = None,
windows=None, # DEPRECATED
):
"""Create a new App instance.

Expand Down Expand Up @@ -281,7 +262,22 @@ def __init__(
:param description: A brief (one line) description of the app. If not provided,
the metadata key ``Summary`` will be used.
:param startup: A callable to run before starting the app.
:param on_exit: The handler to invoke before the application exits.
:param windows: **DEPRECATED** – Windows are now automatically added to the
current app. Passing this argument will cause an exception.
"""
######################################################################
# 2023-10: Backwards compatibility
######################################################################
if windows is not None:
raise ValueError(
"The `windows` constructor argument of toga.App has been removed. "
"Windows are now automatically added to the current app."
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a full exception? Elsewhere we've logged a DeprecationWarning and transformed the input as best we can.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of the Window audit, I changed it so that Windows are added to the App when they're created, instead of when they're shown. Then as part of this PR, I added a check to make sure that the App is created before the Window:

self._app = None
if App.app is None:
raise RuntimeError("Cannot create a Window before creating an App")
App.app.windows.add(self)

I appreciate that this may break some existing code, but I think it's worth it because it makes it significantly easier to reason about the relationship between App and Window, both for us and for the users. Anyone who's been following the style of the Toga examples will not be affected.

This means that any windows passed to the App constructor must already have been added to a different App. so there's no way we can handle that situation sensibly.

######################################################################
# End backwards compatibility
######################################################################

# Initialize empty widgets registry
self._widgets = WidgetRegistry()

Expand Down Expand Up @@ -416,7 +412,7 @@ def __init__(
self._startup_method = startup

self._main_window = None
self.windows = WindowSet(self)
self._windows = WindowSet(self)

self._full_screen_windows = None

Expand Down Expand Up @@ -525,6 +521,12 @@ def widgets(self) -> WidgetRegistry:
"""
return self._widgets

@property
def windows(self) -> Iterable[Window]:
"""The windows managed by the app. Windows are added to the app when they are
created, and removed when they are closed."""
mhsmith marked this conversation as resolved.
Show resolved Hide resolved
return self._windows

@property
def main_window(self) -> MainWindow:
"""The main window for the app."""
Expand Down
6 changes: 4 additions & 2 deletions core/src/toga/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ def __init__(
)

self._app = None
App.app.windows += self
if App.app is None:
raise RuntimeError("Cannot create a Window before creating an App")
App.app.windows.add(self)

self._toolbar = CommandSet(on_change=self._impl.create_toolbar)
self.on_close = on_close
Expand Down Expand Up @@ -310,7 +312,7 @@ def close(self) -> None:
undefined, except for :attr:`closed` which can be used to check if the window
was closed.
"""
self.app.windows -= self
self.app.windows.discard(self)
self._impl.close()
self._closed = True

Expand Down
19 changes: 14 additions & 5 deletions core/tests/app/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,15 +282,24 @@ def test_create(


@pytest.mark.parametrize(
"kwargs, message",
"kwargs, exc_type, message",
[
(dict(), "Toga application must have a formal name"),
(dict(formal_name="Something"), "Toga application must have an app ID"),
(dict(), RuntimeError, "Toga application must have a formal name"),
(
dict(formal_name="Something"),
RuntimeError,
"Toga application must have an app ID",
),
(
dict(windows=()),
ValueError,
"The `windows` constructor argument of toga.App has been removed",
),
],
)
def test_bad_app_creation(kwargs, message):
def test_bad_app_creation(kwargs, exc_type, message):
"""Errors are raised"""
with pytest.raises(RuntimeError, match=message):
with pytest.raises(exc_type, match=message):
toga.App(**kwargs)


Expand Down
35 changes: 0 additions & 35 deletions core/tests/app/test_windowset.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,38 +58,3 @@ def test_add_discard(app, window1, window2):
match=r"Can only discard objects of type toga.Window",
):
app.windows.discard(object())


def test_add_discard_by_operator(app, window1, window2):
"""An item can be added to a windowset by inline operators"""
# The windowset has 3 windows - the main window, plus 2 extras
assert len(app.windows) == 3

with pytest.raises(
TypeError,
match=r"Can only add objects of type toga.Window",
):
app.windows += object()

# Explicitly re-add a window that is already in the windowset
app.windows += window2
assert len(app.windows) == 3
assert window2 in app.windows
assert window2.app == app

# Explicitly discard a window that is in the windowset
app.windows -= window2
assert window2 not in app.windows

# Duplicate discard - it's no longer a member
with pytest.raises(
ValueError,
match=r"<toga.window.Window object at .*> is not part of this app",
):
app.windows -= window2

with pytest.raises(
TypeError,
match=r"Can only discard objects of type toga.Window",
):
app.windows -= object()
9 changes: 9 additions & 0 deletions core/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ def test_window_created_explicit(app):
assert window.on_close._raw == on_close_handler


def test_window_created_without_app():
"A window cannot be created without an active app"
toga.App.app = None
with pytest.raises(
RuntimeError, match="Cannot create a Window before creating an App"
):
toga.Window()


def test_set_app(window, app):
"""A window's app cannot be reassigned"""
assert window.app == app
Expand Down
2 changes: 0 additions & 2 deletions docs/reference/api/app.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ Reference
.. autoclass:: toga.App
:exclude-members: app

.. autoclass:: toga.app.WindowSet

.. autoprotocol:: toga.app.AppStartupMethod
.. autoprotocol:: toga.app.BackgroundTask
.. autoprotocol:: toga.app.OnExitHandler