-
Notifications
You must be signed in to change notification settings - Fork 784
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
Prefer emoji.raw
without VARIATION SELECTOR 16
#164
Conversation
Did this PR miss a lot of the emojis that have text representations by default? For example I don't see this face in I'm using the emoji.json fail to map These emojis aren't being found by https://github.com/mathiasbynens/emoji-regex anymore, and even github doesn't appear to render it. |
@xPaw Different applications, OSs, and even fonts differ in how they decide to render characters that have no If you find a certain character that should be rendered as a text glyph if it has no |
Well my comment above has an emoji that renders as text on Windows 10. It's a bit weird to base If I want to use |
I don't understand the example in your comment above. You mention “frowning face”, and yet you use Line 71 in 37b14e4
It may appear weird now, but when this project started out, Windows and various popular Linux distributions had little to no native emoji support. Apple was and is still a leader in emoji support, meaning that their decisions influence emoji adoption and support on other platforms too.
If you want a character to be rendered a color emoji, then just always append |
You're right I used incorrect example. As it turns out it also depends on the font used (e.g. it shows in github comment, but not in diff), and that doesn't help.
Does github's |
It does ensure that all inserted emoji are always color emoji. For instance, typing GitHub's emoji completer gets all its information from gemoji. In this case, gemoji's TEXT_GLYPHS dictates that |
So I looked at this again. Do i understand correctly that the I made a diff of the map we generate: https://gist.github.com/xPaw/d3b3e1236b22f0fc19494dad279278dc Quickly looking at the diff, I see plenty of emojis that should have the variant selector (these emojis don't get detected by emoji-regex). There's also some emojis with male/female sign that seem to require the variant selector to be parsed/rendered correctly. From what I can tell, there's only a few emojis that don't need the variant selector (like |
Hmm I'm not sure what your diff represents or what the overarching problem is. I might be missing something. Just to backtrack a little bit and make sure we're on the same page:
I'm curius—what is the problem that you're experiencing with this library? Lets scope the problem around a single example character, to keep it simple. Where is this character and its unicode sequence present in gemoji? Where is it missing? |
That's what I assumed, and how I've been using it. The diff shows emoji map from before this PR, and after. There are plenty of emojis that no longer render as colored emoji, at least on Windows. Emojis like eye and speech bubble would be such examples (but pretty much most of the emojis in the diff don't render correctly). It worked fine before, but now it looks like TEXT_GLYPHS needs to be a way longer list. We use the JSON file, not the library it self, there's sadly no good source to get it from, and your map is good enough. |
Ah so this is on Windows! Thanks for the leads. I will look into it. I don't often test on Windows. As a temporary workaround, when you're generating |
That might be worse than before this PR, right now I simply haven't updated the map as there's no reason to until there are new emojis. Here's the generator script we use, it simply re-maps it into an object: https://github.com/thelounge/thelounge/blob/master/scripts/generate-emoji.js
Not just, because Thank you for looking. EDIT: Github also doesn't detect/wrap some of these as
|
I'm not familiar with emoji-regex. How is it related or affected by changes to gemoji? Also, which example characters does now emoji-regex not catch? |
The "emoji" unicode sequence is not detected as an actual emoji, even Github comments don't detect them as emojis and don't wrap them as
Pretty much most of the emojis that were affected by this PR (I think there's also a separate issue with male/female modifier not having the variant selector in ZWJ emojis). |
Here's a simple script to test againt emoji-regex (which really, just follows the emoji/unicode specification). Before this PR, it detected every single sequence as an emoji. After this PR, the follow emojis no longer get detected:
"use strict";
const got = require("got");
const emojiRegExp = require("emoji-regex")();
(async () => {
const response = await got(
"https://raw.githubusercontent.com/github/gemoji/master/db/emoji.json"
// "https://raw.githubusercontent.com/github/gemoji/22b920f8bd6c2e453832955fe9e13971b95772c5/db/emoji.json"
);
const emojiStrategy = JSON.parse(response.body);
for (const emoji of emojiStrategy) {
let match;
let found = false;
while ((match = emojiRegExp.exec(emoji.emoji))) {
found = true;
}
if (!found) {
console.log(emoji.description);
}
}
})(); |
@xPaw Thanks for arguing your case; I've reverted it https://github.com/github/gemoji/releases/tag/v4.0.0.rc1 I saw your point re: ever-expanding TEXT_GLYPHS and I didn't want to go down that route. |
Thank you! |
The
emoji.raw
value should preferrably be a shorter sequence without the\u{FE0F}
suffix.The only characters whose
.raw
value remains with the VARIATION SELECTOR 16 suffix are ones that would commonly render as text glyphs otherwise, or those that have variants with\u{FE0F}
appearing in multiple places.Fixes #163