Improve DefaultSlugGenerator to preserve complex unicode letters, numbers, and marks #467
Conversation
When the slug is generated from the title text, try to preserve letters, numbers and marks declared in Unicode spec. Certain complex languages declare code points for marks that are supposed to be used with letters. This allows to use titles in every Unicode-recognized language to safely generate a slug from the title text without dissecting to a point that it loses the meaning.
There was a problem hiding this comment.
Thank you for catching this issue!
Would you mind adding those test cases to https://github.com/thephpleague/commonmark/blob/1.4/tests/unit/Extension/HeadingPermalink/Slug/DefaultSlugGeneratorTest.php as well? Once that's done we can merge and release this fix :)
colinodell
added
bug
|
Thanks for your quick response and review @colinodell. You are right the tests could improve with the new strings. I added a few more tests with same examples. Tests passed locally, but Travis CI seems to be stuck on the master results. |
|
Thank you so much! And yeah, I'm not sure why Travis passes on the "Pull Request" build but doesn't pass on the other - regardless, it confirms your tests are passing so I'll get this merged and released momentarily :) |
colinodell
added
implemented
|
Tagged and released: https://github.com/thephpleague/commonmark/releases/tag/1.4.3 Thanks again for your contribution, I truly appreciate it! |
|
Awesome, thanks a lot for the quick responses and reviews 🎉. |
The
DefaultSlugGeneratorimplementation currently strips out certain unicode code-points that can render titles in many languages not work properly.Test: https://gist.github.com/Ayesh/f357dcc18b60e117ab771476606dda3a
league/commonmarkslugTest#test#testඅත්හදා බලන මාතෘකාව#අත්හදා-බලන-මාතෘකාව#--අත්හදා බලන මාතෘකාව -#අත්හදා-බලන-මාතෘකාව---#----测试标题#测试标题#試験タイトル#試験タイトル#The last second and third strings are from my native Sinhalese language. Some of the glyphs are letters (
\p{L)), but we also have marks (\p{M}). These marks are not symbols (\p{S}) or punctuation (\p{P}). I think the current slug-ify process makes it not possible to use the HeadingPermalink extension in a meaningful way for those who write in complex scripts, which includes Eastern Asian and South Asian languages (which, statistically, accounts for more than half of the world population)Pretty much every news site, and even WikiPedia processes slugs this way.
This PR relaxes the stripping logic to allow complex scripts to preserve the title. Punctuation, symbols, emoji, etc are still removed. After this change,
league/commonmarkslugs match GitHub's slugs verbatim.