-
Notifications
You must be signed in to change notification settings - Fork 190
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
Update toSnakeCase
to better handle plurals, version numbers, and other pathological cases
#3037
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
toSnakeCase
to better handle plurals, version numbers, and other pathological cases
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
private fun String.splitOnWordBoundaries(): List<String> { | ||
val out = mutableListOf<String>() | ||
// These are whole words but cased differently, e.g. `IPv4`, `MiB`, `GiB`, `TtL` | ||
val completeWords = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "ttl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Will the algorithm work correctly if we add a word to the list that has a substring of it already in the list? For example, if "sigv4a" is added to the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add "kib"
to be future proof?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah should work (but sigv4a should probably work properly already because the issue is about when you get to the capital V
)
private fun String.splitOnWordBoundaries(): List<String> { | ||
val out = mutableListOf<String>() | ||
// These are whole words but cased differently, e.g. `IPv4`, `MiB`, `GiB`, `TtL` | ||
val completeWords = listOf("ipv4", "ipv6", "sigv4", "mib", "gib", "ttl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add "kib"
to be future proof?
This commit introduces a word splitting algorithm used for snake casing that has much better results than the current Smithy algorithm. Specifically, it has better behavior for acronyms ending in `s`, version numbers, and compound acronyms like `MiB` and `GiB`.
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
There are currently a lot of fields in the SDK that are clearly converted to snake-case wrong:
Description
Testing
Checklist
CHANGELOG.next.toml
if I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.toml
if I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.