iamcal / emoji-data

Easy to parse data and spritesheets for emoji
MIT License
2.57k stars 306 forks source link

update Emojione assets to latest Emojione version #132

Closed oriolbcn closed 6 years ago

oriolbcn commented 6 years ago

Fixes https://github.com/iamcal/emoji-data/issues/85

I have updated the Emojione related assets to the latest Emojione version, also adding all previously missing images (now all has_img_emojione are true 😄).

First off, I wanted to thank and congratulate you on the magnific work done, all the build process is elegantly created and easy to use. Using all the scripts and everything it's been easy to do this (except one part that I struggled with and I detail below). Cheers!

In the process of doing the update I have done some more necessary changes that I detail here.

Emojione build changes

Starting from Emojione 3.0, they have separated all assets into a dedicated repo, so I added a reference to the repo (submodule) from build/emojione

Also, I have adapted the script that gets the images to reference the new directory and done another change (which took me a while to figure out, that's the thing I struggled with) to transform the filenames in emojine-assets to fully qualified names (ZWJ sequences).

The thing is that emojione has named the files with their short form, for example 🏋🏿‍♀️ is named 1f3cb-1f3ff-2640.png (missing the optional 200d and fe03 characters). BUT in the catalog we build we search only by the fully qualified name. Fortunately for us, Emojione provides a mapping indicating the fully qualified name for each file. What I have done is to use this mapping to rename the files copied to this repo in their fully qualified form.

build_image changes

When building the images I had a problem with the pipes system, I think it was causing some sort of overflow issue in my machine (Mac OS), so I changed to the "normal" form of calling montage by passing the full ordered list of files to the command. I think this is easier to understand and more portable, but I may be perfectly missing something here.

bash script changes

Since I am using Mac, stat -c was not working since the syntax is different, I just extracted that into a method that executes the correct command depending on whether it's run on Mac or Linux.

other things

I have already done the whole build process:

oriolbcn commented 6 years ago

I added a new commit to the PR to add also the non_qualified attribute to skin_variations, because iOS and Android sometimes output the Emojis without the \uFE0F character.

vanniktech commented 6 years ago

What's stopping this from getting merged?

HarshitOnGitHub commented 6 years ago

Hey! Firstly, I would like to thank everyone involved in this project for their amazing work. We @zulip use this project for building our emoji infrastructure. We were in the process of upgrading to the latest release of this project when we encountered the issue related to this PR. This is currently a release blocker for us. Any update on when this will get merged? Thanks!

vanniktech commented 6 years ago

@iamcal friendly ping - could we get a review here?

iamcal commented 6 years ago

this is great - thanks for all the work that went into this.

the pipe stuff might be due to php changes, assuming you're using a newer version than i was

iamcal commented 6 years ago

v4.0.4 is pushing to npm now