Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Added Inflector#parameterize for easy slug generation ("Donald E. Knu…
…th".parameterize => "donald-e-knuth") #713 [Matt Darby]
- Loading branch information
Showing
5 changed files
with
51 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would have been even better with stringex, since that catches foreign characters much better (i.e. at all).
http://github.com/rsl/stringex/tree/master
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stringex looks very cool, but seems like it could be overkill here. Slugalizer is very little code but handles accented characters as well as some corner cases that parameterize doesn’t:
http://github.com/henrik/slugalizer/tree/master
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is great, but unfortunately this is of no use for any accented characters (Czech, Polish, other alphabets).
Example:
Stringex has very good implementation.
We have been using with good results Iconv for this in Czech context:
Even beter solution is this one from http://workingwithrails.com/person/12298-adam-cig-nek:
(See the “cig-nek” in URL? That should be “ciganek”. )
See http://forum.rubyonrails.cz/forums/1/topics/9?page=2#posts-227 for explanation if you can read Czech, otherwise write here.
For the purpose is the implementation certainly insufficient. In case of names (see above “cig-nek”) maybe downright insulting :)
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the train-wreck with the pre tags.
And more one thing to clarify the point: to me, Rails is above all about best practices in web development. Dropping letters from people’s names with accented chars, as you see on so many Rails-based websites (WWR, Slideshare, etc) is certainly not best practice.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for slugalizer
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails should be as lightweight as possible that’s why I think this implementation is great and is good for 80% ppl using it.
For the rest of us we could use plugins like slugarizer oder stringex, which are really awesome, too!
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. 80% rule applies here.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, for an English text, it’s almost 100% accurate. The problem with such implementation is that it promises some functionality which is in principle insufficient and for every non-English text broken.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remember we are not talking about L18N here. Even in the US and UK (is that what others regard as 80%?) there are people with names from different cultures. I don’t see why you would make any application so English-centric and not allow foreign names on it (or make them look ugly).
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think karmi hit the nail on the head – the documentation ’’’promises’’’ some functionality that is insufficient.
Perhaps a simple change of documentation to make a note about non-English characters not being parameterized properly is sufficient? Maybe a recommendation to use Slugalizer or some suitable plugin too.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can make something less dumb using the String#chars stuff we already have, then we should. Otherwise we can just update the documentation. I don’t want to depend on iconv.
FWIW, the irony of this changeset mangling david’s new home town isn’t lost on me ;)
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Precisely. It still hurts me that WWR drops two letters from my name so I end up being “karel-mina-k” there. Much more examples in almost every Rails application on the web.
I vote for the documentation caveat.
Moreover, as stated on Lighthouse, we use with very good results this little cryptic, but working code:
string.chars.normalize(:kd).to_s.gsub(/[^\x00-\x7F]/, ’’)Try this in
script/console
:> > “Malm\303\266”.chars.normalize(:kd).to_s.gsub(/[^\x00-\x7F]/, ’’)
> > => “Malmo”
(Note: Mac Terminal escapes non-ASCII)
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Karmi, why wouldn’t we change the parameterize implementation to use your example above?
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something like this: http://gist.github.com/10227
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gist appears to be determined to wreck the non-ascii characters in that patch, but you get the gist.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While stringex is huge, note that Slugalizer is a oneliner, if you ignore the whitespace-for-readability and validating the argument. With all that, it’s about ten short lines. Most of slugalizer.rb are tests.
I think Slugalizer strikes a good balance between lightweight and best practice.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NZKoz, that would go much better with accented chars, although I don’t think the first line of the method body is needed? Ie.:
(Note please that I am not the author of the code.)
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> … don’t think the first line of the method body is needed?
Eh, I am sorry. Apparently I need to slow down a bit to be able to read colorized (!) diff at least :)
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now handles accented characters – further discussion here: http://github.com/rails/rails/commit/1ddde91303883b47f2215779cf45d7008377bd0d#comments
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve got a plugin that does the same, but the string sanitizing seems better.
You should take a look at http://github.com/Bounga/acts_as_nice_url/
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounga: There have been more commits since this discussion. See my link above.
I see your plugin uses iconv. That has some issues. See the README of Slugalizer, linked above.
b8e8be8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounga: There have been more commits since this discussion. See my link above.
I see your plugin uses iconv. That has some issues. See the README of Slugalizer, linked above.