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

Option to limit Slug Length #50

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ntbm
Copy link

@ntbm ntbm commented Oct 1, 2022

Implements #18

Added Option to Limit Output Slug Length.
Default Value is null to not introduce any breaking Changes into the Library.

@ntbm ntbm changed the title [#18] Option to limit Slug Length Option to limit Slug Length Oct 1, 2022
@ntbm ntbm marked this pull request as draft October 1, 2022 07:47
Copy link
Owner

@ctolkien ctolkien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: typo in comment

README.md Outdated Show resolved Hide resolved
@ctolkien
Copy link
Owner

ctolkien commented Oct 4, 2022

Hi @ntbm - thanks for this!

@ctolkien
Copy link
Owner

ctolkien commented Oct 4, 2022

The failing test is an interesting one... normally the lib won't leave trailing -'s but we are specifically asking for a max length....

@ntbm
Copy link
Author

ntbm commented Oct 4, 2022

The failing test is an interesting one... normally the lib won't leave trailing -'s but we are specifically afking for a max length....

That's why I added it. Handling all the specific edgescases will be a nightmare. So I'd vote to just accept that edge case. Question is if it be best to document it 🤔

@poke
Copy link
Contributor

poke commented Oct 9, 2022

Would it make sense to make the limiting a bit “smarter” so that it would not break within words if possible?

For example "this is an example" would slugify to "this-is-an-example". But cutting it at 15 characters would result in "this-is-an-exam" which might not be ideal. Instead, there could be some more sophisticated limiting approach which would result in "this-is-an" instead since example doesn’t fit anymore.

@ctolkien
Copy link
Owner

Would it make sense to make the limiting a bit “smarter” so that it would not break within words if possible?

If there is a fast way of doing that? I imagine you need scan back to find the last non - character within the string limit length.

@ctolkien ctolkien changed the base branch from master to main July 24, 2023 08:43
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

3 participants