Implement basic ordering for emoji results #68
Conversation
| return { | ||
| uid: name, |
There was a problem hiding this comment.
This is the only way to get Alfred to respect the ordering that we feed it.
| } | ||
|
|
||
| // Prioritize emojis that were matched on the name rather than on keywords | ||
| return [...emojiNameResults, ...emojiKeywordResults] |
There was a problem hiding this comment.
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.
| @@ -72,12 +72,6 @@ test('applies modifier if possible', (t) => { | |||
| t.ok(Object.keys(found.items).length > 0) | |||
| }) | |||
|
|
|||
| test('enables uid', (t) => { | |||
There was a problem hiding this comment.
uids are not enabled anymore as it breaks custom ordering aka results scoring.
There was a problem hiding this comment.
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/
There was a problem hiding this comment.
@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).
There was a problem hiding this comment.
I think it is desired. I definitely rely upon it. Adding a toggle via an environment variable would be acceptable.
There was a problem hiding this comment.
@jsumners Good suggestion, I'll update the PR and ask for rereview once done.
|
Needs to be updated due to merged PR: #66 |
|
Reconciliating the changes was a big pain, and now coverage doesn't pass. I'm giving up and staying on my fork. |
|
@jsumners By the way, what would you think about reducing the coverage to e.g. 80%? I think it's a big deterrent to productivity and I don't think its brings significant benefits to aim for 100%. |
|
There needs to be a clear demonstration as to why 100% is not attainable. |
|
@jsumners Disagree. There needs to be a clear demonstration that introducing a 100% code coverage requirement is providing enough benefits that it justifies the significantly reduced productivity. 100% coverage encourages useless ad-hoc tests. Goodhart's law at play. I may do a hard fork and probably rewrite the code from the ground up because the 100% coverage requirement and the search design choices don't suit me. Also I believe I can have significantly better performances by reducing the warm-up time tremendously. |
Close #65
Emojis that match on name are on top VS emojis that match on keywords.
Before:
After:
