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

Add support for print command in local (command line -d) debugger #97218

Merged

Conversation

wenqiangwang
Copy link
Contributor

@wenqiangwang wenqiangwang commented Sep 20, 2024

@wenqiangwang
Copy link
Contributor Author

This is an attempt to reopen PR #73729 after the branch deleted

@AThousandShips AThousandShips added this to the 4.4 milestone Sep 20, 2024
@AThousandShips AThousandShips changed the title Added support for 'print' command in local (command line -d) debugger Add support for print command in local (command line -d) debugger Sep 20, 2024
@wenqiangwang
Copy link
Contributor Author

wenqiangwang commented Sep 20, 2024

Thank for the feedback, everyone. The comments are now resolved.

@wenqiangwang
Copy link
Contributor Author

With the change, we can evaluate expressions in local debugger like in the screenshot below:

Screenshot 2024-09-19 223448

@wenqiangwang
Copy link
Contributor Author

Can you please approve or give feedback?

@wenqiangwang
Copy link
Contributor Author

@akien-mga, at the moment, do you think there is anything I can do to get this change in? Thanks.

@akien-mga
Copy link
Member

You can squash the commits to make the PR ready to merge. But it still requires an approval for a maintainer, which needs some time. I plan to test it myself to confirm the fix, but I'm quite busy, so I can't say when I will do it.

@wenqiangwang wenqiangwang force-pushed the local_debuggger_expr_evalulation branch from acde6be to 9a94353 Compare September 28, 2024 20:28
Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. There are differences between GDScript and Expression, so it would be ideal to implement this separately (parsing and compiling individual GDScript expressions). But since we don't have a foundation for this, it might make sense to use Expression for now. Especially since the recently merged REPL (#97647) also uses Expression.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Sorry for the delay, I was on leave in October.

@wenqiangwang
Copy link
Contributor Author

Thanks for looking, everyone!

@Repiteo Repiteo merged commit 4d43531 into godotengine:master Nov 12, 2024
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 12, 2024

Thanks! Congratulations on your first contribution! 🎉

@akien-mga
Copy link
Member

And thanks for your patience with our slow review process :)

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

Successfully merging this pull request may close these issues.

'print' command in command line debugger does not show anything
5 participants