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

add slug with occurrences #20

Merged
merged 5 commits into from
Jan 8, 2019
Merged

add slug with occurrences #20

merged 5 commits into from
Jan 8, 2019

Conversation

UziTech
Copy link
Contributor

@UziTech UziTech commented Dec 3, 2018

add slug with occurrences to the occurrences array to prevent collisions with text ending in a number

Fixes #19

@UziTech
Copy link
Contributor Author

UziTech commented Dec 3, 2018

right now I have it recursively check each slug with its occurrence to prevent collisions but I'm not sure if that is the best option. When a slug with an occurrence exists should it just add another occurrence or should it increment the occurrence.

var GithubSlugger = require('github-slugger')
var slugger = new GithubSlugger()

slugger.slug('foo-1')
// returns 'foo-1'

slugger.slug('foo')
// returns 'foo'

slugger.slug('foo')
// returns 'foo-1-1'
// should it return 'foo-2'?

@wooorm
Copy link
Collaborator

wooorm commented Dec 3, 2018

@UziTech For your question, that depends on what GH is doing! This project should match that. For an example, I’ve created a little markdown file that shows that to people in a PR here: #17

@UziTech
Copy link
Contributor Author

UziTech commented Dec 3, 2018

it looks like github sets the second foo to foo-2

@UziTech
Copy link
Contributor Author

UziTech commented Dec 3, 2018

I changed it so it matches github

var GithubSlugger = require('github-slugger')
var slugger = new GithubSlugger()

slugger.slug('foo-1')
// returns 'foo-1'

slugger.slug('foo')
// returns 'foo'

slugger.slug('foo')
// returns 'foo-2'

@wooorm wooorm merged commit ee6a3dd into Flet:master Jan 8, 2019
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.

3 participants