search

preaction / Statocles

Static website CMS
http://preaction.me/statocles
Other
84 stars 33 forks source link

Improve slugs #514

Closed ferki closed 7 years ago

ferki commented 7 years ago

This PR adds some basic tests for slug generation, and fixes two related issues. On top of that, it tries to convert non-ASCII characters in slugs too, and as a side-effect of removing the leading/trailing dashes from slugs, I had up update some tests.

I'm happy to rearrange and/or split the PR before merging if you prefer that, it just seemed like a good idea to keep these changes together. Feel free to let me know.

note: I found the instructions about where should I add myself as a contributor a bit confusing, so I left that out. I can send a followup commit or open a separate issue about that, but also feel free to add me wherever you think it fits :)

preaction commented 7 years ago

Thanks for the PR!

I should check those "How to add yourself as a contributor" notes, because contributors are calculated automatically from the commit log. So, you don't have to do anything ;)

I've been terrible so far about Unicode, but you're probably right in assuming ASCII as a lowest-common denominator in filesystems. I've read some things that make my brain hurt...

I'm going to try to track down why Travis is failing, then look this over.

ferki commented 7 years ago

I guess I just need to add Text::Unidecode to dist.ini. Let me run it through dzil locally and see if I can fix this up (I just used prove earlier :)

preaction commented 7 years ago

Ah, yes. Looks like I must have Text::Unidecode from some other module, which is why it worked locally. This is why Travis exists ;)

Otherwise, once that's done, this looks good to go. You even found the trap I laid in the tests for exactly this issue ;)

preaction commented 7 years ago

Thanks! I'll tag a release and get this out: I really hate the current slug behavior in my blogs.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.02%) to 93.145% when pulling 0cc15b983740a253774fbadd5573ae94dde388f1 on ferki:slugs into e11fc935c179c2358460fa8f986ae7d30b8b641f on preaction:master.

ferki commented 7 years ago

Great! I wanted to squash away the fixup! commit before merging, but you were too fast! :D

preaction commented 7 years ago

Sorry. I can rebase it away for you and force-push. Then it'll look all nice.

ferki commented 7 years ago

If you think it's worth to slightly rewrite the history in master branch in order to have a clean history there, then sure, go ahead. I'm fine with both, and I think you're in a much better position to judge that, so I'll let you decide :) On my part, I'll be more careful/verbose next time!