#6923 Handle github emoji
closed
General
2018-10-30
2013-11-26
No
Emoji (e.g. 👍 thumbs up) are heavily used on some GH projects and would be good to carry over during import. However, the images are not licensed for us to redistribute https://github.com/github/gemoji/blob/master/LICENSE Maybe we could use https://github.com/Genshin/PhantomOpenEmoji if the mapping is straightforward. We also need to decide if we want an emoji-like feature throughout our markdown or handle this just at import.
It is really cool to have allura emoji support. Currently user need to copy and paste unicode of emoji to use emoji in markdown. How about adding something like https://pypi.org/project/emoji/
your ideas?
Hi..
See my draft work https://forge-allura.apache.org/u/shalithasuranga/allura/ci/6c2710d2a0da3959a70fe3d348ea9860833e0f5e/
Update - I pushed a new commit for commit message emoji support
What I did was adding another preprocessor class I guess that is the proper way rather than just replacing somewhere
If this looks good I will add some test cases and we can add
your idea?
The emoji library from pypi is BSD-licensed so that's good. And uses shortcode formats that are common on github, slack, etc.
I tried out your commit and it seems to work ok, but perhaps too broadly. If you have something in backticks or a code-block, those shouldn't be replaced. Like this I mean:
So it would have to understand markdown semantics. A little googling turned up a few libraries you could try that might do it https://facelessuser.github.io/pymdown-extensions/extensions/emoji/ and https://pypi.org/project/mdx_unimoji/ (not updated in many years though). Want to try those out?
Our initial (current) emoji support is twemoji js/css for display. A few concerns/ideas for development back then were: https://forge-allura.apache.org/p/allura/git/merge-requests/135/#4df8/4a2f/9b97
:smile:to 😄 (unicode) right as you type and not even use the short codes then. Not sure how easy or hard that would be, but it could be nice alternative way of doing this.Since the initial purpose of this ticket is handling imported markdown text, then my last sentence about maybe skipping shortcodes altogether would not be a good approach. (Still would be nice to have autocomplete suggestions though)
Hi.. Dave
+1 for pointing out above case. I also googled and found the links you mentioned and I tried those. extension in the first link requires markdown v2.6.0+ (see https://facelessuser.github.io/pymdown-extensions/installation/) but we have 2.2.1 and When updating there were some errors. And second one is not updated.
What I did was I wrote our own extension and this will not interfere with situation you mentioned
Commit is here https://forge-allura.apache.org/u/shalithasuranga/allura/ci/ff774b510721e1b5b3268b9fedf5fa1bdd6acd20/
Note that I have added only 👍 and :simile: emoji codes to extension
We will want to upgrade Markdown sooner or later anyway, so if the pymdown-extensions looks best, we can try to work through the Markdown upgrade first. I tried a while ago in commit [27c46d] and ran into issues then too though, so it would be some serious work.
That said, your extension seems like a good start. We would need a complete list of emoji and codes of course. Maybe from the
emojipackage you referenced earlier? And this kind of thing would need a variety of tests to make sure it works correctly (seems right in my manual testing).test_globals.pyhas a lot of existing markdown tests.Yes the upgrade makes many issues I found that inlinepatterns is changed from previous So LinkPattern, sanitize function like things were removed/changed So I also think doing upgrade to current version of py md brings lot of core changes.
I added more emojis from previous package diff is here . Advantage is if we write our own extension it may not affect with library upgrades and also we use unicodes not images so existing emoji js will take care.
I found that 4 cases failing there related to macro such as
allura.tests.test_globals.test_macro_memberseven for the last commit from upstream(without these changes) but those macros are working properly manually. I am not sure where is the issue. will checkYea test_globals does have some failures, actually depending on how it is run. If you are in the
Allurasubdirectory (where there is a setup.cfg file present) and run it there, they will pass. That can be quite confusing and is not ideal, but that's the way it is currently (it happens because the setup.cfg has a nosetests config entry in it - I don't know why a few of the tests only pass that way though).For the emoji list, would it be better to depend on the
emojipackage and import the list from there into our extension? I don't think we want to maintain a big list of emoji within Allura if something else can do that better. We would just have to make sure our version of twemoji.min.js supports all the emoji in the shortcodes list. The twemoji.min.js file doesn't have a version number in it, but it was added June 2016 so it might need to be updated to handle all the unicode emoji that theemojipackage does?Thanks I will check test cases thing and try. I was thinking to seperate emoji array too to another py file. but getting from package like https://pypi.org/project/emoji/ is great as you told since we will not include those emojis in to Allura source. I will verifly unicode vs twemoji support by writing sample script
Hi Dave
I checked this further. https://pypi.org/project/emoji/ + updated twemoji + our emoji extension will solve this. Here I have attached several files proving the work. Please check zip archive
Download here
But one issue was seems
32x32images are replaced with72x72images now.If it looks good I will add relevant test cases
See test work at patch6
Cool, good testing. Makes me think about native browser support for emoji, where twemoji isn't even need. I'll make a ticket for that and several other emoji ideas for next or later
This does look good, go ahead with test cases. That regex looks pretty crazy, but I figure its since some short codes have those characters in them. Tests with one or two of those might be good to include. Then I'll do a final review
Thanks for checking that. Yeah regex has crazy non-ascii characters too :D . Submitted a MR with the test cases and aso I have added support for commit messages. I will look the other tickets you mentioned above.
🎉
See here to look emoji in commit message
👍 It works 🚀 haha.. 😄