modified to repect serde rename directive for struct fields #47
Conversation
src/utils.rs
Outdated
| // this detects the '(...)' part in #[serde(rename_all = "UPPERCASE", tag = "type")] | ||
| // we can use this to get the value of a particular argument | ||
| // or to see if it exists at all | ||
|
|
||
| // make sure the delimiter is what we're expecting |
There was a problem hiding this comment.
this comment seems to have been duplicated
There was a problem hiding this comment.
Should be fixed in the latest commits.
| @@ -10,177 +23,154 @@ pub fn has_attribute(needle: &str, attributes: &[syn::Attribute]) -> bool { | |||
| }) | |||
| } | |||
|
|
|||
| fn check_expression_is_path(expr: Expr) -> Option<ExprPath> { | |||
There was a problem hiding this comment.
could I request you add a comment explaining this method? (why would someone want to use this?)
There was a problem hiding this comment.
Sure, no problem. I took a shot in the latest commits. Please let me know what you think.
src/utils.rs
Outdated
| } | ||
|
|
||
| /// Check has an attribute arg. | ||
| pub fn has_attribute_arg(needle: &str, arg: &str, attributes: &[syn::Attribute]) -> bool { | ||
| get_attribute_arg(needle, arg, attributes).is_some() | ||
| } | ||
|
|
||
| fn check_token_tree(tt: proc_macro2::TokenTree) -> Option<String> { |
There was a problem hiding this comment.
could we also add a comment on how and when to use this method?
Similar to:
// this detects the '(...)' part in #[serde(rename_all = "UPPERCASE", tag = "type")]
// we can use this to get the value of a particular argument
// or to see if it exists at all
There was a problem hiding this comment.
Sure, no problem. I took a shot in the latest commits. Please let me know what you think.
src/typescript.rs
Outdated
| "String" => "string".to_owned().into(), | ||
| "NaiveDateTime" => "Date".to_owned().into(), | ||
| "DateTime" => "Date".to_owned().into(), | ||
| "Uuid" => "string".to_owned().into(), |
There was a problem hiding this comment.
No problem. I actually needed it in my use case, so it worked out.
src/typescript.rs
Outdated
| "Uuid" => "string".to_owned().into(), | ||
| x if x.contains("Cow") => "string".to_owned().into(), |
There was a problem hiding this comment.
This worries me a bit! What if someone uses Cow in their struct/enum name?
There was a problem hiding this comment.
I agree and was definitely short-sighted, I abstracted the check to a method in the latest commits. It should grab the T from Cow<'_, T>, and send the T back through the original method. Please let me know what you think.
test/directory_input/typescript.d.ts
Outdated
| } | ||
|
|
||
| /** Time in UTC seconds */ | ||
| type UTC = number |
There was a problem hiding this comment.
hmmm -- do we expect this test to output nothing?
There was a problem hiding this comment.
Nope, sorry I missed this when running my tests. It was due to using the source path object instead of the Walkdir entry, tested again and should be working as expected.
|
hey @aaronh-mood, thanks for taking the time to cleanup and add support for the rename directive 🙌 ! No disrespect taken, it's a community effort at the end of the day and we're all trying to make the rust ecosystem better in our spare times :-) |
|
Thanks @Wulf! This was actually my first PR to a public repo, so I wasn't quite sure how it would be taken but I appreciate the nice reception and hopefully I can add some value to this project. Keep up the great work. This already helped me a great deal. |
Modified tests and some of the big for loops abstracting some of the logic to other functions. Mostly personal preference but I do believe that there are some Rust performance gains when using iterators and chaining compared to for loops. (I'm not 100% certain though).
For the Debug flag, added a state::InitCell which is populated once the entry point is called, so you don't have to pass around the "debug" flag to all methods and callers.
In the main lib.rs file - changed the path parameter from
PathbuftoP: AsRef<Path>so now you can pass around references, rather than cloning the path object.Tests all complete and output looks similar to existing tests in the repo. No disrespect intended with re-writing some of the methods, just something fun to play around with. If you don't like it or want it, let me know and I can modify the request to only include the part that applies directly to struct field names.
Thanks!