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

[RTL] Add pop_all, push_context and pop_context methods, and use it for print_rich to avoid unclosed tags. #79011

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jul 4, 2023

Fixes #78520

Before After
Screenshot 2023-07-04 at 09 50 44 Screenshot 2023-07-04 at 09 51 06

@MewPurPur
Copy link
Contributor

Are there any benefits to doing this other than less code? Just curious if I should still do things the old way unless necessary.

@bruvzg
Copy link
Member Author

bruvzg commented Jul 4, 2023

Are there any benefits to doing this other than less code?

pop_all should be faster, but in real use cases it probably won't have any visible effect. The only real advantage, there's no need to count push_ calls to match with the same number of pops.

@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Jul 7, 2023
@YuriSizov
Copy link
Contributor

The only real advantage, there's no need to count push_ calls to match with the same number of pops.

I believe in our built-in documentation we have plenty of cases where we need to pop N levels from the stack, but not all of it. Would something like a scope or a context make sense as well?

push_context(); // Start counting pushes.
push_...();
push_...();
push_...();
push_...();
pop_context(); // Pops 4 stack levels and removes context.

…e it for `print_rich` to avoid unclosed tags.
@bruvzg bruvzg changed the title [RTL] Add pop_all method, and use it for print_rich to avoid unclosed tags. [RTL] Add pop_all, push_context and pop_context methods, and use it for print_rich to avoid unclosed tags. Jul 9, 2023
@bruvzg
Copy link
Member Author

bruvzg commented Jul 9, 2023

Added push_context / pop_context as well.

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Lovin' it!

@YuriSizov YuriSizov merged commit 5c56206 into godotengine:master Jul 14, 2023
@YuriSizov
Copy link
Contributor

Thanks! Now someone could do a pass simplifying calls in the built-in help :)

@MewPurPur
Copy link
Contributor

Code simplification with new methods? I'm in.

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.

Printing with print_rich a lot of colored text breaks formating after output overflow.
4 participants