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

added support for custom separators #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BrianLeishman
Copy link

Added support for setting custom separators via slug.SetSeparator("-")

This keeps the backwards compatibility, passes all the tests, and allows for the use of longer strings, i.e. not just single characters.

There's also a slug.GetSeparator() function so you can see what the current separator is set to.

@BrianLeishman
Copy link
Author

Basically created this because this is almost perfect for use in file paths, but only being different because we want to keep the spaces in our file/folder names.

So our use case would be setting slug.SetSeparator(" "), and that's possible with this pull.

@matrixik
Copy link
Member

Hi, thank you for this PR. I'm sick now and don't have much strength for review so just some short comments.

Could you add tests to this new functionality? I prefer tests generated by https://github.com/cweill/gotests if possible.

Could you move 'init' to top?

I'm not so fond of getters and setters but looks like we need them to not broke API. But could you change they names to stick with official guidelines
https://softwareengineering.stackexchange.com/a/278830/143306

Thank you

@matrixik
Copy link
Member

Also did you looked at benchmarks how much speed degrade with your change?

You could add info about it to doc.go and readme or I will do it after merging.

@BrianLeishman
Copy link
Author

BrianLeishman commented Sep 16, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants