Proof-of-concept to allow initialisms to be set through flags. #89
Conversation
internal/plugin/plugin.go
Outdated
There was a problem hiding this comment.
Most relevant part of the change is here and the strcase.go file below.
There was a problem hiding this comment.
Hey @cory-miller, this is great! Thanks for the submission, I like where this is going.
I have a few small changes/nits that I think would ago a long ways in improving the overall UX of this new plugin option:
- Rename
InitialismsStrCase []stringtoIgnoreAcronyms []stringand theignore-strcase-initialismstoignore-acronymsor even justAcronyms/acronymsand sort alphabetically for consistency with the other Config fields - move the actual registration logic (i.e.
strcase.ConfigureAcronym()out oftoCamel/toLowerCameland into the plugin'sRunmethod - remove the
.ideadirectory and consider adding an entry to.gitignore - bonus points for updating the docs and changelog
Let me know your thoughts!
internal/plugin/strcase.go
Outdated
|
|
||
| s := fmt.Sprintf(format, args...) | ||
|
|
||
| for _, initialism := range svc.cfg.InitialismsStrCase { | ||
| if strings.Contains(s, initialism) { | ||
| strcase.ConfigureAcronym(s, s) | ||
| } | ||
| } | ||
|
|
||
| return strcase.ToCamel(s) |
There was a problem hiding this comment.
instead of calling ConfigureAcronym everytime toCamel and toLowerCamel are called, consolidate and move this logic into the plugin's Run method
There was a problem hiding this comment.
So unfortunately I don't think strcase works in the way you'd expect. Instead of letting you mask particular parts of the input, you have to mask everything. It might be easier to illustrate with an example:
https://go.dev/play/p/8UUJHeOxQSo
Maybe that should be a bug on the strcase part. I'm not sure what the intended behavior is.
|
@cludden I took care of 1, 3, and 4 from #89 (review) and left a comment in #89 (comment). Another option I thought of - when calling out to the protogen library and the |
Showcasing what I see in #90