Revive textual backend

[rmartin16]

Changes

• Closes Textual apps crash at startup with MountError #2816
• Closes Textual apps are not managing the event loop correctly #2817

Related

• Run testbed with Textual in CI #2823
• Allow test mode for console apps on macOS briefcase#1992

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[rmartin16]

I'm not sure how I feel about this deferred mounting technique. It works...but it feels a little hacky. Definitely open to criticism and other ideas.

[rmartin16]

The Textual docs go in to some depth about composing; a cursory review suggests this may allow for a more natural way to layout widgets before adding them to the app. Although, I'm still quite unfamiliar with Textual's internals...so, can't say for sure....

[rmartin16]

Oh...while I was thinking about it, a lot of Textual's API calls appear to return a Future...however, we don't await any of these. In general, this may not be problematic but...I'd expect possible race conditions to eventually surface.

[freakboy3742]

FWIW - I don't hate the DeferredMount approach; but I can't help but think there's a better approach hiding somewhere under the surface.

There's no inherent reason that the creation of the interface-level hierarchy needs to be immediately mirrored on the impl-level heirarchy - I suspect that what you've done here with an "on-mount" handler could be deferred back to the set_app or set_window handler on the impl... but I'd need to dig a bit to be certain.

[rmartin16]

There's no inherent reason that the creation of the interface-level hierarchy needs to be immediately mirrored on the impl-level heirarchy - I suspect that what you've done here with an "on-mount" handler could be deferred back to the set_app or set_window handler on the impl... but I'd need to dig a bit to be certain.

I think that might be too limited, though. IIUC, hooks like set_app and set_window do not run anytime that window content changes. For instance, if I completely change out the content for main_window in the middle of the app running.

Furthermore, though, we would also need to run such on_mount logic if even nested children change. For instance, I change out the content of an arbitrary box somewhere in the layout.

So, I think we would need a hook that could iterate all children at the time a widget is made part of the "real" layout. Alternatively, though, like I mentioned, I still wonder if Textual's "compose" functionality could be leveraged here.

[freakboy3742]

There's no inherent reason that the creation of the interface-level hierarchy needs to be immediately mirrored on the impl-level heirarchy - I suspect that what you've done here with an "on-mount" handler could be deferred back to the set_app or set_window handler on the impl... but I'd need to dig a bit to be certain.

I think that might be too limited, though. IIUC, hooks like set_app and set_window do not run anytime that window content changes. For instance, if I completely change out the content for main_window in the middle of the app running.

If a widget is part of a deep hierarchy on a window, and the root content of that window is changed, set_window should be called on the deeply nested widget AFAIK - and likewise for the deeply nested widget on the newly added content.

The current handling of content on window calls set_window on the root content widget, then calls set_content on Window to do the mount; if we restructured that so that calling set_window on root content did the mount as well, I think that might avoid the issue?

So, I think we would need a hook that could iterate all children at the time a widget is made part of the "real" layout. Alternatively, though, like I mentioned, I still wonder if Textual's "compose" functionality could be leveraged here.

I'm definitely still on the "learning Textual" part of the curve; and the Textual backend is sufficiently new that I wouldn't rule out a major rewrite if there's a feature we could be using better to make our life easier - especially if it allows for a closer alignment with what Textual considers "correct" usage of their API.

[rmartin16]

If a widget is part of a deep hierarchy on a window, and the root content of that window is changed, set_window should be called on the deeply nested widget AFAIK - and likewise for the deeply nested widget on the newly added content.

Ahhh, ok; I was clearly confused about set_app and set_window 🤦🏼 A quick test, though, shows hope here.

Given this app:

    def startup(self):
        print("creating my-label")
        self.label = toga.Label(text="Hello", id="my-label")
        print("creating my-box")
        self.main_box = toga.Box(children=[self.label], id="my-box")

        print("setting my-box as main_window content")
        self.main_window = toga.MainWindow(content=self.main_box)
        self.main_window.show()

        asyncio.create_task(self.my_task())

    async def my_task(self):
        await asyncio.sleep(2)

        print("creating my-label-2")
        self.label2 = toga.Label(text="Hello", id="my-label-2")
        print("creating my-box-2")
        self.main_box2 = toga.Box(children=[self.label2], id="my-box-2")

        print("setting my-box-2 as main_window content")
        self.main_window.content = self.main_box2

