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

Initial work on character preservation option. #70

Merged
merged 7 commits into from
Jan 30, 2023
Merged

Initial work on character preservation option. #70

merged 7 commits into from
Jan 30, 2023

Conversation

aegatlin
Copy link
Contributor

Fixes #55

I wanted to preserve the HTML fragment, #, in my use-case for slugify. I saw #55 and thought I'd kick off the PR for it.

@aegatlin
Copy link
Contributor Author

hmm, I actually just realized # should probably be treated as a one-time occurrence and not something that should be allowed repeatedly in a slug... 🤔

We could still keep this and modify the allowSet to exclude #, and then address that separately. Let me know what you think 👍

@sindresorhus
Copy link
Owner

and then address that separately

Address it how?

@aegatlin
Copy link
Contributor Author

what I'm doing in my workflow to slugify urls is basically...

const [a, b] = s.split('#')
const slug = `${slugify(a)}#${slugify(b)}`

But, this depends on the extent to which you want slugify === urlify.

@sindresorhus
Copy link
Owner

I think if the user sets # as a preserved, it's up to them to ensure it works in a URL. I don't think we should have any special handling.

@aegatlin
Copy link
Contributor Author

Sounds good to me. I think the PR is ready to review, then. 😀👍

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@aegatlin aegatlin requested a review from sindresorhus January 9, 2023 22:29
readme.md Outdated Show resolved Hide resolved
@aegatlin aegatlin requested a review from sindresorhus January 24, 2023 23:23
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.

dot "." is legal in a URL but it is replaced with "-"
2 participants