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

text-minimessage: Use a Consumer rather than Appendable for debug output #671

Merged
merged 2 commits into from Feb 5, 2022

Conversation

zml2008
Copy link
Member

@zml2008 zml2008 commented Feb 4, 2022

Fixes GH-667

@gepron1x
Copy link

gepron1x commented Feb 4, 2022

May i ask, why do you use null if debug is disabled? Why not just empty consumer?

@gepron1x
Copy link

gepron1x commented Feb 4, 2022

Also, consider adding convenience method to builder that accepts Appendable and sneaky throws IOException?

@kezz
Copy link
Member

kezz commented Feb 4, 2022

May i ask, why do you use null if debug is disabled? Why not just empty consumer?

There are some non-zero cost calculations/string formatting that takes place that doesn't need to happen if nobody is caring about the logging.

Also, consider adding convenience method to builder that accepts Appendable and sneaky throws IOException?

I'm not massively interested in adding a convenience method for Appendable, it's a bit of a rubbish class to use and considering how easy it is to wrap in a consumer I don't think we really need to add/encourage use of such a method.

@zml2008 zml2008 requested a review from kashike February 5, 2022 05:59
@zml2008 zml2008 self-assigned this Feb 5, 2022
@zml2008 zml2008 merged commit 893e46f into main/4 Feb 5, 2022
@zml2008 zml2008 deleted the feature/no-longer-appendable branch February 5, 2022 09:05
This was referenced Feb 28, 2022
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.

Remove Appendable usage
4 participants