Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix short tokens in getEmojiByShortcode #90

Merged
merged 2 commits into from Dec 24, 2020
Merged

Conversation

nolanlawson
Copy link
Owner

Fixes #88

The issue here is that shortcodes like smiling_face_with_3_hearts contain a short string, 3, which is not indexed in the search tokens. We also have problems with shortcodes like v (:v:) where there are no usual shortcodes at all.

Of course the easiest solution here would be to create an IDB index on shortcodes. However, this seems like a wasteful use of disk space to me, as 1) getEmojiByShortcode() is not always used, and 2) all shortcodes become tokens that are indexed for searching anyway.

So I'm going to keep the current system of filtering the database using the shortcode broken up into search tokens, while also adding special cases for tokens like 3 or v (which requires a full database scan – reasonable IMO given that so few shortcodes are like this).

@nolanlawson
Copy link
Owner Author

In Chrome, even on 6x slowdown, the difference between the full DB scan and the fast lookup is not terrible (~6ms vs ~60ms, 10x slower). Given how rare a shortcode like v is (or a nonexistent shortcode), this seems fine to me.
Screenshot from 2020-12-24 13-36-50

In Firefox it's equally negligible, <10ms in both cases (no slowdown).

@nolanlawson nolanlawson merged commit 992ac10 into master Dec 24, 2020
@nolanlawson
Copy link
Owner Author

Well actually it can get pretty slow if it has to do a full scan because the shortcode doesn't exist. I may have to look into a batching cursor.

Chrome 6x slowdown on left, Firefox on right

Screenshot from 2020-12-24 13-50-29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getEmojiByShortcode('smiling_face_with_3_hearts') returns null
1 participant