Adds support custom name resolver in codegen#680
Adds support custom name resolver in codegen#680negezor wants to merge 176 commits intoSeaQL:masterfrom negezor:resolve-module-names
Conversation
| impl NameResolver for SingularNameResolver { | ||
| fn resolve_module_name(&self, name: &str) -> String { | ||
| singular(name.to_snake_case()) | ||
| } | ||
|
|
||
| fn resolve_entity_name(&self, name: &str) -> String { | ||
| singular(name.to_camel_case()) | ||
| } | ||
|
|
||
| fn resolve_relation_name(&self, name: &str) -> String { | ||
| singular(name.to_camel_case()) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Having a name resolver is a good idea :)
Can we supply the output of NameResolver's default implementation to singular method?
There was a problem hiding this comment.
@billy1624 of course. Then under what flag to leave the current behavior? 🤔
There was a problem hiding this comment.
i don't understand this statement either
sea-orm-cli/src/commands.rs
Outdated
| let name_resolver = NameResolver::new(singularize); | ||
|
|
||
| let output = EntityTransformer::transform(table_stmts, name_resolver)? | ||
| .generate(expanded_format, WithSerde::from_str(with_serde).unwrap()); |
There was a problem hiding this comment.
Hey @negezor, thanks for the updates! I think the semantic isn't seems right. The transformer, EntityTransformer::transform(table_stmts, name_resolver), don't need name resolver. Instead, we could have introduce a new struct called EntityWriterContext which contains three things...
pub struct EntityWriterContext {
pub(crate) expanded_format: bool,
pub(crate) with_serde: WithSerde,
pub(crate) name_resolver: NameResolver,
}Then, EntityWriter::generate method would take an EntityWriterContext. Thoughts?
There was a problem hiding this comment.
@billy1624 In the case where we discussed about saving a context info on which datetime crate the user chooses in the --date-time-crate flag for sea-orm-cli, we'd basically write:
pub struct EntityWriterContext {
pub(crate) expanded_format: bool,
pub(crate) with_serde: WithSerde,
pub(crate) name_resolver: NameResolver,
pub(crate) date_time_crate: &str,
}and pass this context all the way down to Column.get_rs_type() for resolving the Rust types based on chrono vs time? :)
| pub fn resolve_conjunct_relations_via_module_name(&self, name_resolver: &NameResolver) -> Vec<Ident> { | ||
| self.conjunct_relations | ||
| .iter() | ||
| .map(|con_rel| con_rel.resolve_via_module_name(name_resolver)) | ||
| .collect() | ||
| } | ||
|
|
||
| pub fn resolve_conjunct_relations_to_module_name(&self, name_resolver: &NameResolver) -> Vec<Ident> { | ||
| self.conjunct_relations | ||
| .iter() | ||
| .map(|con_rel| con_rel.resolve_to_module_name(name_resolver)) | ||
| .collect() | ||
| } | ||
|
|
||
| pub fn resolve_conjunct_relations_to_relation_name(&self, name_resolver: &NameResolver) -> Vec<Ident> { | ||
| self.conjunct_relations | ||
| .iter() | ||
| .map(|con_rel| con_rel.resolve_to_relation_name(name_resolver)) | ||
| .collect() | ||
| } | ||
|
|
There was a problem hiding this comment.
All these resolve_* methods (here, and in other files except those in name_resolver.rs) seems just a duplicate of the original methods few line below, e.g. resolve_conjunct_relations_via_module_name method is just get_conjunct_relations_via_snake_case method with name resolve applied. We should change the logic in the original method, i.e. applying name resolver in it.
There was a problem hiding this comment.
I didn't touch them originally, as that would have required more changes. Also, they are now more than just getters. Perhaps this one could be changed by later refactoring. 🤔
|
Also, since in both of our PRs we're using |
|
Hey @negezor, any updates? And do you need any help? :) |
Good suggestions! We could consolidate all flags into |
* introduce rustfmt * Cargofmt on all sea-orm crates & examples * Revert testing script * cargo fmt --manifest-path sea-orm-rocket/Cargo.toml --all * add a script for formatting * add nightly component for formatting * set timeout and manual nightly install in github action * add flag for manifest-path Co-authored-by: Billy Chan <[email protected]>
* Fix clippy warnings * cargo fmt * Fix clippy warnings * cargo fmt
|
I think I did the rebase wrong somehow 😅 |
| pub fn resolve_module_name(&self, name: &str) -> String { | ||
| let name = name.to_snake_case(); | ||
|
|
||
| if self.singularize { | ||
| singular(name) | ||
| } else { | ||
| name | ||
| } | ||
| } | ||
|
|
||
| pub fn resolve_entity_name(&self, name: &str) -> String { | ||
| let name = name.to_camel_case(); | ||
|
|
||
| if self.singularize { | ||
| singular(name) | ||
| } else { | ||
| name | ||
| } | ||
| } | ||
|
|
||
| pub fn resolve_relation_name(&self, name: &str) -> String { | ||
| let name = name.to_camel_case(); | ||
|
|
||
| if self.singularize { | ||
| singular(name) | ||
| } else { | ||
| name | ||
| } | ||
| } |
There was a problem hiding this comment.
Hey @negezor, thanks for the hard rebase (it's hard as this PR is far behind from master).
I wonder why we need three different methods? I think we can merge it and have two methods? One for snake_case and the other for camel_case.
There was a problem hiding this comment.
A fair remark, but I proceeded from the clear goals of the method. I can make changes if necessary.
20 participants
I added the
NameResolvertrait for better maintainability. I haven't touched columns and other parts that don't depend on module names yet, and I haven't deleted the getter code either.Adds
--singularize, which generates singular module names.sea-orm-codegenBreaking Changes
EntityTransformer::transform