-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Remove extra braces in html_nested macro #2169
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
Remove extra braces in html_nested macro #2169
Conversation
|
@hamza1311 sorry for the repeated effort in #2167 , I initially marked the issue #2157 as |
| # test target can be optionally specified like `cargo make test html_macro`, | ||
| args = ["test", "${@}"] |
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 find this useful in development
|
|
||
| tokens.extend(quote_spanned! {ty.span()=> | ||
| { | ||
| #[allow(clippy::unit_arg)] |
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.
Previously the now __yew_props was inlined in the new call. When there are no properties (e.g. html_nested!{<MyComponent/>}), #build_props would return MyComponent::Properties::builder().build() which is the unit type, triggering the unit arg warning.
Extracting a variable solves that.
|
Can you also remove the |
| // can test "unused braces" warning inside the macro | ||
| // https://github.com/yewstack/yew/issues/2157 | ||
| fn make_my_component()-> ::yew::virtual_dom::VChild<MyComponent>{ | ||
| ::yew::html_nested!{<MyComponent/>} | ||
| } | ||
|
|
||
| // can test "unused braces" warning inside the macro | ||
| // https://github.com/yewstack/yew/issues/2157 | ||
| fn make_my_component_html()-> ::yew::Html{ | ||
| ::yew::html!{<MyComponent/>} | ||
| } |
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.
test for both html and html_nested
| fn to_tokens(&self, tokens: &mut TokenStream) { | ||
| let new_tokens = self.0.to_token_stream(); | ||
| tokens.extend(quote! {{ | ||
| #[allow(clippy::useless_conversion, unused_braces)] |
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.
this part is for the html! macro, it's no longer needed.
| } | ||
| } | ||
| TagName::Expr(name) => { | ||
| #[allow(unused_braces)] |
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.
Small correction in the html! marco. This is most likely misplaced. It does nothing here. No wonder, cuz the previous paragraphs repeated this pattern, this must have been copy-pasted here.
| #[allow(unused_braces)] | ||
| // e.g. html!{<@{"div"}/>} will set `#expr` to `{"div"}` | ||
| // (note the extra braces). Hence the need for the `allow`. | ||
| // Anyways to remove the braces? | ||
| let mut #vtag_name = ::std::convert::Into::< | ||
| ::std::borrow::Cow::<'static, ::std::primitive::str> | ||
| >::into(#expr); |
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.
This is where the misplaced #[allow] really belongs. Maybe the allow here is also liftable? I'm however too inexperienced with proc macro to figure it out. So I just left some comments instead
@hamza1311 done now :D |
voidpumpkin
left a comment
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.
Seems fine to me
mc1098
left a comment
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.
Seems more than fine to me 😏
* removed unused braces from html_nested marco * allow specifying test name * also fix for html! macro * also fix for html! macro * remove misplaced #[allow(unused_braces)] (cherry picked from commit 4c3d693)
Description
Refactored the
html_nestedmacro to avoid extra bracesFixes #2157
Checklist
cargo make pr-flow