I get this order of events (with a few print commands in toga_core):

creating my-label
creating my-box
calling set_app for my-label
calling set_window for my-label (window=None)
setting my-box as main_window content
calling set_app for my-box
calling set_app for my-label
calling set_window for my-box (window=<toga.window.MainWindow object at 0x73a8de5d5a40>)
calling set_window for my-label (window=<toga.window.MainWindow object at 0x73a8de5d5a40>)

creating my-label-2
creating my-box-2
calling set_app for my-label-2
calling set_window for my-label-2 (window=None)
setting my-box-2 as main_window content
calling set_window for my-box (window=None)
calling set_window for my-label (window=None)
calling set_app for my-box-2
calling set_app for my-label-2
calling set_window for my-box-2 (window=<toga.window.MainWindow object at 0x73a8de5d5a40>)
calling set_window for my-label-2 (window=<toga.window.MainWindow object at 0x73a8de5d5a40>)

I'm thinking in this approach...a Widget's parent would be responsible for mounting it.... 🤔 I'll play around with it.

Thanks for the clarification.

[rmartin16]

The next problem, though, is existing content isn't removed from the main_window....the new content just keeps getting appended. i might not find my way out of this rabbit hole 🤣

[freakboy3742]

The next problem, though, is existing content isn't removed from the main_window....the new content just keeps getting appended. i might not find my way out of this rabbit hole 🤣

I suspect that's oversight, rather than intention. On every other platform, the window can only have one root content object, so set_content() is sufficient. If textual will allow more than one, then an if existing content: clear preamble to the textual backend's set_content() handler should do the job.

[rmartin16]

Yeah...so, RE: clearing the current content: Textual appears to use a CSS selector-like interface to find mounted widgets; given the widgets aren't being assigned IDs, these APIs aren't immediately useful. Furthermore, though, all the widgets are being added to the Screen that Toga creates....so, it's non-trivial to differentiate the widgets that Toga and Textual adds versus which ones the user has added.

As it relates to Textual creating a Task for most API calls, I'm also already starting to see issues with not awaiting them.

For instance, consider this straight-forward app:

    def startup(self):
        self.left_box = toga.Box(id="left-box", style=Pack(flex=1, direction=COLUMN))
        self.left_box._impl.native.styles.background = "blue"

        self.right_box = toga.Box(id="right-box", style=Pack(flex=1, direction=COLUMN))
        self.right_box._impl.native.styles.background = "red"

        self.main_box = toga.Box(
            id="main-box",
            children=[self.left_box, self.right_box],
            style=Pack(flex=1, direction=ROW),
        )

        self.main_window = toga.MainWindow(content=self.main_box)
        self.main_window.show()

Sometimes, the blue box is on the left...other times it is on the right.

Sooo, at this point, I think I'm mostly inclined to just trying to get this back to its previous state where it can at least run.

[freakboy3742]

Yeah...so, RE: clearing the current content: Textual appears to use a CSS selector-like interface to find mounted widgets; given the widgets aren't being assigned IDs, these APIs aren't immediately useful.

We may not be passing it through at present, but every Toga widget has a unique ID... so we could use an ID-based scheme.

Furthermore, though, all the widgets are being added to the Screen that Toga creates....so, it's non-trivial to differentiate the widgets that Toga and Textual adds versus which ones the user has added.

Ok... but any user-created widget will be added either at a known position in a list of children, or as a child of a known container... what am I missing here?

As it relates to Textual creating a Task for most API calls, I'm also already starting to see issues with not awaiting them.

Hrm... if the APIs are explcitly async, that's going to be problematic... I'm intrigued how they're able to offer a sync API if all the internal calls are awaitables, though...

[rmartin16]

Just to be sure it's clear, I'm not saying any of these things are inherently blockers....just sharing what I'm seeing.

Yeah...so, RE: clearing the current content: Textual appears to use a CSS selector-like interface to find mounted widgets; given the widgets aren't being assigned IDs, these APIs aren't immediately useful.

We may not be passing it through at present, but every Toga widget has a unique ID... so we could use an ID-based scheme.

This was actually my first thought; although, Textual requires the ID to start with a letter 🙂 but that's easy enough

Furthermore, though, all the widgets are being added to the Screen that Toga creates....so, it's non-trivial to differentiate the widgets that Toga and Textual adds versus which ones the user has added.

Ok... but any user-created widget will be added either at a known position in a list of children, or as a child of a known container... what am I missing here?

This is true; you can retrieve the children for a widget via the children attribute....however, I couldn't figure out how to "unmount" that widget... The only example I found in the docs used the CSS selector API to return an object you could use to do this. I'm sure this isn't insurmountable...it was just proving harder than I expected....or willing to keep digging for (but a lot of this is stemming from my ignorance of Textual).

As it relates to Textual creating a Task for most API calls, I'm also already starting to see issues with not awaiting them.

Hrm... if the APIs are explcitly async, that's going to be problematic... I'm intrigued how they're able to offer a sync API if all the internal calls are awaitables, though...

Looks like there some magic that returns an object that can be awaited....but it is not necessary to await it. So, Textual must be putting a Task on the event loop to do something....and that something will happen soon. If you await the return value of the API call, you can be assured the Task finished once awaiting returns. These docs talk about it.

[rmartin16]

As it relates to Textual creating a Task for most API calls, I'm also already starting to see issues with not awaiting them.

Hrm... if the APIs are explcitly async, that's going to be problematic... I'm intrigued how they're able to offer a sync API if all the internal calls are awaitables, though...

Looks like there some magic that returns an object that can be awaited....but it is not necessary to await it. So, Textual must be putting a Task on the event loop to do something....and that something will happen soon. If you await the return value of the API call, you can be assured the Task finished once awaiting returns. These docs talk about it.

I looked more closely at this today.

First, it appears the changing order of events for mounting the left and right boxes in my sample app was not an artifact of Textual....but instead that of my implementation at the time for deferred mounting in Toga. So, now, I'm not seeing the layout randomly change for the same layout definition.

Second, I dug in to what's happening synchronously vs asynchronously. When a widget is mounted, the actual registration and addition of the widget to the app is synchronous. What is asynchronous is starting the message processor for the widget; therefore, the widget may not be visible to everything until this message processor is fully up and running. So, not awaiting the mount process shouldn't be as problematic as I thought.

[rmartin16]

The current changes work to restore Textual functionality. I did look again at how support to replace the content of MainWindow might be added. Each of Textual's Widget objects have a remove() method. However, if you use this, it seems to destroy the widget as well such that if you try to restore a widget after calling remove(), you aren't able to restore the layout it once specified. So, this will require more in depth knowledge of Textual's DOM management to implement properly.

[rmartin16]

One other thing while we're here....I haven't been able to dig in to this but sometimes when I press CTRL-C to exit a Textual app....it prints a bunch of ANSI in to my prompt. I'm not sure if this a function of Textual itself...or how Toga is using it.

❯ briefcase dev

[helloworld] Starting in dev mode...
===========================================================================
Executing <Task finished name='Task-299' coro=<App._shutdown() done, defined at /home/russell/.pyenv/versions/briefcase-3.10/lib/python3.10/site-packages/textual/app.py:3039> result=None created at /home/russell/github/beeware/toga/core/src/toga/app.py:502> took 0.169 seconds
^[[<35;66;31M
tmp/beeware/helloworld via 🐍 v3.10.14 (briefcase-3.10) took 4m40s 
❯ 51;64;31M51;64;31M51;64;31M35;64;31M35;64;31M35;64;31M35;65;31M35;65;31M35;65;31M35;65;31M35;65;31M35;65;31M35;65;31M35;65;31M35;66;31M35;66;31M35;66;31M^C

[freakboy3742]

Looks very clean; one question about a possible simplification.

As for the ANSI line noise - I can't say I've seen that...

[freakboy3742]

Is the _pending_children collection needed? Is there any situation where _pending_children will be different to self.interface.children? I can't think of one... the widget is either mounted, in which case all children will be added, or the widget isn't mounted, in which case all children will not be mounted.

[rmartin16]

Yeah; that's probably true. I think I was imagining the abstract case where the mounted state could change over time; this would introduce the possibility that the superset of children for the widget is not the same as the widgets that still need to be mounted on it.

Right now, though, it doesn't even work to remove a widget with children and then add that same widget back to the app; when you do this, the children of that widget are lost.

Given all this, I'll just implement the simpler design where you can mount a widget once; therefore, mounting children is only deferred once.

[freakboy3742]

👍