Classification issues

[chansen]

While working on an alternative emoji segmentation implementation I noticed three presentation classification issues in the Ragel grammar:

Bare Emoji_Modifier misclassified as text
A lone modifier has Emoji_Presentation=Yes and should be classified as is_emoji=true. However, since EMOJI_MODIFIER is not included in any_emoji, it falls through to text_run and is classified as is_emoji=false.

Bare Emoji_Modifier_Base misclassified as emoji
Nine codepoints in the EMOJI_MODIFIER_BASE category have Emoji_Presentation=No, because EMOJI_MODIFIER_BASE is listed directly in emoji_presentation, a bare occurrence is classified as is_emoji=true regardless of the codepoint's default presentation property.

Bare Regional_Indicator misclassified as text
A lone regional indicator has Emoji_Presentation=Yes and should be classified as is_emoji=true. However, since emoji_presentation only includes emoji_flag_sequence (a pair of regional indicators), a single regional indicator falls through to text_run and is incorrectly classified as is_emoji=false.

Unqualified keycap sequence not recognised
An unqualified keycap sequence (KEYCAP_BASE + COMBINING_ENCLOSING_KEYCAP, e.g. U+0023 U+20E3) is not matched by any emoji rule. emoji_keycap_sequence requires an intervening VS-15 or VS-16, so the two codepoints are scanned separately as individual text_run tokens. Unicode includes unqualified keycap sequences in emoji-test.txt and they appear in legacy content, so failing to recognise them as a single emoji sequence is likely incorrect.

[behdad]

Thanks. @drott can you take a look at this? If not, I'll try to find some time for it. Thanks.

[behdad]

@chansen Can you send a pull-request for this perhaps? If not, I'll see what I can do. I've been away from this code for 10+ years. Thanks.

[chansen]

I ended up not using the Ragel grammar in my C implementation, it didn't fit my requirement for a single-pass O(n) implementation, so I hand-rolled a minimal DFA instead.

The grammar changes should be fairly straightforward. The most invasive is splitting EMOJI_MODIFIER_BASE into two categories, mirroring the existing EMOJI_TEXT_PRESENTATION / EMOJI_EMOJI_PRESENTATION distinction:

EMOJI_MODIFIER_BASE_TEXT  = 3;   # Emoji_Presentation=No
EMOJI_MODIFIER_BASE_EMOJI = 16;  # Emoji_Presentation=Yes  (new category slot)

Then adjust any_emoji and emoji_presentation to use the split:

any_emoji = EMOJI_TEXT_PRESENTATION | EMOJI_EMOJI_PRESENTATION | KEYCAP_BASE |
  EMOJI_MODIFIER_BASE_TEXT | EMOJI_MODIFIER_BASE_EMOJI | TAG_BASE | EMOJI;

emoji_presentation = EMOJI_EMOJI_PRESENTATION | TAG_BASE | EMOJI_MODIFIER_BASE_EMOJI |
  EMOJI_MODIFIER | REGIONAL_INDICATOR |
  ...

The remaining fixes are more contained, adding EMOJI_MODIFIER and REGIONAL_INDICATOR to emoji_presentation, and adding an unqualified_keycap_sequence rule.

That said, I would not be comfortable making changes to the grammar without a test harness to verify correctness. The test units from my own implementation could serve as a starting point: emoji_scan_test.c.

What do you think?

[behdad]

Thanks. I wonder if we should deprecate this implementation and port to yours.

[chansen]

Another benefit of my implementation is that it requires fewer property trie lookups.

I've have submitted a PR with grammar changes and adding a test harness.