-
Notifications
You must be signed in to change notification settings - Fork 9
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
Improve macro hygiene #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the idea of not relying on the prelude import, but not sure if it's the best way. Not very knowledgeable in macros, tho 😅
I just saw that bevy_atmosphere
and leafwing-input-manager
use proc_macro_crate, maybe do the same?
There is also bevy_macro_utils::BevyManifest.
What do you think?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5 +/- ##
=======================================
Coverage 90.37% 90.37%
=======================================
Files 28 28
Lines 904 904
=======================================
Hits 817 817
Misses 87 87 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Apparently that has problems with examples: bkchr/proc-macro-crate#14 Also, I don't think this needs something that heavyweight which parses the I see that |
I've changed this to:
|
- Use `accumulation` because macro generates it only if it's set. - Use `Dummy` for conssistency with the rest of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs something that heavyweight which parses the Cargo.toml.
Agree, it's a bit weird.
I see that bevy_reflect uses BevyManifest
It works the exact same way as proc-macro-crate
.
But I think, if a user uses this crate and renames bevy_enhanced_input to something else in the manifest, the derive macro will break.
Yeah, this is what proc-macro-crate
and bevy_macro_utils
trying to solve. And this is why I decided to ask. I personally agree with the current PR state :)
Thanks for looking into it! |
No description provided.