Skip to content
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

Better support for custom emoji source path #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Better support for custom emoji source path #99

wants to merge 1 commit into from

Conversation

MahdiBaghbani
Copy link

@MahdiBaghbani MahdiBaghbani commented Sep 10, 2019

Hi
Let's start with some history, actually your history ...

#73 (comment)
#51 (comment)

as you see people have problems with something ridiculous called "emoji" directory appended to every thing by default and without any possible way to disable it 👎 even with this little (actually nothing) option:

emoji:
   src: "something/that/doesn't/matter/at/all/because/jemoji/will/append/'emoji'/"
# FACT: no documentation! users have to see a broken link and try to guess
# what the hell is wrong!

I went into this problem too (are you surprised?), I googled and then read your entire issue list only to find out this problem existed for 3 years and you just said:

#73 (comment)

after this reply I understood that no maintainer gives a damn about this problem. So I moved on to fix this by myself (I had 0% knowledge of Ruby this morning, I might have 5% now).

Now lets see what causes this PAIN IN THE ASS:

filter_with_emoji function is the key function here:

      # Public: Create or fetch the filter for the given {{src}} asset root.
      #
      # src - the asset root URL (e.g. https://github.githubassets.com/images/icons/)
      #
      # Returns an HTML::Pipeline instance for the given asset root.
      def filter_with_emoji(src)
        filters[src] ||= HTML::Pipeline.new([
          HTML::Pipeline::EmojiFilter,
        ], :asset_root => src, :img_attrs => { :align => nil })
      end

filter_with_emoji uses html pipeline gem, the "emoji" directory is appended by this module
file: https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/emoji_filter.rb

# frozen_string_literal: true

require 'cgi'
HTML::Pipeline.require_dependency('gemoji', 'EmojiFilter')

module HTML
  class Pipeline
    # HTML filter that replaces :emoji: with images.
    #
    # Context:
    #   :asset_root (required) - base url to link to emoji sprite
    #   :asset_path (optional) - url path to link to emoji sprite. :file_name can be used as a placeholder for the sprite file name. If no asset_path is set "emoji/:file_name" is used.
    #   :ignored_ancestor_tags (optional) - Tags to stop the emojification. Node has matched ancestor HTML tags will not be emojified. Default to pre, code, and tt tags. Extra tags please pass in the form of array, e.g., %w(blockquote summary).
    #   :img_attrs (optional) - Attributes for generated img tag. E.g. Pass { "draggble" => true, "height" => nil } to set draggable attribute to "true" and clear height attribute of generated img tag.

if you didn't/can't find your sins (yes, its your mistake) in the above code, let me guide you:

:asset_path (optional) - url path to link to emoji sprite. :file_name can be used as a placeholder

I guess when you were busy writing jemoji, you saw this line and told yourselves:

"It's optional and if we don't give this argument to the pipeline function, nothing would possibly go wrong"

😄 YEAH NOTHING ...
Exceptionally brilliant way of reading source docs, if it's optional then why should I even bother myself looking at what it does?

Let's see what's wrong:

      # The url path to link emoji sprites
      #
      # :file_name can be used in the asset_path as a placeholder for the sprite file name. If no asset_path is set in the context "emoji/:file_name" is used.
      # Returns the context's asset_path or the default path if no context asset_path is given.
      def asset_path(name)
        if context[:asset_path]
          context[:asset_path].gsub(':file_name', emoji_filename(name))
        else
          File.join('emoji', emoji_filename(name))
        end
      end

now you see this right? if you don't provide the :asseth_path, pipeline will append "emoji" to filenames,
and your brilliant way for "Customizing" as explained in README.md is useless as hell because no one can simply serve any emoji from a CDN like https://cdn.jsdelivr.net/emojione/assets/3.0/png/32/1f600.png

Now, I (some random guy with 0% Ruby knowledge) had fixed this in 1 day (the same thing you didn't give a damn for 3 years).

I have fixed it in a way that it won't break current installations, and let people get rid of "emoji" directory, by a simple new keyword called "asset":

emoji:
   src: "something/that/does/matter/because/jemoji/won't/append/'emoji'/"
   asset: "and/this/path/will/be/appended/instead"
# emojis will serve from this URL:
# "something/that/does/matter/because/jemoji/won't/append/'emoji'/and/this/path/will/be/appended/instead"

backward compatibility ensures if user doesn't define "asset", or "emoji" at all, "emoji" directory will appended and this won't break anything (something you should know about me is that I don't like testing, so if I were you, I wouldn't trust this)

and I didn't touch README.md because I suck at English, someone else with proper grammar understanding should edit and update it.

Best,
Mahdi

(Oh, I was about to forget apologizing for being a bit salty and flaming you in this message. Sorry)

this commit fixes the "emoji" directory appended
by default to the user pre-defined source in _config.yml
@tkrunning
Copy link

This would be a great addition!

@MahdiBaghbani
Copy link
Author

Hi @tkrunning, I write this about two months ago and seems nobody from Jemoji team is willing to respond :) so I have been re inventing the wheel for a while now and if you are interested in using emojis with custom paths and even custom extensions like .svg or .jpg, please check out this repository: https://github.com/azadeh-afzar/OpenMoji-Jekyll-Plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants