Textual console showing wrong path #3237
Comments
|
On initial look at this, I don't think the issue is as broad as is suggested here. Given this code: from textual import on
from textual.app import App, ComposeResult
from textual.widgets import Button
class LogLocationApp(App[None]):
def compose(self) -> ComposeResult:
yield Button("Print", id="print")
yield Button("Log", id="log")
@on(Button.Pressed, "#print")
def print_to_console(self) -> None:
print("This was from a print")
@on(Button.Pressed, "#log")
def log_to_console(self) -> None:
self.log.debug("This was from a log")
if __name__ == "__main__":
LogLocationApp().run()I see a variety of locations that mostly look okay:
Presumably the thing that's the issue here is when something is logged to the console via |
|
Looks like the change was brought in with the capture print facility: 4406b9d |
Fixes Textualize/textual#3237 I'm undecided about this. It doesn't feel quite right to special-case this, but on the other hand `print` is a form of logging that seems to work differently in Textual vs the normal logging facility (which seems to do its own dance to handle the call location, before it makes it to devtools). This does correct the issue, this does ensure that `print` reports the location the same as any `self.log` call. So this is a commit made to solicit more detail on the design decisions in the core of Textual.
This time, don't special-case the print down in the StdoutRedirector, but instead allow the caller to pass in the frame we're supposed to work from. Note that with this change it *won't* fix the problem that this seeks to fix, but it will allow for a change in Textual that will fix the problem. As such this code should be merged and released *before* such code is merged into Textual. Meanwhile, this code should be safe to release first as it's designed to be backward-compatible. See Textualize/textual#3237
|
@davep What's the resolution here? It's in For Review, but no linked PR. |
|
The description of the PR linked just above (#3294) has the explanation. I believe @darrenburns reviewed the |
|
Can you please make that happen, and ping me when you need something deployed. |
|
Don't forget to star the repository! Follow @textualizeio for Textual updates. |
|
Reopening for a moment (GitHub helpfully closed it when I merged the PR on @willmcgugan |
|
Don't forget to star the repository! Follow @textualizeio for Textual updates. |
The
textual consolecommand is showing the wrong path.IIRC there is an offset in the stack which it uses to find the caller. We may have introduced another level in the stack.
The text was updated successfully, but these errors were encountered: