Textual apps require window content to run

[rmartin16]

Describe the bug

If a Toga textual app does not define any window content, the app will crash.

❯ TOGA_BACKEND=toga_textual \
python -c 'import toga; toga.App(formal_name="MyApp", app_id="MyApp").main_loop()'

╭───────────────────────────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────────────────────────╮
│ /home/russell/github/beeware/toga/textual/src/toga_textual/window.py:111 in on_resize                                                                               │
│                                                                                                                                                                     │
│   108 │   │   self.impl = impl                                                                                                                                      │
│   109 │                                                                                                                                                             │
│   110 │   def on_resize(self, event) -> None:                                                                                                                       │
│ ❱ 111 │   │   self.interface.content.refresh()                                                                                                                      │
│   112                                                                                                                                                               │
│   113                                                                                                                                                               │
│   114 class Window:                                                                                                                                                 │
│                                                                                                                                                                     │
│ ╭───────────────────────────────────────── locals ─────────────────────────────────────────╮                                                                        │
│ │ event = Resize(size=Size(width=167, height=43), virtual_size=Size(width=167, height=43)) │                                                                        │
│ │  self = TogaMainWindow()                                                                 │                                                                        │
│ ╰──────────────────────────────────────────────────────────────────────────────────────────╯                                                                        │
╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'NoneType' object has no attribute 'refresh'

Steps to reproduce

Run this app:

TOGA_BACKEND=toga_textual \
python -c 'import toga; toga.App(formal_name="MyApp", app_id="MyApp").main_loop()'

Windows:

$env:TOGA_BACKEND="toga_textual"
python -c 'import toga; toga.App(formal_name="MyApp", app_id="MyApp").main_loop()'

Expected behavior

Apps using the textual backend are not required to use window content.

Screenshots

No response

Environment

• Operating System: all
• Python version:
• Software versions:
  • Briefcase: 0.3.20.dev192+g32f529a4
  • Toga: 0.4.7.dev43+g63480d545

Logs

No response

Additional context

No response

[freakboy3742]

This came up recently in the context of #2473, there's some edge cases where it would be helpful to guarantee that window.content is always a valid widget, even if it's just an empty toga.Box(). That's easy enough to guarantee in the constructor, and has no appreciable downside that I can see, so we might as well do it.

[cgkoutzigiannis]

I would like to give this a go! If I understand the codebase correctly the change should be made here, to check if the interface.content is a valid Widget: https://github.com/beeware/toga/blob/bd0a5922890648b56aee37d01d22d0f8acf107cf/textual/src/toga_textual/window.py#L114-L119 Sidenote: It's kind of hard to debug the Textual backend. I usually use the pdb debugger, but Textual completely hijacks the terminal window so that doesn't work at all. Any tips to overcome that?

[rmartin16]

Sidenote: It's kind of hard to debug the Textual backend. I usually use the pdb debugger, but Textual completely hijacks the terminal window so that doesn't work at all. Any tips to overcome that?

I've been using two different methods.

Write to a log file on the App object:

class HelloWorld(toga.App):
    file = open(Path.home() / "toga_app_log.txt", "w")

    @classmethod
    def log(cls, msg: str):
        cls.file.write(f"{msg}\n")
        cls.file.flush()

Or add headless=true when running the app in main_loop().

[freakboy3742]

I would like to give this a go! If I understand the codebase correctly the change should be made here, to check if the interface.content is a valid Widget:

Not quite. The issue is that right now, this is possible:

def startup(self):
    self.my_window = toga.Window("Window Title")
    self.my_window.show()

That's legal, but the Window has no content (i.e., self.my_window.content is None). That's not a huge problem for most backends, but it is for Textual.

What we need to do is ensure that at time of construction, the initial value of self.my_window.content always has a value, even if it's a freshly created instance of toga.Box() (i.e., an empty container). That's effectively what the None case ends up rendering anyway; we could fix this as a specific case for Textual, but we might as well fix it for all platforms, and remove the possibility that content is ever None.

Sidenote: It's kind of hard to debug the Textual backend. I usually use the pdb debugger, but Textual completely hijacks the terminal window so that doesn't work at all. Any tips to overcome that?

Textual has some developer tools that involves running a second terminal that gives you access to the "console" that you'd normally see when running an app; see https://textual.textualize.io/guide/devtools/ for details. Run your Toga app with those tools, and it should be a lot easier to debug.

[cgkoutzigiannis]

So, I should instead make the change on the Window constructor on toga-core, right?

[freakboy3742]

So, I should instead make the change on the Window constructor on toga-core, right?

Correct - along with updates to the tests in the toga-core package to validate the change (I suspect you'll find there's a test that will fail when you add a default widget).

Also - make sure you have a read of the contribution guide (if you haven't already) - it's got lots of details of how to set up your development environment, and the other details you'll need to have in place in any pull request you submit.

[cgkoutzigiannis]

I can't find a way to provide a default value (an empty toga.Box()) to the content constructor argument, without causing circular import errors. I could leave None as a default value and make sure that we change that to an empty Box, but that doesn't feel right.

[freakboy3742]

@cgkoutzigiannis Using None as the default value is unambiguously the right approach in this case. Here's the reason why:

def my_func(value=[]):
    value.append("bing")
    print(value)

my_func()
my_func()

The output of the first call to my_func() is:

['bing']

But the output of the second call to my_func() is:

['bing', 'bing']

And a third call would be

['bing', 'bing', 'bing']

That is - the default value isn't instantiated on each call to the method - it's instantiated once, when the method is parsed. This doesn't matter when the object is a primitive like an integer or None, or an immutable object like a string; but when it's an object that can have state (like a list, dictionary... or a toga.Box) it's a problem.

If you use toga.Box() as the default value, aside from any circular import issues, you'll also be creating a single box instance that Python will try to use on every Window. Which is find as long as you have one window... but as soon as you have 2 empty windows, you have a problem.

So - using None as the default value, and then creating an instance of toga.Box() when a value of None is observed is absolutely the right approach.

It has the additional benefit that toga.Window(content=None) also works - an explicit None will be converted into an actual empty widget.

[jgao8]

I'm currently working on this.

[mhsmith]

there's some edge cases where it would be helpful to guarantee that window.content is always a valid widget, even if it's just an empty toga.Box().

@jgao8 made a good attempt at this in #3478, but it revealed some complications which are discussed there. So it might be better to fix the original Textual issue with one or more None checks in the Textual backend, such as already exist in the other backends.