-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
Add compatibility with gemoji v2 #129
Conversation
Cool, I think thats a good idea. |
Could you add a comment and a link back to this pr above the feature detect? I'd be fine with also deprecating gemoji 1.x for the 1.9 release, and move towards gemoji 2.x in a few months when we bump this gem to a 2.0 release. We can add another Travis CI gemfile to make sure we test both gemoji's cc @simeonwillbanks |
👍 for multiple gemfiles. We should try them in #126. |
Allows subclasses to augment: - EmojiFilter.emoji_names - EmojiFilter#emoji_pattern - EmojiFilter#emoji_filename(name)
Done. Also refactored the implementation a little bit, so check the diff again. Yeah multiple Gemfiles is a good idea after we fix CI. I can do that in a separate pull. |
@@ -74,6 +71,35 @@ def asset_path(name) | |||
def emoji_url(name) | |||
File.join(asset_root, asset_path(name)) | |||
end | |||
|
|||
# Build a regexp that matches all valid :emoji: names. | |||
def self.emoji_pattern |
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.
Why'd you refactor EmojiPattern
into methods? 👍 the interface, and maybe we can refactor other filters with Regexp constants? Thanks!
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.
Ah. You reference emoji_names
. 😄
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.
Also, one can now override the method in a subclass 😉
@mislav merged this. let me know if you make another PR with multiple gemfiles for testing this. |
This adds compatibility with upcoming gemoji v2 release that has an API completely incompatible with the previous one. I opted for supporting both APIs here because it was easy to do feature detection and so that html-pipeline users can update this library without having to necessarily upgrade gemoji in their apps.
The Gemfile and tests are still configured to use gemoji v1.x for the time being since test output (image names, to be precise) can be different when running against gemoji v2.
/cc @aroben @josh