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.

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

feat: copy inner composite for struct array keys #67

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

MartianGreed
Copy link
Contributor

For dojo.js bindgen we need to add copies of composites inside nested core::array::Array fields.

We might need to do the same for tuple but I need to test with real abi so PR will be completed tomorrow

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Minor reviews, lint and let's move on. 👍

@@ -121,4 +123,68 @@ impl Token {
_ => (),
}
}

pub fn hydrate(token: Self, filtered: &HashMap<String, Token>) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we have a function doc here to keep in mind what was the need of it? Cainome generally needs a bit more doc. :)

Comment on lines +330 to +351
if let Token::Composite(composite_0) = &tokens[0] {
let unique_composite = composite_0.clone();
let inners = composite_0
.inners
.iter()
.map(|inner| {
let inner_tokens = tokens
.iter()
.filter_map(|__t| {
__t.to_composite().ok().and_then(|comp| {
comp.inners
.iter()
.find(|__t_inner| __t_inner.name == inner.name)
})
})
.fold(HashMap::new(), |mut acc, __t_inner| {
let type_path = __t_inner.token.type_path();
let counter = acc
.entry(type_path.clone())
.or_insert((0, __t_inner.clone()));
counter.0 += 1;
acc
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part we need to make a new pass. Thanks for the refactoring though.

@glihm glihm merged commit 7a75856 into cartridge-gg:main Oct 25, 2024
3 checks passed
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.

2 participants