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.

Sign up for GitHub

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

Implement basic ordering for emoji results #68

Closed
wants to merge 4 commits into from
Closed

Implement basic ordering for emoji results #68

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1468cf5
Show emojis that match on name on top
devnoname120 Jan 27, 2022
3fc371d
Merge branch 'master' into order-search-results
devnoname120 Jan 27, 2022
1458933
Merge remote-tracking branch 'upstream/master' into order-search-results
devnoname120 Apr 2, 2022
713250e
Add two missing files
devnoname120 Apr 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions src/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ const getIconName = (emoji, name) => {
const alfredItem = (emoji, name) => {
const modifiedEmoji = addModifier(emoji, modifier)
const icon = getIconName(emoji, name)

// No `uid` property otherwise Alfred ignores the ordering of the list and uses its own
return {
uid: name,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only way to get Alfred to respect the ordering that we feed it.

title: name,
subtitle: `${verb} "${modifiedEmoji}" (${name}) ${preposition}`,
arg: modifiedEmoji,
Expand Down Expand Up @@ -84,11 +85,36 @@ const libHasEmoji = (name, term) => {
emojilib.lib[name].keywords.some((keyword) => keyword.includes(term))
}
const matches = (terms) => {
return emojiNames.filter((name) => {
return terms.every((term) => {
return name.includes(term) || libHasEmoji(name, term)
})
})
const emojiNameResults = [];
const emojiKeywordResults = [];

for (const emojiName of emojiNames) {
let hasNameMatch = false;
let hasMatch = true;

for (const term of terms) {
if (emojiName.includes(term)) {
hasNameMatch = true;
continue;
} else if (!libHasEmoji(emojiName, term)) {
hasMatch = false;
break;
}
}

if (!hasMatch) {
continue;
}

if (hasNameMatch) {
emojiNameResults.push(emojiName);
} else {
emojiKeywordResults.push(emojiName);
}
}

// Prioritize emojis that were matched on the name rather than on keywords
return [...emojiNameResults, ...emojiKeywordResults]
Copy link
Author

@devnoname120 devnoname120 Jan 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to have more advanced scoring (how many terms match on name VS keywords, prioritize matches on the start of the name), but I think it's a good first step.

}

// :thumbs up: => ['thumbs', 'up']
Expand Down
6 changes: 0 additions & 6 deletions test/search.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ test('applies modifier if possible', (t) => {
t.ok(Object.keys(found.items).length > 0)
})

test('enables uid', (t) => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uids are not enabled anymore as it breaks custom ordering aka results scoring.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's going to be a deal breaker. We return the UID so that Alfred will prioritize results based on usage heuristics.

https://www.alfredapp.com/help/workflows/inputs/script-filter/json/
image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsumners Is this actually wanted for emoji search results? In my experience with alfred-emoji it leads to poor-quality results + inconsistent results ordering.

This makes it hard to input emojis fast because the ordering changes regularly for a given input. You can't just type your usual keyword and press Return. Instead, you have to make sure that the top result is actually the one you want, rather than another result that jumped on top because you used it a lot (possibly in another query).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is desired. I definitely rely upon it. Adding a toggle via an environment variable would be acceptable.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsumners Good suggestion, I'll update the PR and ask for rereview once done.

t.plan(1)
const found = search('grimacing')
t.ok(found.items[0].uid === 'grimacing')
})

test('enables autocomplete', (t) => {
t.plan(1)
const found = search('think')
Expand Down