zed icon indicating copy to clipboard operation
zed copied to clipboard

Support emoji shortcodes in chat

Open bennetbo opened this issue 1 year ago โ€ข 13 comments

Completes: https://github.com/zed-industries/zed/issues/7299

Suggestions

https://github.com/zed-industries/zed/assets/53836821/2a81ba89-4634-4d94-8370-6f76ff3e9403

Automatically replacing shortcodes without using the completions (only enabled when message_editor > auto_replace_emoji_shortcode is enabled in the settings):

https://github.com/zed-industries/zed/assets/53836821/10ef2b4b-c67b-4202-b958-332a37dc088e

Release Notes:

  • Added autocompletion for emojis in chat when typing emoji shortcodes (#7299).
  • Added support for automatically replacing emoji shortcodes in chat (e.g. typing ":wave:" will be converted to "๐Ÿ‘‹")

bennetbo avatar Feb 26 '24 22:02 bennetbo

Oh hellllll yes!

JosephTLyons avatar Feb 26 '24 23:02 JosephTLyons

Is it possible to have a way to not accept the suggestion?

I could think of cases where you might want to type :something: without it being converted into an emoji.

maxdeviant avatar Feb 26 '24 23:02 maxdeviant

Oh hellllll yes!

Haha so my assumption was correct that you were the one that mentioned something about wanting this feature in one of the first fireside hacks

bennetbo avatar Feb 26 '24 23:02 bennetbo

Oh hellllll yes!

Haha so my assumption was correct that you were the one that mentioned something about wanting this feature in one of the first fireside hacks

Yeah, that was likely me!

JosephTLyons avatar Feb 26 '24 23:02 JosephTLyons

Is it possible to have a way to not accept the suggestion?

I could think of cases where you might want to type :something: without it being converted into an emoji.

I don't think so, at least not in a straight forward way. You could type ":wav", then ":", then move the cursor and insert "e". So yeah, im not sure what the best behavior is there. Personally I think I would want that behavior in markdown files (channel notes) and definitely in the chat.

Edit: It would also have to be an explicit shortcode that exists that you're trying to type, e.g. :wave: would get replaced, but not :something:

Oops looks like GitHub doesn't let me type the : w a v e : above, and displays it as :wave: automatically

bennetbo avatar Feb 26 '24 23:02 bennetbo

Is it possible to have a way to not accept the suggestion? I could think of cases where you might want to type :something: without it being converted into an emoji.

I don't think so, at least not in a straight forward way. You could type ":wav", then ":", then move the cursor and insert "e". So yeah, im not sure what the best behavior is there. Personally I think I would want that behavior in markdown files (channel notes) and definitely in the chat.

Edit: It would also have to be an explicit shortcode that exists that you're trying to type, e.g. ๐Ÿ‘‹ would get replaced, but not :something:

Oops looks like GitHub doesn't let me type out the : w a v e : above, and displays it as ๐Ÿ‘‹ automatically

Yea, I think the difference (on GitHub, at least), is that they only replace emojis in the preview, while leaving the source unchanged.

Having an emoji inserted into the source without having a way to opt out of it feels wrong, to me (at least when working in a file; this behavior is ~probably okay in places like chat).

I was envisioning something more like this where we autosuggest emojis based on the shortcode, but it requires an actual action to accept the replacement (N.B. I haven't actually used Rocket, I'm just going off of what it looks like in their demo GIF).

maxdeviant avatar Feb 26 '24 23:02 maxdeviant

Is it possible to have a way to not accept the suggestion? I could think of cases where you might want to type :something: without it being converted into an emoji.

I don't think so, at least not in a straight forward way. You could type ":wav", then ":", then move the cursor and insert "e". So yeah, im not sure what the best behavior is there. Personally I think I would want that behavior in markdown files (channel notes) and definitely in the chat. Edit: It would also have to be an explicit shortcode that exists that you're trying to type, e.g. ๐Ÿ‘‹ would get replaced, but not :something: Oops looks like GitHub doesn't let me type out the : w a v e : above, and displays it as ๐Ÿ‘‹ automatically

Yea, I think the difference (on GitHub, at least), is that they only replace emojis in the preview, while leaving the source unchanged.

Having an emoji inserted into the source without having a way to opt out of it feels wrong, to me (at least when working in a file; this behavior is ~probably okay in places like chat).

I was envisioning something more like this where we autosuggest emojis based on the shortcode, but it requires an actual action to accept the replacement.

I definitely get that, I implemented something like rocket for the chat (see video above) but not for the other places. I guess we could switch to displaying these suggestions for markdown files as well and remove the auto replace stuff.

bennetbo avatar Feb 26 '24 23:02 bennetbo

I definitely get that, I implemented something like rocket for the chat (see video above) but not for the other places. I guess we could switch to displaying these suggestions for markdown files as well and remove the auto replace stuff.

I think that sounds like a good idea.

Maybe we can add the auto-replace behavior as an option (defaulting to false) if people want that, but it seems like a fairly opinionated choice to make the default when working in a buffer.

maxdeviant avatar Feb 26 '24 23:02 maxdeviant

Having an emoji inserted into the source without having a way to opt out of it feels wrong, to me (at least when working in a file; this behavior is ~probably okay in places like chat).

It most certainly is NOT okay in chat either. Some languages use colons like English uses apostrophes, and seeing things like "Zed:ssรค" ("in Zed" in Finnish, for example) consistently get sent as "Zed :worried: sรค " in some chat applications (Skype/Teams being some great examples) makes me want to shoot whoever came up with that logic.

jansol avatar Feb 27 '24 15:02 jansol

@bennetbo The code here seems fine, but let's address some of the feedback above:

  • We should only trigger with a space before the : to avoid the apostrophe's (and just not completing when people use punctuation: like this).
  • It doesn't seem to work in markdown files for me. I would like it in channel notes; but let's not do this in markdown files for now.

ConradIrwin avatar Mar 01 '24 23:03 ConradIrwin

@bennetbo The code here seems fine, but let's address some of the feedback above:

  • We should only trigger with a space before the : to avoid the apostrophe's (and just not completing when people use punctuation: like this).
  • It doesn't seem to work in markdown files for me. I would like it in channel notes; but let's not do this in markdown files for now.

Hey @ConradIrwin, thanks for the feedback. The first bullet point is a good suggestion. As for the second point: Im not sure what's not working for you, but I was planning to implement Marshall's suggestion (only showing emoji completions for now, no auto-replace), so it might not be relevant after the adjustments that need to be made.

I spent some time looking into how to implement Marshall's suggestion, here's what I found:

  • Currently the auto suggestion is only implemented for the message editor, which was quite easy to integrate using the CompletionProvider API
  • CompletionProvider is implemented for Model<Project>, which is used for all editors except the message editor
  • Therefore we cannot override the completion provider field inside editor.rs with a completion provider that suggests emoji's.
  • Idea: Introduce the ability to add multiple CompletionProvider's?

TLDR: Im not sure what's the best way to implement this, pairing on it (if possible) or suggestions would be great.

bennetbo avatar Mar 04 '24 16:03 bennetbo

@bennetbo I suggest we de-scope doing this in buffers completely for now. As the 90% use-case is in the chat, but the 90%-complexity is making it work with buffers, it seems like we're suffering priority inversion.

Happy to pair! https://calendly.com/conradirwin/pairing has some official slots, but I'm around most of this week if you have some time.

ConradIrwin avatar Mar 05 '24 00:03 ConradIrwin

@bennetbo I suggest we de-scope doing this in buffers completely for now. As the 90% use-case is in the chat, but the 90%-complexity is making it work with buffers, it seems like we're suffering priority inversion.

Happy to pair! https://calendly.com/conradirwin/pairing has some official slots, but I'm around most of this week if you have some time.

Sounds like a good idea! I removed the markdown handling and also the language configuration stuff. I added a new setting which can be configured for the message editor (auto_replace_emoji_shortcode) which when enabled, replaces stuff like :wave: with the corresponding emoji (Default is false).

We should only trigger with a space before the : to avoid the apostrophe's (and just not completing when people use punctuation: like this).

If I understood that correctly, this should be correctly handled now.

Would still be great to talk through the changes and see if my approach is correct. I'll try to join in later today if you are around.

bennetbo avatar Mar 05 '24 11:03 bennetbo

This looks good! I've unilaterally changed the default so that typing :wave: does the same as it does in Slack/Discord. Will merge when tests pass.

ConradIrwin avatar Mar 07 '24 02:03 ConradIrwin