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

Resurrect integrated error display for the Debugger. #21884

Merged
merged 1 commit into from
Sep 12, 2018

Conversation

RyanStein
Copy link
Contributor

@RyanStein RyanStein commented Sep 8, 2018

I have taken a look at how the editor has changed since I originally made this feature(#15272) and found it simpler to just re-write on top of the changes made since. I was able to integrate the right-click error copy as well. No rebase should be necessary.

I made use of @akien-mga 's suggested changes from the branch indicated. We should finally be able to close #15234 and salvage this.

image

I would appreciate any feedback. Thank you.

Edit: Also fixes #21676

@Chaosus Chaosus added this to the 3.1 milestone Sep 9, 2018
@groud
Copy link
Member

groud commented Sep 9, 2018

Nice, I am very happy you managed to rebase those changes. Thanks for your hard work!

@Chaosus
Copy link
Member

Chaosus commented Sep 9, 2018

I guess this will be fixed too - #21676. Good job !

@RyanStein
Copy link
Contributor Author

@Chaosus Absolutely. You may click on either the warning itself or the <Source> item to jump immediately to the line in question. The same goes for Errors where applicable, such as each individual stack trace item.

@akien-mga akien-mga requested a review from reduz September 10, 2018 21:17
@akien-mga
Copy link
Member

akien-mga commented Sep 12, 2018

Thanks for the contribution, it looks great!

We briefly discussed it on IRC and had some suggestions/concerns, but those can be worked on in follow-up PRs, namely:

@akien-mga akien-mga merged commit c4311b6 into godotengine:master Sep 12, 2018
@ghost
Copy link

ghost commented Sep 13, 2018

this is very cool, but i noticed something odd:

previously, if we scrolled all the way down on the scrollbar in the error log, any new errors that would show, would make the scroll bar go down to the bottom automatically.

now, it just stays wherever you left the scrollbar at, and then now you need to scroll down to get to the last error message.

akien-mga added a commit to akien-mga/godot that referenced this pull request Sep 25, 2019
This was removed by @RyanStein in godotengine#21884 in the case where an error
message is provided, but this is actually useful information to have
even when there is a custom error message.

This PR makes it so that the "C++ Error" is shown whenever there is
a custom error message provided.

Also adds method name to the error item title, and re-adds the most
relevant info in the tooltip for quick error checks without expanding.

Renames C Error/Source to C++ Error/Source, since that's what it is.
And fix untranslatable entry due to misuse of TTR().

And some more cleanup for readability.

Cf. godotengine#32276 (comment)
pchasco pushed a commit to pchasco/godot that referenced this pull request Oct 23, 2019
This was removed by @RyanStein in godotengine#21884 in the case where an error
message is provided, but this is actually useful information to have
even when there is a custom error message.

This PR makes it so that the "C++ Error" is shown whenever there is
a custom error message provided.

Also adds method name to the error item title, and re-adds the most
relevant info in the tooltip for quick error checks without expanding.

Renames C Error/Source to C++ Error/Source, since that's what it is.
And fix untranslatable entry due to misuse of TTR().

And some more cleanup for readability.

Cf. godotengine#32276 (comment)
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.

[UX] Please let me double click warnings Long error messages are displayed outside screen in console
5 participants