add import parser/ast/visitor/codegen #126
Conversation
| Hey @kamijin-fanta! Sorry for not picking this up sooner, I'll review asap. |
maciejhirsz left a comment
maciejhirsz left a comment There was a problem hiding this comment.
This looks pretty cool! I'll try to make suggestions as a PR back to your branch.
ratel-codegen/src/statement.rs Outdated
| return | ||
| } | ||
| | ||
| for spec in self.specifiers { |
There was a problem hiding this comment.
There is probably a way to make this more DRY.
Could try using gen.write_list here with a filter or filter_map iterators if the inner types got ToCode implemented for them. This would also avoid having to keep track of the tail boolean, which seems like an easy thing to get wrong if this ever gets refactored.
There was a problem hiding this comment.
gen.write_list does not seem to accept Vec<ForImportSpecifier>.
How can I implement it?
I thought of such a code.
let specs = self.specifiers.iter().filter_map(|spec| { match spec.item { ForImportSpecifier::ImportDefaultSpecifier(_) | ForImportSpecifier::ImportNamespaceSpecifier(_) => Some(spec.item), _ => None } }).collect::<Vec<_>>(); gen.write_list(specs);There was a problem hiding this comment.
write_list operates on IntoInterator types, so you can skip collecting to Vec part:
let specs = self.specifiers.iter().filter_map(|spec| { match spec.item { ForImportSpecifier::ImportDefaultSpecifier(_) | ForImportSpecifier::ImportNamespaceSpecifier(_) => Some(spec.item), _ => None } }); gen.write_list(specs); ratel-codegen/src/statement.rs Outdated
| } | ||
| } | ||
| let mut need_close = false; | ||
| for spec in self.specifiers { |
There was a problem hiding this comment.
Ditto about this loop, though here it's a bit more complicated due to the opening and closing brace. I'll write some suggestions for the code.
ratel/src/astgen/statement.rs Outdated
| S: Serializer, | ||
| { | ||
| self.in_loc(serializer, "ImportDeclaration", 1, |state| { | ||
| // state.serialize_field("specifiers", &self.specifiers)?; |
There was a problem hiding this comment.
I'm sorry. I forgot to implement astgen. I implemented it now.
| I did rewrite it to using write_list. |
| @maciejhirsz Is there anything I can do? I want to support this project through PR. |
| @kamijin-fanta I haven't worked on this for a very long time now. Would you like to be added to the organization so you can do PRs without approval? |
| @maciejhirsz Thank you for the merge! It is nice idea! But I can't make much time like you... |
| Sent you an invite. It's better if someone works on this, even if just with a tiny amount of tine, than for this repo to sit here stale :). |
3 participants
I am a beginner in rust language. I tried writing it.
ref #82