textual icon indicating copy to clipboard operation
textual copied to clipboard

Prevent adding self as a child of self

Open MatrixManAtYrService opened this issue 3 years ago • 2 comments

First of all, thanks for working on Textual, I'm excited to use it to make cool stuff.

Here's some code that works:

from textual.app import App, ComposeResult
from textual.widget import Widget

class Somewidget(Widget):
    pass

class MyApp(App):
    def compose(self) -> ComposeResult:
        yield Somewidget()

Running textual console in one terminal and textual run --dev src/fluoresce/bug.py in another causes this to appear in the console:

[12:15:52] SYSTEM                                                                   app.py:998
Connected to devtools ( ws://127.0.0.1:8081 )
[12:15:52] SYSTEM                                                                  app.py:1002
---
[12:15:52] SYSTEM                                                                  app.py:1004
driver=<class 'textual.drivers.linux_driver.LinuxDriver'>
[12:15:52] SYSTEM                                                                  app.py:1005
loop=<_UnixSelectorEventLoop running=True closed=False debug=False>
[12:15:52] SYSTEM                                                                  app.py:1006
features=frozenset({'devtools', 'debug'})
[12:15:53] EVENT                                                           message_pump.py:428
Resize(size=Size(width=119, height=103), virtual_size=Size(width=119, height=103)) >>>
MyApp(title='MyApp') method=<App.on_resize>

And pressing Ctrl+C closes the app and returns the user to the terminal.

But if you change it like so:

from textual.app import App, ComposeResult
from textual.widget import Widget

class Somewidget(Widget):
    def __init__(self):
       super().__init__(self)

class MyApp(App):
    def compose(self) -> ComposeResult:
        yield Somewidget()

Then something gets stuck before the console gets as far as it did before:

[12:21:14] SYSTEM                                                                   app.py:998
Connected to devtools ( ws://127.0.0.1:8081 )
[12:21:14] SYSTEM                                                                  app.py:1002
---
[12:21:14] SYSTEM                                                                  app.py:1004
driver=<class 'textual.drivers.linux_driver.LinuxDriver'>
[12:21:14] SYSTEM                                                                  app.py:1005
loop=<_UnixSelectorEventLoop running=True closed=False debug=False>
[12:21:14] SYSTEM                                                                  app.py:1006
features=frozenset({'debug', 'devtools'})

In this stuck state, Ctrl+C is ignored by the app, so I have to kill -2 {the pid} in order to reclaim my terminal.

I recognize that I might be subclassing Widget improperly (I'll figure that out soon). But even if so, it would be cool if this failed in a way that was:

  • more verbose about the problem
  • more easily recovered from

MatrixManAtYrService avatar Oct 30 '22 18:10 MatrixManAtYrService

The problem is this line:

       super().__init__(self)

You don't need the self in the arguments. It should be:

       super().__init__()

The __init__ accepts its child widgets in as positional argument. You are essentially making a widget a child of itself, probably causing an infinite loop.

While we may be able to detect this one. I'm afraid we can't protect you against all programming errors!

willmcgugan avatar Oct 30 '22 19:10 willmcgugan

Oh that should've been obvious. Thanks for the clarity!

I won't shed any tears if you close this--but I'll leave it for you to decide because detecting this condition and giving the user a hint would be pretty cool.

MatrixManAtYrService avatar Oct 30 '22 19:10 MatrixManAtYrService

A catch for that situation will be in 0.5.0.

davep avatar Nov 14 '22 09:11 davep

Did we solve your problem?

Glad we could help!

github-actions[bot] avatar Nov 14 '22 09:11 github-actions[bot]