-
-
Notifications
You must be signed in to change notification settings - Fork 860
add unicode support in slugs #836
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
Conversation
|
Hmm, so this does not do any transliteration, but instead only removes those characters which should not appear in URLs? Non-ASCII characters would remain, right? As per discussion in #194, that's not what we wanted to do, but I'm unsure whether this solution may be okay, too. |
|
there are unicode TLDs for example and also i have not invented this method of creating slugs it is created by wikipedia |
yes
yes
right |
|
Without commenting on the actual method used for slug generation... I was under the impression that recently @franzliedke made a change so that slugs are generated on the server-side. This PR however only touches the JS version of the function, which is no longer used. |
|
Well, it is used somewhere; I believe the tags extension uses it. |
updated php version (have not tested) |
|
@franzliedke @tobscure any progress on this? |
|
If @franzliedke moved slug generation to the backend, wouldn't it make more sense to remove all logic from the frontend - SRP? |
|
@luceos the slug logic on the frontend is only used to live autocomplete the slug field when typing a tag name in the admin interface. |
|
I see, so both frontend and backend need to mimic the same behavior closely. Isn't slug creation something we want enforced from one side, with the frontend being the most logical choice? |
|
Well, if anything, we can only enforce it on the server. But since we want these live-preview features on the client, we'll have to go with a duplicate implementation here. (The only way around that is isomorphic JS and, well, that doesn't really fit here, hehe.) |
|
The URLs is something we definitely would like to control server side as we would not like somebody to control from his client side how a post URL is going to look like as the potencial for abuse/trolling is enormous. @franzliedke Maybe for preview we can query the server? On my opinion, this commit fixes #194. What is stopping this PR to be accepted and merged? |
|
@johannsa We haven't tested it yet. And we might use a library instead. |
|
I'll look at it in time for the next beta, though. |
I like the idea of unicode slugs, but some admins may prefer transliterated slugs if that's the norm in their region. So it might be a good idea to add an admin preference selecting between this method and a slug transliteration library/whatever. Going back to the comment by @luceos ...
There's an important difference between tag slugs and title slugs:
Given this difference, we could probably get away with leaving the tag slugs as they are now: no unicode support, no transliteration ... there's no need for transliteration, since the admin should be able to do that manually as well, so this could be an option if, for example, there's some reason that unicode can't be used in tag slugs. But the best solution would probably be to have the admin panel suggest tag slugs using the same logic (transliteration or unicode support) as is used for the title slugs. |
|
Superseded by #1385. |
Unicode compatible slug generation code is taken from here https://github.com/bryanbraun/anchorjs/blob/master/anchor.js#L208
so if this accepted we need to add @bryanbraun in contributors list
for me this fixes #194
this will work for any language around the globe