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

Report a bug about switch_screen #2650

Closed
tsingwang opened this issue May 25, 2023 · 8 comments · Fixed by #2692
Closed

Report a bug about switch_screen #2650

tsingwang opened this issue May 25, 2023 · 8 comments · Fixed by #2692
Assignees
Labels
bug Something isn't working Task

Comments

@tsingwang
Copy link

from textual.app import App, ComposeResult
from textual.screen import Screen
from textual.widgets import Static, Header, Footer

class ScreenA(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Static("A")
        yield Footer()

class ScreenB(Screen):
    def compose(self) -> ComposeResult:
        yield Header()
        yield Static("B")
        yield Footer()

class ModalApp(App):
    SCREENS = {
        "a": ScreenA(),
        "b": ScreenB(),
    }

    BINDINGS = [
        ("1", "switch_screen('a')", "A"),
        ("2", "switch_screen('b')", "B"),
    ]

    def on_mount(self) -> None:
        self.push_screen("a")


if __name__ == "__main__":
    app = ModalApp()
    app.run()

This is sample code. When '21' pressed, it will crash. Log below:

╭──────────────────────────────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────────────────────────────╮
│ /home/tsing/.local/lib/python3.10/site-packages/textual/app.py:2488 in action_switch_screen                                                                                │
│                                                                                                                                                                            │
│   2485 │   │   Args:                                                                            ╭────────────────────────── locals ───────────────────────────╮            │
│   2486 │   │   │   screen: Name of the screen.                                                  │ screen = 'a'                                                │            │
│   2487 │   │   """                                                                              │   self = ModalApp(title='ModalApp', classes={'-dark-mode'}) │            │
│ ❱ 2488 │   │   self.switch_screen(screen)                                                       ╰─────────────────────────────────────────────────────────────╯            │
│   2489 │                                                                                                                                                                   │
│   2490 │   async def action_push_screen(self, screen: str) -> None:                                                                                                        │
│   2491 │   │   """An [action](/guide/actions) to push a new screen on to the stack and make it                                                                             │
│                                                                                                                                                                            │
│ /home/tsing/.local/lib/python3.10/site-packages/textual/app.py:1446 in switch_screen                                                                                       │
│                                                                                                                                                                            │
│   1443 │   │   │   )                                                                            ╭─────────────────────────────── locals ───────────────────────────────╮   │
│   1444 │   │   if self.screen is not screen:                                                    │ previous_screen = ScreenB(pseudo_classes={'enabled'})                │   │
│   1445 │   │   │   previous_screen = self._replace_screen(self._screen_stack.pop())             │          screen = 'a'                                                │   │
│ ❱ 1446 │   │   │   previous_screen._pop_result_callback()                                       │            self = ModalApp(title='ModalApp', classes={'-dark-mode'}) │   │
│   1447 │   │   │   next_screen, await_mount = self._get_screen(screen)                          ╰──────────────────────────────────────────────────────────────────────╯   │
│   1448 │   │   │   self._screen_stack.append(next_screen)                                                                                                                  │
│   1449 │   │   │   self.screen.post_message(events.ScreenResume())                                                                                                         │
│                                                                                                                                                                            │
│ /home/tsing/.local/lib/python3.10/site-packages/textual/screen.py:572 in _pop_result_callback                                                                              │
│                                                                                                                                                                            │
│   569 │                                                                                        ╭────────────────── locals ──────────────────╮                              │
│   570 │   def _pop_result_callback(self) -> None:                                              │ self = ScreenB(pseudo_classes={'enabled'}) │                              │
│   571 │   │   """Remove the latest result callback from the stack."""                          ╰────────────────────────────────────────────╯                              │
│ ❱ 572 │   │   self._result_callbacks.pop()                                                                                                                                 │
│   573 │                                                                                                                                                                    │
│   574 │   def _refresh_layout(                                                                                                                                             │
│   575 │   │   self, size: Size | None = None, full: bool = False, scroll: bool = False                                                                                     │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
IndexError: pop from empty list

The direct reason is _ push_ result_ callback not called when switch_screen, or check self._result_callbacks is empty when pop.

@github-actions
Copy link

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

@chitter99
Copy link

Same thing happened to me. I'm switching a screen that another screen has a callback on.

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this May 30, 2023
@rodrigogiraoserrao rodrigogiraoserrao added bug Something isn't working Task labels May 30, 2023
@rodrigogiraoserrao
Copy link
Contributor

Thanks for opening this issue @tsingwang.

@chitter99 can you add a bit more context to your specific problem, please? Ideally with a minimal reproducible example?
It's likely that the issues are related but a code sample from you could be quite helpful.

@rodrigogiraoserrao
Copy link
Contributor

Studying this a bit further revealed that there was another similar issue because switching to the currently active screen should be a no-op but it wasn't.

@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@mzebrak
Copy link

mzebrak commented Jul 28, 2023

@rodrigogiraoserrao It looks like a similar issue still exists, but now for pop_screen. One of our users sent us:
(Textual version of 0.30.0)

/lib/python3.10/site-packages/textual/app.py:1732 in                                                                               │
│ pop_screen                                                                                                                       │
│                                                                                                                                  │
│   1729 │   │   │   │   "Can't pop screen; there must be at least one screen on the stack"                                        │
│   1730 │   │   │   )                                                                                                             │
│   1731 │   │   previous_screen = self._replace_screen(screen_stack.pop())                                                        │
│ ❱ 1732 │   │   previous_screen._pop_result_callback()                                                                            │
│   1733 │   │   self.screen._screen_resized(self.size)                                                                            │
│   1734 │   │   self.screen.post_message(events.ScreenResume())                                                                   │
│   1735 │   │   self.log.system(f"{self.screen} is active")                                                                       │
│                                                                                                                                  │
│                                                                                                                                  │
│/lib/python3.10/site-packages/textual/screen.py:635 in                                                                            │
│ _pop_result_callback                                                                                                             │
│                                                                                                                                  │
│   632 │                                                                                        ╭─── locals ────╮                 │
│   633 │   def _pop_result_callback(self) -> None:                                              │ self = Cart() │                 │
│   634 │   │   """Remove the latest result callback from the stack."""                          ╰───────────────╯                 │
│ ❱ 635 │   │   self._result_callbacks.pop()                                                                                       │
│   636 │                                                                                                                          │
│   637 │   def _refresh_layout(                                                                                                   │
│   638 │   │   self, size: Size | None = None, full: bool = False, scroll: bool = False                                           │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
IndexError: pop from empty list

@davep
Copy link
Contributor

davep commented Jul 28, 2023

@mzebrak Do you think you could create a minimal isolated example of what your code is doing, such that it displays the issue? Would be an idea to create it as a fresh issue if this is something that's a bug in the current release; feel free to refer back to this one too.

@mzebrak
Copy link

mzebrak commented Jul 31, 2023

@mzebrak Do you think you could create a minimal isolated example of what your code is doing, such that it displays the issue? Would be an idea to create it as a fresh issue if this is something that's a bug in the current release; feel free to refer back to this one too.

Unfortunately, I have no idea how to reproduce it, that's all I have. I suppose some sequence of several pop screens, switch screens and pop screens again

==== EDIT:

I've got it!
I think it's related to switch_screen call with screen as str and not Screen instance
My logging includes with screen as string postfix when push_screen or switch_screen is called with screen as str
Here's the sequence:

2023-07-31 12:43:03.893 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:03.895 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:04.438 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:05.852 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:08.068 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:09.302 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:11.912 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:13.077 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:14.283 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.301 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.315 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.330 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.341 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.350 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.353 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:14.422 | ⚠️ WARNING  | Textual issue with _pop_result_callback: switch_screen called with screen as string
2023-07-31 12:43:16.090 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:19.593 | ⚠️ WARNING  | Textual issue with _pop_result_callback: push_screen called 
2023-07-31 12:43:31.498 | ⚠️ WARNING  | Textual issue with _pop_result_callback: switch_screen called 
2023-07-31 12:43:34.968 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called
2023-07-31 12:43:35.002 | ⚠️ WARNING  | Textual issue with _pop_result_callback: pop_screen called

==== EDIT:

I've created a separate issue for this: #3041

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants