Allow plugin functions in plugin_group!#24345
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨ |
Jengamon
force-pushed
the
allow-plugin-func-in-group
branch
from
4cb37a0 to
9d8b233
Compare
introduces a little idiosyncratic `fn {}` block for containing the
function members, mostly as a workaround for the fact that macro parsing
cannot backtrack
Jengamon
force-pushed
the
allow-plugin-func-in-group
branch
from
9d8b233 to
30f8623
Compare
kfc35
added
A-App
NiklasEi
left a comment
NiklasEi
left a comment
There was a problem hiding this comment.
Please also extend the existing tests with fn plugins (hidden and unhidden). Make sure to check the build order after fixing it.
| group = group.add(<$($plugin_path::)*$plugin_name>::default()); | ||
| } | ||
| )* | ||
| $($( |
There was a problem hiding this comment.
With these changes, the order of the plugins in the expanded macro is no longer the same order they have in the macro block. Since plugin order matters for dependencies (e.g. on Resource), the order should stay consistent.
| " - [`", stringify!($plugin_group_name), "`](" $(, stringify!($plugin_group_path), "::")*, stringify!($plugin_group_name), ")" | ||
| $(, " - with feature `", $plugin_group_feature, "`")? | ||
| )])+)? | ||
| $( |
There was a problem hiding this comment.
We should also add the documentation of the fn plugins here.
| /// #[doc(hidden)] | ||
| /// internal:::InternalPlugin | ||
| /// internal:::InternalPlugin, | ||
| /// // The last thing within the block can be a "fn block", which allows using plugin functions |
There was a problem hiding this comment.
The last part of this sentence reads strange.
alice-i-cecile
added
X-Uncontroversial
4 participants
Objective
Allow plugin functions to be used inside
plugin_group!declarations.Right now, functions of signature
fn(&mut App)implementPlugin, but cannot be added to plugin groups, dueto how the macro is implemented.
This fixes that.
Solution
This PR fixes the problem by introducing a block after the top-level members, labeled with
fn, where the names are considered to be referencing values directly, rather than types. It looks like:Testing
I edited the doctest above the macro implementation, and made sure that passes, and the syntax is non-invasive otherwise, so overall, not much should change.