-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add NodeName::Block #11
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.
Thanks for working on this, looks good!
Would be nice if you could document that feature in the README as well (top of the arbitrary rust code block list).
Rebasing on main should also fix the bench CI check |
48d8200
to
4db8ff9
Compare
Codecov Report
@@ Coverage Diff @@
## main #11 +/- ##
==========================================
- Coverage 82.91% 82.73% -0.19%
==========================================
Files 4 4
Lines 474 498 +24
==========================================
+ Hits 393 412 +19
- Misses 81 86 +5
Continue to review full report at Codecov.
|
Sorry for the mess, Git is definitely not my strength; but I think this fixes those conflicts. |
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.
No worries, looks good now, thanks!
@@ -144,6 +153,9 @@ pub enum NodeName { | |||
|
|||
/// Name separated by colons, e.g. `<div on:click={foo} />` | |||
Colon(Punctuated<Ident, Colon>), | |||
|
|||
/// Arbitrary rust code in braced `{}` blocks | |||
Block(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.
Just realized, is there a specific reason that this is Expr
and not ExprBlock
?
It was startling that it was so easy to do this, but it passes tests, and actually works as I was hoping it would work in the library I'm building.
Open question how
NodeName::Block
shouldimpl Display
but it returnsNone
for.name_as_string()
so maybe it's not a big deal one way or the other.