-
Notifications
You must be signed in to change notification settings - Fork 110
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
Number word boundary bug #22
Comments
I also have this problem in this case: strcase.ToSnake("ID3") // returns id_3 but I think it should return id3
strcase.ToSnake("ID3v2_3") // returns id_3_v2_3 but I think it should return id3_v2_3 version commit I use: |
#24 introduces the change for @jpotterm with these tests: Lines 51 to 52 in 23e9d4e
@nasermirzaei89 I'm not sure what the particular rule would be to differentiate those particular cases, can you describe the intention please? It looks like maybe you mean "any numbers following the token 'ID' or 'v' should be considered part of that token and not split from it"? Is it specifically for <ID|V>+? The current rule is that numbers act as word boundaries, so ToSnake("ID3v2_3") gives "id_3_v_2_3". |
I think the intention is to only introduce delimiters after numbers, not before them. This is a super easy change with my recent contributions, see here. I'd also prefer this behavior but didn't include it in #24 since the current behavior isn't necessarily wrong so it feels a little more like a breaking change. |
@iancoleman I think the solution of @NathanBaulch is good. You should check if it doesn't break any other cases. Thanks for the follow up 🎉 |
#24 is looks merged, however it doesn't solve the numbering issue: |
There are some many possibilities here, that I doubt you can make this work for everyone without introducing a bunch of configuration options. In my case I am looking at |
I have faced the same issue. I want |
Numbers usually act as word boundaries, but not always (which I believe is a bug). Here are inconsistent test cases:
The text was updated successfully, but these errors were encountered: