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

Preserve active message pump in call_after_refresh callback #2750

Closed
davep opened this issue Jun 7, 2023 · 5 comments · Fixed by #2806
Closed

Preserve active message pump in call_after_refresh callback #2750

davep opened this issue Jun 7, 2023 · 5 comments · Fixed by #2806
Assignees

Comments

@davep
Copy link
Contributor

davep commented Jun 7, 2023

As of the time of posting this, I'm not sure yet if this is a bug so won't mark it as such; to start with I'm looking to check the how and why of this and to see if it's expected behaviour. The code that follows is a bit involved, but it's designed to illustrate three different scenarios for posting a message and how one of them behaves different from the other two. Consider this code as a playground to test this with, not the primary illustration of the issue. See after the code for some detail.

The Code

from __future__ import annotations

from dataclasses import dataclass

from textual            import on
from textual.app        import App, ComposeResult
from textual.containers import Vertical, Horizontal
from textual.message    import Message
from textual.widget     import Widget
from textual.widgets    import Button, TextLog

class MessageSource( Widget ):

    @dataclass
    class Boop( Message ):
        source: MessageSource

        def control( self ) -> MessageSource:
            return self.source

    def send_now( self ) -> None:
        self.post_message( self.Boop( self ) )

    def send_after_refresh( self ) -> None:
        self.call_after_refresh( self.post_message, self.Boop( self ) )

    def indirect_sender( self ):
        self.post_message( self.Boop( self ) )

    def send_indirectly_after_refresh( self ) -> None:
        self.call_after_refresh( self.indirect_sender )

class ReportingContainer( Vertical ):

    @on( MessageSource.Boop )
    def report( self ) -> None:
        self.app.query_one( TextLog ).write( "Container got boop" )

class DelayedMessageApp( App[ None ] ):

    CSS = """
    Screen > Horizontal > * {
        width: 1fr;
    }

    #buttons {
        height: auto;
    }

    ReportingContainer {
        display: none;
    }
    """

    def compose( self ) -> ComposeResult:
        with Horizontal(id="buttons"):
            yield Button( "Send Now", id="now" )
            yield Button( "Send After Refresh", id="refresh" )
            yield Button( "Send Indirectly After Refresh", id="indirectly" )
        with ReportingContainer():
            yield MessageSource()
        yield TextLog()

    @on( Button.Pressed, "#now" )
    def send_now( self ) -> None:
        self.query_one( TextLog ).write( "-" * 50)
        self.query_one( MessageSource ).send_now()

    @on( Button.Pressed, "#refresh" )
    def send_after_refresh( self ) -> None:
        self.query_one( TextLog ).write( "-" * 50)
        self.query_one( MessageSource ).send_after_refresh()

    @on( Button.Pressed, "#indirectly" )
    def send_indirectly_after_refresh( self ) -> None:
        self.query_one( TextLog ).write( "-" * 50)
        self.query_one( MessageSource ).send_indirectly_after_refresh()

    @on( MessageSource.Boop )
    def report( self ) -> None:
        self.query_one( TextLog ).write( "App got boop" )

if __name__ == "__main__":
    DelayedMessageApp().run()

The Detail

Long story short: while working on #2710 I found myself wanting to perform a post_message as late as possible after some other operation has taken place. In this case I want to let the caller of the code know when some operation has likely finished; the innards of that operation performing actions using call_after_refresh so I don't have a direct way of knowing when that's done.

As a (mostly successful) experiment, I decided to try and delay the sending of the message using the same approach. Initially I had:

self.call_after_refresh( self.post_message, self.TheMessage( self ) )

this worked fine. The message bubbled up to where I expected (from the widget, through any container, and finally up to the App).

I then found I actually wanted to perform a couple of other things before sending the message, but after refresh, so the above changed to something more like:

def thing_do_do_later() -> None:
    self.do_some_thing()
    self.do_another_thing()
    self.post_message( self.TheMessage( self ) )

self.call_after_refresh( thing_to_do_later )

This works too, save for one surprise difference: the message doesn't appear to bubble up to the App.

In the example code pasted above, this is what MessageSource.send_indirectly_after_refresh is illustrating. With the code above there are three buttons: one posts the message right away from the test widget; one posts the message using call_after_refresh; the third posts the message from within a method that is called via call_after_refresh. Only in the final case does it not hit the App:

Screenshot 2023-06-07 at 11 02 51

Like I say, right now, I'm not sure if this is obvious and expected behaviour and I'm missing the obvious; or if it's a bug. Feedback welcome on this. I do think it's a surprising difference though so worth a better understanding of what's going on.

davep added a commit to davep/textual-sandbox that referenced this issue Jun 7, 2023
@davep
Copy link
Contributor Author

davep commented Jun 7, 2023

After a wee bit of digging to understand, it looks like it's down to what active_message_pump.get() evaluates to when the message is created. In the problematic case it's the Screen. In the non-problematic case it's the widget. Point being, if I modify the example code above like this:

diff --git a/delayed_message.py b/delayed_message.py
index a66487f..bbdef84 100644
--- a/delayed_message.py
+++ b/delayed_message.py
@@ -26,11 +26,11 @@ class MessageSource( Widget ):
     def send_after_refresh( self ) -> None:
         self.call_after_refresh( self.post_message, self.Boop( self ) )
 
-    def indirect_sender( self ):
-        self.post_message( self.Boop( self ) )
+    def indirect_sender( self, to_send: MessageSource.Boop ):
+        self.post_message( to_send )
 
     def send_indirectly_after_refresh( self ) -> None:
-        self.call_after_refresh( self.indirect_sender )
+        self.call_after_refresh( self.indirect_sender, self.Boop( self ) )
 
 class ReportingContainer( Vertical ):

it works as would be expected.

Not sure this is a bug in Textual as such, but a surprising outcome given the circumstances aren't too far-fetched so something to be aware of at least.

@davep
Copy link
Contributor Author

davep commented Jun 8, 2023

I think I've convinced myself that this is a case of "well don't do that then". Feels like a bit of an edge case; not that wild that it won't ever happen and could never happen for legitimate reasons, but as I show above it's easy enough to work around and I suspect an actual "fix" for it would be out of proportion to the prevalence of the symptom.

Closing it as such, while making a mental note that it's something to keep an eye out for if anyone runs into similar problems.

@davep davep closed this as completed Jun 8, 2023
@github-actions
Copy link

github-actions bot commented Jun 8, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@davep
Copy link
Contributor Author

davep commented Jun 8, 2023

After a team chat we've decided to reopen this. While the reason for this makes sense once you follow it through, @willmcgugan has said that it would be good to make this work as you'd expect when writing such code. (I think the suggestion was to have call_after_refresh track the current message queue, and then have it be the current message queue for the duration of the call once it happens).

@davep davep reopened this Jun 8, 2023
@darrenburns darrenburns changed the title Apparent inconstant message bubbling when posting a message via an indirect route Apparent inconsistent message bubbling when posting a message via an indirect route Jun 8, 2023
@willmcgugan willmcgugan changed the title Apparent inconsistent message bubbling when posting a message via an indirect route Preserve active message pump in call_after_refresh callback Jun 12, 2023
@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Jun 13, 2023
@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants