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

[FEATURE] Add toSnakeCase #321

Merged
merged 4 commits into from Dec 23, 2017
Merged

[FEATURE] Add toSnakeCase #321

merged 4 commits into from Dec 23, 2017

Conversation

Chalarangelo
Copy link
Owner

Converts a string to snakecase.

Description

Lodash[ACTION] #100 -> https://lodash.com/docs/4.17.4#snakeCase

What does your PR belong to?

  • Website
  • Snippets
  • General / Things regarding the repository (like CI Integration)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking improvement of a snippet)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have checked that the changes are working properly
  • I have checked that there isn't any PR doing the same
  • I have read the CONTRIBUTING document.

@rohitanwar
Copy link
Contributor

toSnakeCase('Teletubbies say Eh-oh') returns "_teletubbies_say__eh_oh" on my google chrome console. Adding the following line in end will help
str.startsWith('-') ? str.slice(1,str.length) : str;

@Chalarangelo
Copy link
Owner Author

@kriadmin Good edge case I missed, however that might be sub-optimal. Maybe there's a regex way to deal with that?

@rohitanwar
Copy link
Contributor

I already knew about this edge case from about a month ago. This can't be dealt with regex(according to me). Checking if it starts with '-' is the only solution(acc to me).

@Chalarangelo
Copy link
Owner Author

Alright, I'll not merge and wait for a couple more opinions until tomorrow. If there's no better (i.e. more elegant) way, this is a pretty decent fix nonetheless, so I'll update and merge later.

@iamsoorena
Copy link
Contributor

@Chalarangelo I also see that the last test returns this:
some_mixed_string__with_spaces_underscores_and_hyphens
take a closer look before with
It added two underscores.

@Chalarangelo
Copy link
Owner Author

@iamsoorena Good catch, I'll see if I can figure out a way to update it tomorrow, then. Feel free to apply fixes if you figure it out before I do!

checks if the uppercase charcater is preceded by another character
@elderhsouza
Copy link
Contributor

@kriadmin @Chalarangelo
I tried my hand on it, checking if the uppercase character is preceded by a character first.

@rohitanwar
Copy link
Contributor

@elderhsouza Try this text "I     have   Lot's of   _Spaces"

@Chalarangelo
Copy link
Owner Author

I just made some updates, replacing multiple spaces/hyphens/underscores with a single underscore, this should now work pretty much fine. Can you find any more edge-cases we might have missed?

@rohitanwar
Copy link
Contributor

What about 'ThisIsAURL' ?

@Chalarangelo
Copy link
Owner Author

@kriadmin I see how you would want to handle this but I don't think that any library would handle it properly anyways.

@rohitanwar
Copy link
Contributor

@Chalarangelo Lodash returns this_is_url

@rohitanwar
Copy link
Contributor

@Chalarangelo See the second comment here lodash/lodash#561 by jdalton

@Chalarangelo
Copy link
Owner Author

Chalarangelo commented Dec 23, 2017

@kriadmin That would require a lot of code to handle just one (or a select few) cases. If it doesn't make the snippet very difficult to read, I'm not against it, however I am not sure I can implement this on my own. Suggestions are welcome, I'm marking this snippet on hold, although I believe the toKebabCase snippet was merged already by someone else and has the same issue. I am merging this one, too, as they behave the same and feel free to open a PR fixing the issue if it is somehow doable.

@Chalarangelo Chalarangelo merged commit 58cce7f into master Dec 23, 2017
@flxwu flxwu deleted the Chalarangelo-snakeCase branch December 23, 2017 22:03
@lock lock bot locked as resolved and limited conversation to collaborators Dec 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants