#6923 Handle github emoji

v1.10.0
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.

Related

Tickets: #8250

Discussion

  • Shalitha Suranga

    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?

     
  • Shalitha Suranga

    • status: open --> in-progress
     
  • Dave Brondsema

    Dave Brondsema - 2018-10-16

    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:

    `:smile:`
    or
    ```
    :smile:
    ```
    

    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

    • other outputs like email & rss feeds & apis - these seem good
    • having autocomplete suggestions with CodeMirror's showHint functionality would be a logical next step. Otherwise people will have to know the names of all the emoji. Implementing this would also could give us the option of changing :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.
     
    • Dave Brondsema

      Dave Brondsema - 2018-10-16

      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)

       
  • Shalitha Suranga

    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/

     
    • Shalitha Suranga

      Note that I have added only 👍 and :simile: emoji codes to extension

       
      • Dave Brondsema

        Dave Brondsema - 2018-10-17

        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 emoji package 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.py has a lot of existing markdown tests.

         
  • Shalitha Suranga

    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_members even 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 check

     
    • Dave Brondsema

      Dave Brondsema - 2018-10-18

      Yea test_globals does have some failures, actually depending on how it is run. If you are in the Allura subdirectory (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 emoji package 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 the emoji package does?

       
  • Shalitha Suranga

    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

     
  • Shalitha Suranga

    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 32x32 images are replaced with 72x72 images now.

    If it looks good I will add relevant test cases

    See test work at patch6

     
    • Dave Brondsema

      Dave Brondsema - 2018-10-22

      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

       
  • Dave Brondsema

    Dave Brondsema - 2018-10-22
    • labels: github, import --> github, import, emoji
     
  • Shalitha Suranga

    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.

     
  • Dave Brondsema

    Dave Brondsema - 2018-10-23
    • status: in-progress --> closed
    • assigned_to: Shalitha Suranga
    • Reviewer: Dave Brondsema
     
  • Dave Brondsema

    Dave Brondsema - 2018-10-23

    🎉

     
    • Shalitha Suranga

      See here to look emoji in commit message

       
  • Shalitha Suranga

    👍 It works 🚀 haha.. 😄

     
  • Dave Brondsema

    Dave Brondsema - 2018-10-30
    • Milestone: unreleased --> v1.10.0
     

Log in to post a comment.