fix: convert all emoji shortnames in a message, not just the last#1316
Open
yash113gadia wants to merge 1 commit into
Open
fix: convert all emoji shortnames in a message, not just the last#1316yash113gadia wants to merge 1 commit into
yash113gadia wants to merge 1 commit into
Conversation
parseEmoji only ever converted a single shortname: it collected every `:shortname:` match but then picked the last one and ran a single `String.replace`, which swaps the first occurrence of that match. So a message with multiple shortnames left all but one as raw text, e.g. ":smile: and :heart:" rendered as ":smile: and ❤️", and ":tada: foo :tada:" converted the wrong occurrence. This is most visible on pasted text and on attachment captions (AttachmentPreview parses the whole description once on submit), since incremental typing happened to convert each shortname as it completed. emoji-toolkit's shortnameToUnicode already converts every shortname in a string and leaves unknown shortnames and plain text untouched, so the hand-rolled match/replace is both buggy and unnecessary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1315
Problem
parseEmojicollected every:shortname:match but then picked only the last one and ran a singleString.replace, which swaps the first occurrence of that match. As a result a message with multiple shortnames left all but one as raw text:":smile: and :heart:"→":smile: and ❤️"(first emoji not converted)":tada: foo :tada:"→"🎉 foo :tada:"(wrong occurrence converted)Most visible on pasted text and on attachment captions (
AttachmentPreviewparses the whole description once on submit). Incremental typing masked it because each shortname was converted as it completed.Fix
emoji-toolkit'sshortnameToUnicode()already converts every shortname in a string and leaves unknown shortnames and plain text untouched, so the hand-rolled match/replace is both buggy and unnecessary:Verification
Checked against
emoji-toolkit@9.0.1(the version this package depends on)::smile: and :heart::smile: and ❤️😄 and ❤️:tada: foo :tada:🎉 foo :tada:🎉 foo 🎉:not_real_xyz: :smile::not_real_xyz: 😄:not_real_xyz: 😄no emoji herePlain text and unknown shortnames are preserved.