Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: use syn-file-expand instead of cargo and add Wit type #25

Merged
merged 9 commits into from
Apr 9, 2022
Merged

feat: use syn-file-expand instead of cargo and add Wit type #25

merged 9 commits into from
Apr 9, 2022

Conversation

willemneal
Copy link
Collaborator

fixes #23

And add new Wit type and refactor to use Display and remove old temporary file logic.

@willemneal willemneal marked this pull request as draft April 7, 2022 22:24
bnjjj
bnjjj reviewed Apr 8, 2022
View reviewed changes
Copy link
Owner

@bnjjj bnjjj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great and very promising ! If you need help let me know !

crates/witgen_macro_helper/src/wit.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/wit.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/wit.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/wit.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/generator.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/wit.rs Show resolved Hide resolved
@willemneal
feat: use syn-file-expand to gather all project into one file
b8d928f 
Also add new `Wit` type and refactor to use Display and remove old temporary file logic.
@willemneal willemneal marked this pull request as ready for review April 8, 2022 18:36
@willemneal
Copy link
Collaborator Author

@bnjjj Ready for a review!

I think this is enough for this PR. I think that the next PR should switch to generating the official wit AST, which then can be turned into a string. This way we can then do some semantic analysis to pull in dependencies as needed.

But this is already a big step forward. Adding the From traits made it super clean! And now the output order is deterministic!

@bnjjj
Copy link
Owner

bnjjj commented Apr 9, 2022

Will take a look when I'm on my computer but definitely when I started this project I knew that in the future we had to use the AST. We could use this one https://github.com/bytecodealliance/wit-bindgen/blob/main/crates/parser/src/lib.rs and develop an encoder over it

bnjjj
bnjjj reviewed Apr 9, 2022
View reviewed changes
Copy link
Owner

@bnjjj bnjjj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good overall ! Just some nits and we are ready to merge ! Could you add an entry in the changelog and bump the version of each crates please ? It will be faster to release

crates/witgen_macro_helper/src/lib.rs Outdated Show resolved Hide resolved
crates/witgen_macro_helper/src/wit.rs Show resolved Hide resolved
examples/my_witgen_example/index.wit Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
crates/cargo_witgen/src/app.rs Outdated Show resolved Hide resolved
crates/cargo_witgen/src/app.rs Outdated Show resolved Hide resolved
@willemneal
Copy link
Collaborator Author

@bnjjj Ready for final review! Thanks for the help!

bnjjj
bnjjj approved these changes Apr 9, 2022
View reviewed changes
crates/witgen_macro_helper/Cargo.toml Outdated Show resolved Hide resolved
@bnjjj bnjjj merged commit e10b3f9 into bnjjj:master Apr 9, 2022
@bnjjj
Copy link
Owner

bnjjj commented Apr 9, 2022

Will make a new release on Monday ! Feel free to ping me if I forgot or if you need it before

willemneal added a commit to AhaLabs/witgen that referenced this pull request Apr 10, 2022
@willemneal
feat: use syn-file-expand instead of cargo and add Wit type (bnjjj#25)
bcd8dc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnjjj bnjjj bnjjj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: use cargo to get all source files used in project
2 participants
@willemneal @bnjjj