force parenthesis for elements in the children! macro list #20760
Conversation
| @@ -665,14 +665,14 @@ mod linear_gradient { | |||
| angle: LinearGradient::TO_RIGHT, | |||
| stops: stops.clone(), | |||
| }), | |||
| children
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
alice-i-cecile
added
A-ECS
|
It looks like your PR is a breaking change, but you didn't provide a migration guide. Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes. |
|
I'm not sure if a better error message requires a proc macro, but I feel strongly that we shouldn't do this without very good, clear error messages. With one, I'm weakly in favor of this :) |
| TextSpan::new("IME Active: "), | ||
| TextSpan::new("false\n"), | ||
| TextSpan::new("IME Buffer: "), | ||
| (TextSpan::new("Click to toggle IME. Press return to start a new line.\n\n",)), |
There was a problem hiding this comment.
This choice introduces significant syntactic noise. I personally still think the "parentheses are optional" rule is preferable.
The the intuitive read of this list is correct:
children! [
A,
B,
C,
]Rust trains us to read this as "this is a list of the children A, B, and C".
The grammar is "this is a list of children: if you want a child to have more than one component, add parenthesis". That is not so hard to learn that it justifies the added labor of changing it to "this is a list of children, each child must have parenthesis".
There was a problem hiding this comment.
Wherever this lands, the resulting choice needs to be in sync with BSN
There was a problem hiding this comment.
Mm, I really understand where you're coming from, and if I didn't experience this confusion myself a couple times, I would probably agree with you.
I think the confusion comes from the fact that conceptually bundles are also lists. Given that lists of lists of components are not a common pattern anywhere else, it's super easy to miss the extra nesting. This in itself would again not be so bad if the resulting bugs are usually not immediately obvious and quite difficult to diagnose.
Not to sound too melodramatic, but I fear that a novice making this mistake by accident could be inclined to write Bevy off as unrealible. It really doesnt take long for a new user to run into the children macro, especially when doing ui related stuff.
Finally, I was also never a huge fan of the parenthesis solution, would you maybe have something else in mind?
Objective
children!macro is easy to subtly hold wrong #20429Solution
Testing
Open questions