Added 'augment-en' script to pull keywords from platform data #226
Conversation
|
Un-drafting as an excuse to ping you @muan. 😁 |
|
Sorry! I will try to take some time to look at this. |
|
Sorry it took me so long. The other thing I'd note still is that I don't think this fixes #194. There is still a question for whether "penis" is an acceptable keyword for 🍆. I think that's worth of documenting. I think we have talked about this over the email for a bit a long time ago. I thought this PR was that actually, so I didn't read it sooner. Once again apologies for being absent-minded. So, just that review comment, and if the fixes 194 keyword can be taken out that'd be great. Or you can simply reopen afterwards. |
|
Also this feels like a minor version bump. What do you think? |
I was seeing this as answering that by defining
...meaning, for "penis" and 🍆, that'd be a no from me.
I think technically it would be... but I worry about adding in so much new data in a minor. Can I suggest a major just to play it safe? |
I think this library should always accept user defined keyword which was how most of the existing keywords came from. That was what I think was missing from CLDR therefore the existence of this library. Otherwise it seems like people can just bypass this library altogether by combining unicode emoji json with emoji platform data and get pretty much this, right? |
|
Ah gotcha, thanks, that's helpful. I realize now that you're right, this really doesn't fix #194. It completely sidesteps it and just adds a bunch of new keywords 😄! Removed the auto-close note.
Yes, exactly. IMO it'd be a lot easier for maintenance to stick with just that exact list of keywords, so politically fraught decisions like "does searching for |
JoshuaKGoldberg
changed the title
|
😐 accidentally force-pushed a removal of branch history... and now GitHub isn't letting me re-open this PR. Re-made this PR at #227. Sorry for the noise. |
|
Edit: and now I can. Very confusing. Maybe it was from adding the merge from |
| "fast-xml-parser": "^4.3.6", | ||
| "prettier": "^1.18.2", | ||
| "promptly": "^3.2.0", | ||
| "tape": "^5.1.1", | ||
| "unicode-emoji-json": "0.6.0" | ||
| } | ||
| }, | ||
| "node_modules/@graphql-typed-document-node/core": { |
There was a problem hiding this comment.
Aside: I don't love that adding emoji-platform-data as a dev dependency also transitively adds in the packages emojipedia depends on for its Node.js API. Seems silly. Tracked in JoshuaKGoldberg/emoji-platform-data#200.
|
👋 @muan sorry, I think I lost track here - are you waiting on me for anything? |
|
I am so extremely sorry about the delay. I have been terrible at emails. I agree with #226 (comment) and I'd rather if they aren't added at all. But uh, seems like it doesn't affect user experience and we can remove it at a latter date if that end up being annoying to maintain. |
|
Also please feel free to merge in the future. |
Does not fix #194, but somewhat sidesteps the issue by adding in a lot of keywords.
This is mostly the same strategy as #194 (comment).
scripts/augment-en.jspulls in all keywords and names from the Emojipedia/Unicode, GitHub, Fluent UI, and Twitter platforms viaemoji-platform-data. It filters out duplicate and unnecessary keywords, then pushes any new ones todist/emoji-*.json.Note that this PR adds a couple more filters, to keep the change a little smaller:
"health"as a part of"health_worker")_(e.g."big_grin"in 😆)"><"in 😆)In total, this augments 1728 emoji with a total of 5956 keywords.
Leaving this in draft for now as I haven't gotten approval for this strategy 😄un-drafted, with the same note. I won't be upset at all if it's just blanket denied. But it was a ton of fun to get to this PR!