Skip to content

Commit

Permalink
Improve the emoji extension (code review remarks)
Browse files Browse the repository at this point in the history
  • Loading branch information
mlaily committed Jan 16, 2020
1 parent 0a36382 commit aecdf21
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 60 deletions.
14 changes: 6 additions & 8 deletions src/Markdig/Extensions/Emoji/EmojiExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,19 @@ namespace Markdig.Extensions.Emoji
/// <seealso cref="Markdig.IMarkdownExtension" />
public class EmojiExtension : IMarkdownExtension
{
public EmojiExtension(bool enableSmiley = true, EmojiMapping customEmojiMapping = null)
{
EnableSmiley = enableSmiley;
CustomEmojiMapping = customEmojiMapping;
public EmojiExtension(EmojiMapping emojiMapping)
{
EmojiMapping = emojiMapping;
}

public bool EnableSmiley { get; set; }
public EmojiMapping CustomEmojiMapping { get; }

public EmojiMapping EmojiMapping { get; }

public void Setup(MarkdownPipelineBuilder pipeline)
{
if (!pipeline.InlineParsers.Contains<EmojiParser>())
{
// Insert the parser before any other parsers
pipeline.InlineParsers.Insert(0, new EmojiParser(EnableSmiley, CustomEmojiMapping));
pipeline.InlineParsers.Insert(0, new EmojiParser(EmojiMapping));
}
}

Expand Down
66 changes: 38 additions & 28 deletions src/Markdig/Extensions/Emoji/EmojiMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,18 @@ public class EmojiMapping
/// <summary>
/// The default emojis and smileys mapping.
/// </summary>
public static readonly EmojiMapping DefaultEmojiMapping = new EmojiMapping();
public static readonly EmojiMapping DefaultEmojiAndSmileyMapping = new EmojiMapping();

/// <summary>
/// The default emojis mapping, without smileys.
/// </summary>
public static readonly EmojiMapping DefaultEmojiOnlyMapping = new EmojiMapping(enableSmiley: false);

internal CompactPrefixTree<string> EmojiPrefixTree { get; }
internal CompactPrefixTree<string> EmojiSmileyPrefixTree { get; }
internal char[] EmojiOpeningCharacters { get; }
internal char[] EmojiSmileyOpeningCharacters { get; }
private static readonly Dictionary<string, string> _emptyDictionary = new Dictionary<string, string>();

internal CompactPrefixTree<string> PrefixTree { get; }

internal char[] OpeningCharacters { get; }

#region Emojis and Smileys

Expand Down Expand Up @@ -1729,21 +1735,29 @@ public static IDictionary<string, string> GetDefaultSmileyToEmoji()

#endregion

public EmojiMapping(IDictionary<string, string> emojiToUnicode = null, IDictionary<string, string> smileyToEmoji = null)
/// <summary>
/// Constructs a mapping for the default emojis and smileys.
/// </summary>
public EmojiMapping(bool enableSmiley = true)
: this(GetDefaultEmojiToUnicode(), enableSmiley ? GetDefaultSmileyToEmoji() : _emptyDictionary) { }

/// <summary>
/// Constructs a mapping from a dictionary of emojis to unicode, and a dictionary of smileys to emojis.
/// </summary>
public EmojiMapping(IDictionary<string, string> emojiToUnicode, IDictionary<string, string> smileyToEmoji)
{
if (emojiToUnicode == null)
emojiToUnicode = GetDefaultEmojiToUnicode();

if (smileyToEmoji == null)
smileyToEmoji = GetDefaultSmileyToEmoji();

// Build Emoji and Smiley CompactPrefixTree
throw new ArgumentNullException(nameof(emojiToUnicode));

EmojiPrefixTree = new CompactPrefixTree<string>(emojiToUnicode.Count, emojiToUnicode.Count, emojiToUnicode.Count);
if (smileyToEmoji == null)
throw new ArgumentNullException(nameof(smileyToEmoji));

int jointCount = emojiToUnicode.Count + smileyToEmoji.Count;
// Count * 2 seems to be a good fit for the data set
EmojiSmileyPrefixTree = new CompactPrefixTree<string>(jointCount, jointCount * 2, jointCount * 2);
// Build Emoji and Smiley CompactPrefixTree

int jointCount = emojiToUnicode.Count + smileyToEmoji.Count;

// Count * 2 seems to be a good fit for the data set
PrefixTree = new CompactPrefixTree<string>(jointCount, jointCount * 2, jointCount * 2);

// This is not the best data set for the prefix tree as it will have to check the first character linearly
// A work-around would require a bunch of substrings / removing the leading ':' from emojis, neither one is pretty
Expand All @@ -1753,32 +1767,28 @@ public EmojiMapping(IDictionary<string, string> emojiToUnicode = null, IDictiona

foreach (var emoji in emojiToUnicode)
{
EmojiPrefixTree.Add(emoji);
EmojiSmileyPrefixTree.Add(emoji);

if (string.IsNullOrEmpty(emoji.Key))
throw new ArgumentException("The dictionaries cannot contain null or empty keys", nameof(emojiToUnicode));

firstChars.Add(emoji.Key[0]);
}

EmojiOpeningCharacters = new List<char>(firstChars).ToArray();
PrefixTree.Add(emoji);
}

foreach (var smiley in smileyToEmoji)
{
{
if (string.IsNullOrEmpty(smiley.Key))
throw new ArgumentException("The dictionaries cannot contain null or empty keys", nameof(smileyToEmoji));

if (!emojiToUnicode.TryGetValue(smiley.Value, out string unicode))
throw new ArgumentException(string.Format("Invalid smiley target: {0} is not present in the emoji dictionary", smiley.Value));

if (string.IsNullOrEmpty(smiley.Key))
throw new ArgumentException("The dictionaries cannot contain null or empty keys", nameof(smileyToEmoji));

firstChars.Add(smiley.Key[0]);

if (!EmojiSmileyPrefixTree.TryAdd(smiley.Key, unicode))
if (!PrefixTree.TryAdd(smiley.Key, unicode))
throw new ArgumentException(string.Format("Smiley {0} is already present in the Emoji dictionary", smiley.Key));
}

EmojiSmileyOpeningCharacters = new List<char>(firstChars).ToArray();
OpeningCharacters = new List<char>(firstChars).ToArray();
}
}
}
25 changes: 6 additions & 19 deletions src/Markdig/Extensions/Emoji/EmojiParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,15 @@ namespace Markdig.Extensions.Emoji
/// <seealso cref="InlineParser" />
public class EmojiParser : InlineParser
{
private readonly EmojiMapping _emojiMapping;
private CompactPrefixTree<string> _emojiPrefixTree;
private readonly EmojiMapping _emojiMapping;

/// <summary>
/// Initializes a new instance of the <see cref="EmojiParser"/> class.
/// </summary>
public EmojiParser(bool enableSmiley = true, EmojiMapping emojiMapping = null)
{
EnableSmiley = enableSmiley;
_emojiMapping = emojiMapping ?? EmojiMapping.DefaultEmojiMapping;
OpeningCharacters = null;
}

/// <summary>
/// Gets or sets a boolean indicating whether to process smiley.
/// </summary>
public bool EnableSmiley { get; set; }

public override void Initialize()
{
OpeningCharacters = EnableSmiley ? _emojiMapping.EmojiSmileyOpeningCharacters : _emojiMapping.EmojiOpeningCharacters;
_emojiPrefixTree = EnableSmiley ? _emojiMapping.EmojiSmileyPrefixTree : _emojiMapping.EmojiPrefixTree;
public EmojiParser(EmojiMapping emojiMapping)
{
_emojiMapping = emojiMapping;
OpeningCharacters = _emojiMapping.OpeningCharacters;
}

public override bool Match(InlineProcessor processor, ref StringSlice slice)
Expand All @@ -47,7 +34,7 @@ public override bool Match(InlineProcessor processor, ref StringSlice slice)
}

// Try to match an emoji
if (!_emojiPrefixTree.TryMatchLongest(slice.Text, slice.Start, slice.Length, out KeyValuePair<string, string> match))
if (!_emojiMapping.PrefixTree.TryMatchLongest(slice.Text, slice.Start, slice.Length, out KeyValuePair<string, string> match))
{
return false;
}
Expand Down
24 changes: 19 additions & 5 deletions src/Markdig/MarkdownExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ public static MarkdownPipelineBuilder UseAdvancedExtensions(this MarkdownPipelin
.UseDiagrams()
.UseAutoLinks()
.UseGenericAttributes(); // Must be last as it is one parser that is modifying other parsers
}
}

/// <summary>
/// Uses this extension to enable autolinks from text `http://`, `https://`, `ftp://`, `mailto:`, `www.xxx.yyy`
/// </summary>
Expand Down Expand Up @@ -425,17 +425,31 @@ public static MarkdownPipelineBuilder UseGenericAttributes(this MarkdownPipeline
/// </summary>
/// <param name="pipeline">The pipeline.</param>
/// <param name="enableSmiley">Enable smiley in addition to Emoji, <c>true</c> by default.</param>
/// <param name="customEmojiMapping">Enable optional customization of the emojis and smileys mapping.</param>
/// <returns>The modified pipeline</returns>
public static MarkdownPipelineBuilder UseEmojiAndSmiley(this MarkdownPipelineBuilder pipeline, bool enableSmiley = true, EmojiMapping customEmojiMapping = null)
public static MarkdownPipelineBuilder UseEmojiAndSmiley(this MarkdownPipelineBuilder pipeline, bool enableSmiley = true)
{
if (!pipeline.Extensions.Contains<EmojiExtension>())
{
pipeline.Extensions.Add(new EmojiExtension(enableSmiley, customEmojiMapping));
var emojiMapping = enableSmiley ? EmojiMapping.DefaultEmojiAndSmileyMapping : EmojiMapping.DefaultEmojiOnlyMapping;
pipeline.Extensions.Add(new EmojiExtension(emojiMapping));
}
return pipeline;
}

/// <summary>
/// Uses the emoji and smiley extension.
/// </summary>
/// <param name="pipeline">The pipeline.</param>
/// <param name="customEmojiMapping">Enable customization of the emojis and smileys mapping.</param>
/// <returns>The modified pipeline</returns>
public static MarkdownPipelineBuilder UseEmojiAndSmiley(this MarkdownPipelineBuilder pipeline, EmojiMapping customEmojiMapping)
{
if (!pipeline.Extensions.Contains<EmojiExtension>())
{
pipeline.Extensions.Add(new EmojiExtension(customEmojiMapping));
}
return pipeline;
}
/// <summary>
/// Add rel=nofollow to all links rendered to HTML.
/// </summary>
Expand Down

0 comments on commit aecdf21

Please sign in to comment.