Skip to content
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

kebab case doesn't treat numbers as separate word? #18

Open
Luro02 opened this issue Nov 6, 2019 · 5 comments
Open

kebab case doesn't treat numbers as separate word? #18

Luro02 opened this issue Nov 6, 2019 · 5 comments

Comments

@Luro02
Copy link

Luro02 commented Nov 6, 2019

I was redirected from this issue
Peternator7/strum#72

@Luro02 Luro02 changed the title screaming kebab case doesn't treat numbers as separate word? kebab case doesn't treat numbers as separate word? Nov 6, 2019
@jplatte
Copy link
Collaborator

jplatte commented Dec 29, 2020

It's unfortunate that there is no behaviour here that is obvious to everybody, but this has been the default behaviour for both heck and serde (as I mentioned on that other issue) for a long time, so I think it would be a very bad idea to change it for everybody.

I think it would be feasible to expose an API that supports this, but that would require some refactoring and also afterwards strum would have to add support for it. Are you interested in working on that?

@Luro02
Copy link
Author

Luro02 commented Dec 30, 2020

Are you interested in working on that?

Not sure if I will have the time for this in the near future.

@jplatte
Copy link
Collaborator

jplatte commented Dec 30, 2020

Alright, I've posted this to This Week in Rust's Call for Participation section. Maybe sbd. else wants to work on it 🙂

So my idea is that this could work as .convert_case(ConvertCaseOpt { case: Case::SHOUTY_SNEK, number_starts_word: true }). I think this would have to be done:

  • Introduce enum Case with variants for the unique case conventions (Kebab, LowerCamel, ...) and associated constants for the aliases (Pascal, snek, SHOUTY_SNEK)
  • Add a general ConvertCase trait that provides the convert_case method
  • Update all the other traits to be implemented in terms of convert_case (with number_starts_word: false) or move all methods into it directly

@danburkert
Copy link
Contributor

Adding on to the ‘please don’t change this’ pile - this would be a very invasive breaking change for prost users if enabled by default. As for configuration APIs, quoting from the README:

This library is a little bit opinionated (dropping punctuation, for example). If that doesn't fit your use case, I hope there is another crate that does. I would prefer not to receive PRs to make this behavior more configurable.

@jplatte
Copy link
Collaborator

jplatte commented Jan 1, 2021

So to be clear: I've taken over maintenance of this crate, that's why I wrote some instructions about how to do this and posted it to CfP. Further, changing the default is not an option being discussed here. This is only about having some customization options, for those who need it.

You have a point about that bit from the README, but I think the treatment of numbers is something that would deserve some customization options, and I think the ConvertCase trait (with just a case parameter) would be a worthwhile refactoring independently, after which this would be very easy to implement.

ssd532 added a commit to ssd532/heck that referenced this issue Jan 3, 2021
ConvertCase implements convert_case() which accepts customization options to tweak behaviour of other case conversion functions. Currently, it accepts `number_starts_word` option to have word boundaries when characters in a word change from numeric to alphabetic or vice versa. 

Other case conversion functions are implemented in terms on convert_case() by passing `number_starts_word` as false by default. 

ref: withoutboats#18
ref: Peternator7/strum#72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants