Update to use emojilib v3 #60
Conversation
This will fix jsumners#44, presumably just because the data source is updated.
|
...to elaborate on that, my understanding is that in the time since emojilib v2.4.0 was released back in 2018, platform vendors adjusted how they represent some emoji. Specifically, a bunch of emoji which were specced as gender-neutral but which had always been displayed as a woman got changed to actually display gender-neutral and require the gender codepoint for the female version. This trips up emojilib 2.4, which only included the gender-codepoint for the male versions of those emoji. There's no release of emojilib inbetween 2.4 and 3.0, so the only way to get the updated data that fixes #44 is to do the full major-version upgrade. |
|
Thank you for digging in to this. I'll review it as soon as I am able. |
|
I have pulled the branch down, run the build, installed it in Alfred, and everything seems to be good. I'll work on merging and releasing soon. |
|
Searches for "-1" and "+1" are not returning "👎" and "👍" respectively with this PR. |
|
Hm... that'd be because emojilib doesn't include them as keywords. Technically it didn't in 2.4 either, but it had "+1" and "-1" as the names of the emoji -- so it's the switch to the emoji itself being the object-key that's effectively removing it. I could monkey-patch this into the data, or I could try submitting a pull request to emojilib for it. |
|
I made an emojilib pull request for it. |
|
That pull request got merged, and I have verified that doing
|
|
That said, I just noticed that I missed replacing a use of |
Also, had a reference to emoji.char which no longer exists.
|
Found another one: |
|
Another one, then! |
|
🤔 given comment muan/emojilib#195 (comment) I wonder if this PR is complete. Maybe we need to automate backfilling some items during the workflow build. |
|
I'll argue the point in the PR a bit. If it doesn't get merged, yeah, maintaining some separate monkeypatching data for emojilib is probably needed. |
|
Found another one: 100 does not map to "hundred points". I think we need to step back and rework our build to handle these occurrences. |
|
I think that submitting improvements to upstream libraries is a good idea in general, but you're right that it's going to fall apart as soon as we hit something upstream doesn't want to merge. I'd be inclined to do something that's past the build step as well -- let people configure it like they can the skin tone. Custom aliases for your own personal usage, that won't necessarily line up with how others think of an emoji. |
|
I haven't looked too deeply at this in quite some time, but if I remember correctly, the aliases were moved to a sibling package. We should just iterate everything and build out a new tree with those aliases. |
|
That's what this patch is already doing, unfortunately. The issues we've been seeing are 💯 emoji whose old names weren't transferred over. (Because the migration switched from names as keys to the emoji themselves, and it was spotty about moving those old name-keys into the keywords package.) |
|
👋 ! |
|
I have not had time to get back to this. I want to dig in and see if I can build upon @kemayo's excellent work in order to cover all of these edge cases we have been uncovering. But I'm still in PT from https://jrfom.com/posts/2021/09/03/a-bike-crash/ and also buried under a whole mountain of GitHub notifications for work and OSS projects. If someone else is able to come up with a solution to the issues, that'd be great. |
|
Seems fair! I've not had any free time lately to keep improving it lately either, for sure. |
There were a lot of data structure changes.
I mostly did this because it will fix #44.