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

Use slugs.maxlength when generating slugs #6526

Closed
distantnative opened this issue Jul 12, 2024 · 1 comment · Fixed by #6617
Closed

Use slugs.maxlength when generating slugs #6526

distantnative opened this issue Jul 12, 2024 · 1 comment · Fixed by #6617
Assignees
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Milestone

Comments

@distantnative
Copy link
Member

distantnative commented Jul 12, 2024

Currently, we often ignore the slugs.maxlength option when generating slugs but rather use the default 128 from Str::slug().

I see two options:

  1. Integrate App(null, true)?->option('slugs.maxlength') into Str::slug() to use as default when set. Could this be problematic if someone limits it to a very short length, as we also often use Str::slug() for non-slugs?

  2. Create a new, e.g. App::slug() that takes the option and passes it with all the other parameters to Str::slug(). Search through the code where we should use App::slug() instead of Str::slug(). (or Page::slug(), Router::slug() or so).

@lukasbestle @bastianallgeier Which do you think would be better?

@distantnative distantnative added type: enhancement ✨ Suggests an enhancement; improves Kirby needs: help 🙏 Really needs some help on this labels Jul 12, 2024
@distantnative distantnative changed the title Use slugs.maxlenght when generating slugs Use slugs.maxlength when generating slugs Jul 12, 2024
@lukasbestle
Copy link
Member

I think your point on using the method for other reasons is what counts here. So I think option 2 is better as it doesn't involve a breaking change with hard to overlook consequences.

@distantnative distantnative self-assigned this Aug 15, 2024
@distantnative distantnative removed the needs: help 🙏 Really needs some help on this label Aug 15, 2024
@distantnative distantnative linked a pull request Aug 15, 2024 that will close this issue
4 tasks
@bastianallgeier bastianallgeier added this to the 4.5.0 milestone Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants