Emojis and smileys customization

[mlaily]

Hello,

I found out while updating my nuget packages that emojis and smileys customization was not possible anymore (#308).

This PR is an attempt to bring this feature back, while keeping the new performance optimizations in place.

Please note that I'm not much used to caring about allocations, so it's possible I introduced useless ones. Feel free to point them to me so I can fix them.

---

To summarize the changes:

• I introduced a new immutable EmojiMapping class, wrapping the creation of the compact prefix trees.
• EmojiMapping can be constructed from custom dictionaries (it uses the default emojis and smileys otherwise).
• EmojiMapping exposes two static public methods returning the default emojis and smileys dictionaries, so that they can be used as a base to create a custom EmojiMapping instance.
• EmojiParser takes an EmojiMapping instance, and uses it directly to parse emojis and smileys.

The compact prefix trees creation is mostly the same as before. I only added new safety checks, since the input can be provided externally.

I also added some unit tests ensuring the basic use-cases work as expected.

[MihaZupan]

If we are adding support to customize the emoji set, I don't think having an EnableSmiley flag on EmojiParser makes sense anymore. I would still keep it in the UseEmojiAndSmiley and select the EmojiMapping to pass to the parser there.

EmojiMapping currently represents two possible mappings (smiley or not), I would prefer it only represents one and we instead have two default properties.

[MihaZupan]

Thank you for this, in general LGTM other than some implementation details mentioned above.

[mlaily]

Thanks for the review.

If we are adding support to customize the emoji set, I don't think having an EnableSmiley flag on EmojiParser makes sense anymore. I would still keep it in the UseEmojiAndSmiley and select the EmojiMapping to pass to the parser there.

My understanding is that it's currently possible to change the value of EmojiParser.EnableSmiley dynamically, and it should work. What you suggest would make this impossible, right? (Just checking. I'm not against it)

Also, what would UseEmojiAndSmiley look like? one override that takes a bool enableSmiley and uses the default mappings, and another one that takes just an EmojiMapping that can be customized?

EmojiMapping currently represents two possible mappings (smiley or not), I would prefer it only represents one and we instead have two default properties.

So an EmojiMapping would instead be an optimized representation of a dictionary, containing only a compact prefix tree and an array of opening characters?
Maybe its name should change then?

What would be the two default properties ? One just for the emojis, and the second for both the emojis and the smileys?

[MihaZupan]

change the value of EmojiParser.EnableSmiley dynamically

No. It can technically be changed between the ctor and first use, but that was to allow for customization before. Now we don't need to wait for initialize, that work can be done in the ctor.

In general, parsers should be immutable and thread-safe, to allow for Pipeline reuse to be safe.

Also, what would UseEmojiAndSmiley look like? one override that takes a bool enableSmiley and uses the default mappings, and another one that takes just an EmojiMapping that can be customized?

That was the idea. The one accepting bool would call the one accepting EmojiMapping directly.

Maybe its name should change then?

I think it's still a good name, no hard preference tho

What would be the two default properties ? One just for the emojis, and the second for both the emojis and the smileys?

Yes

[mlaily]

I'm working on the changes, but I also have another question: I don't think the terminology used in the extension is correct.

Since I'm introducing new properties, I would like to use this as an opportunity to use a better terminology.

Specifically, I don't think "emoji" is used properly. I think "emoji" should be used to talk about the unicode representation.
To replace the current meaning in the extension, I suggest using "shortcode" instead. This seem to be the widely used term to describe things like :confused:. https://emojipedia.org/shortcodes/

I'm also not a big fan of "smiley" to describe a face made of text, but I don't have anything better to suggest...

What do you think?

I would make these changes in a separate commit at the end...

[MihaZupan]

The naming change LGTM, I can't really think of a different name for smiley.

[mlaily]

That's alright, thank you :)

[MihaZupan]

Well that's an awful git merge diff, sorry. Looks like the web editor replaced all \n with \r\n

(If you want, you can force-push my commit off your branch - I didn't know that resolving the merge conflict in the web editor would push to your branch, i assumed that it would open a PR against it)

[mlaily]

I rebased my branch on top of the markdig master. The only conflict I had was the changelog.

If for whatever reason you don't like that, tell me and I'll revert the rebase, and do a merge into my branch instead.

[xoofx]

Thanks for that!

[mlaily]

Thank you for your wonderful work on open source projects!

[xoofx]

Thank you for your wonderful work on open source projects!

I forgot to thank also @MihaZupan who is helping a lot to keep this project updated. Thank you again for reviewing this PR @MihaZupan !