Finalise procedural macros

futursolo commented 3 years ago

There're a couple issues that I wish to address before finalising the procedural macros:

[x] Move string interpolation parsing to a separate parser
[x] RemoveError::Interpolation
[x] Remove StringKind
[x] Parser does not handle quoted strings properly (content: ";";)
[x] \${} or $${} escaping on ${} sequence
[x] once_cell::sync::Lazy'ed SheetRef for styles with no string interpolation
[x] Return Result<T> for Style and GlobalStyle
[x] Come up a better name for IntoStyle(StyleSource) and SheetRef(Merge with Sheet)
[x] Move parser back into core
[x] Merge IntoSheet with StyleSource

@WorldSEnder I would appreciate your feedback on the procedural macros. Please let me know if you face any issues.

WorldSEnder commented 3 years ago

* (A separate parser will go through all selectors and values to make replace interpolated values after AST is generated. This limits string interpolation to procedural macros, removes the `Error::Interpolation` variant and solves the complication and performance degradation caused by interpolation parsing.)

On that note, I don't think StringKind::Interpolation should show up in the ast, at all, as it is parser specific. If you abstract the parser a bit, the second variant that's called from the proc macro would not impact the runtime parser with additional variant matching.

* `\${}` or `$${}` escaping on `${}` sequence

\$ would be bash-like, which I was going with ${} anyway.

I'm currently rewiring my PR to the upstream changes, and in a way that should make changes to the ast in the future easier to handle.

About the style and global_style macros: they are currently swallowing errors, I think they should be purposely of type Result<..> not unwrap that error by default. Let the user unwrap or simply ? the expanded macro.

A little bit less of a problem with the impls on IntoStyle, but swallowing errors there should probably get a workaround and a emphasis in the documentation.

futursolo commented 3 years ago

On that note, I don't think StringKind::Interpolation should show up in the ast, at all, as it is parser specific. If you abstract the parser a bit, the second variant that's called from the proc macro would not impact the runtime parser with additional variant matching.

That will also be removed when the first item is addressed.

I'm currently rewiring my PR to the upstream changes, and in a way that should make changes to the ast in the future easier to handle.

I will make changes to the AST as well due to the issue mentioned above. If you don't want to rebase again (in which I apologise), you may wish to hold off the PR until I addressed the first item in the checklist.

About the style and global_style macros: they are currently swallowing errors, I think they should be purposely of type Result<..> not unwrap that error by default. Let the user unwrap or simply ? the expanded macro.

I will address this. These macros were added before css! and that unwrap is added so I don't have to .unwrap() on invocation when used with class= in yew components.

A little bit less of a problem with the impls on IntoStyle, but swallowing errors there should probably get a workaround and a emphasis in the documentation.

I think there's a balance between ergonomic and correctness. css! needs to unwrap to make it ergonomic when used with class=. There's no good way to ? it at the moment as yew does not want to return Result<Html> and manual unwrapping on every css! invocation will make the API very difficult to use.

Just like I know that the .to_string() allocation is the slowest part of the parser. I still don't want to remove them and carry a 'a lifetime in the style.

futursolo / stylist-rs

Finalise procedural macros #24