Skip to content

Conversation

@ryanbalsdon
Copy link

This patch is modifying the html and declare_elements macros to use proc_macro2 everywhere and make it a bit easier to inspect what they're doing. It also adds a few tests against those new implementations to make it even easier to see what they output!

This patch also adds a bunch of tests to the tests/main.rs file to cover some less-documented features that took me some time to figure out. Hopefully this will help the next person who tries to learn this code base (or wants to write some tests against a new feature?).

Some more notes about corner cases I ran into if someone is interested in reviewing this:

  • token.span().unstable() downgrades to proc_macro2::Span into a proc_macro::Span. I didn't see an impact to output by removing this but there might be something untested/undocumented that this was important for.
  • The rust_format dependency was to run rustfmt on the output string because it comes out very messy! Had to switch to PrettyPlease instead because rustfmt ignored some code here and there.
  • The pretty_assertions dependency was because a failing assert_eq on an 800-line file was tough to read!
  • The reason events require the explicit ": String" thing is probably because of some kind of wasm implementation? It's probably straightforward to add String as a default. I might try that if this merges! Not confident in trying it without tests...
  • Wrapping the html output in "fn html() " was to get PrettyPlease to accept the code. It's expecting a complete file and this was the easiest way I could find to accept the code block I wanted cleaned.

@ryanbalsdon
Copy link
Author

Resolved the clippy and fmt issues but I think the Windows tests are complaining about newline formats. To do something clever about that means setting up a Windows machine to test on, which will take me another few days to get to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant