yuya373 / emacs-slack

slack client for emacs
1.11k stars 117 forks source link

Emojis: default slack emojis (slack-emoji-master) are missing #520

Open chasecaleb opened 4 years ago

chasecaleb commented 4 years ago

Describe the bug emojify-user-emojis contains team-specific emojis from (oref team emoji-master) but does not have the default/built-in emojis from slack-emoji-master.

Team emojis are added to emojify-user-emojis here, within slack-download-emoji: https://github.com/yuya373/emacs-slack/blob/d53a57a18fb7034182c3d02503f937761e6a2a64/slack-emoji.el#L58-L62

Whereas there is no equivalent logic in slack-emoji-fetch-master-data (but there should be): https://github.com/yuya373/emacs-slack/blob/d53a57a18fb7034182c3d02503f937761e6a2a64/slack-emoji.el#L113-L133

To Reproduce Steps to reproduce the behavior:

  1. Open a slack room of any sort
  2. M-x slack-insert-emoji
  3. Type :woman-shr and attempt to complete (via tab or whatever, depending on your completion config)
  4. Result: no matches, but it should complete to :woman-shrugging:

Alternative steps:

  1. Open a slack room
  2. Find or send a message containing :woman-shrugging: (type it out by hand, since slack-insert-emoji doesn't recognize it)
  3. Result: :woman-shrugging: is incorrectly shown as text instead of replaced by an emoji

Backtrace Not applicable

Expected behavior Emojis from slack-emoji-master should be added to emojify-user-emojis so that they can be inserted via slack-insert-emoji as well as rendered as an image instead of text in messages.

Environment:

Additional context This is a slight tangent, but I noticed that emojis are added with style "github". Would it make sense to change it to e.g. "slack" instead? I think that would improve the UX since it would be more clear where the emojis are coming from, especially for people who use emojify.el for more than just slack.