Skip to content

Commit 4a5e610

Browse files
gnpriceshivanshsharma13
gnprice
authored and
shivanshsharma13
committed
emoji: Rank by quality of match (exact, prefix, other)
Fixes part of zulip#1068.
1 parent b0257c0 commit 4a5e610

File tree

2 files changed

+167
-56
lines changed

2 files changed

+167
-56
lines changed

‎lib/model/emoji.dart

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,9 @@ class EmojiStoreImpl with EmojiStore {
223223
}
224224

225225
List<EmojiCandidate> _generateAllCandidates() {
226+
// See also [EmojiAutocompleteQuery._rankResult];
227+
// that ranking takes precedence over the order of this list.
228+
//
226229
// Compare `emoji_picker.rebuild_catalog` in Zulip web;
227230
// `composebox_typeahead.update_emoji_data` which receives its output;
228231
// and `emoji.update_emojis` which builds part of its input.
@@ -321,20 +324,30 @@ class EmojiStoreImpl with EmojiStore {
321324
}
322325

323326
/// The quality of an emoji's match to an autocomplete query.
324-
///
325-
/// (Rather vacuous for the moment; this structure will
326-
/// gain more substance in an upcoming commit.)
327327
enum EmojiMatchQuality {
328-
match;
328+
/// The query matches the whole emoji name (or the literal emoji itself).
329+
exact,
330+
331+
/// The query matches a prefix of the emoji name, but not the whole name.
332+
prefix,
333+
334+
/// The query matches somewhere in the emoji name, but not at the start.
335+
other;
329336

330337
/// The best possible quality of match.
331-
static const best = match;
338+
static const best = exact;
332339

333340
/// The better of the two given qualities of match,
334341
/// where null represents no match at all.
335342
static EmojiMatchQuality? bestOf(EmojiMatchQuality? a, EmojiMatchQuality? b) {
336343
if (b == null) return a;
337-
return b;
344+
if (a == null) return b;
345+
return compare(a, b) <= 0 ? a : b;
346+
}
347+
348+
/// Comparator that puts better matches first.
349+
static int compare(EmojiMatchQuality a, EmojiMatchQuality b) {
350+
return Enum.compareByIndex(a, b);
338351
}
339352
}
340353

@@ -404,11 +417,11 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
404417
// Compare get_emoji_matcher in Zulip web:shared/src/typeahead.ts .
405418
@visibleForTesting
406419
EmojiMatchQuality? match(EmojiCandidate candidate) {
407-
if (_adjusted == '') return EmojiMatchQuality.match;
420+
if (_adjusted == '') return EmojiMatchQuality.prefix;
408421

409422
if (candidate.emojiDisplay case UnicodeEmojiDisplay(:var emojiUnicode)) {
410423
if (_adjusted == emojiUnicode) {
411-
return EmojiMatchQuality.match;
424+
return EmojiMatchQuality.exact;
412425
}
413426
}
414427

@@ -421,13 +434,20 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
421434
}
422435

423436
EmojiMatchQuality? _matchName(String emojiName) {
424-
return _nameMatches(emojiName) ? EmojiMatchQuality.match : null;
425-
}
437+
// Compare query_matches_string_in_order in Zulip web:shared/src/typeahead.ts
438+
// for a Boolean version of this logic (match vs. no match),
439+
// and triage_raw in the same file web:shared/src/typeahead.ts
440+
// for the finer distinctions.
441+
// See also commentary in [_rankResult].
426442

427-
// Compare query_matches_string_in_order in Zulip web:shared/src/typeahead.ts .
428-
bool _nameMatches(String emojiName) {
429443
// TODO(#1067) this assumes emojiName is already lower-case (and no diacritics)
444+
if (emojiName == _adjusted) return EmojiMatchQuality.exact;
445+
if (emojiName.startsWith(_adjusted)) return EmojiMatchQuality.prefix;
446+
if (_nameMatches(emojiName)) return EmojiMatchQuality.other;
447+
return null;
448+
}
430449

450+
bool _nameMatches(String emojiName) {
431451
if (!_adjusted.contains(_separator)) {
432452
// If the query is a single token (doesn't contain a separator),
433453
// the match can be anywhere in the string.
@@ -438,21 +458,46 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
438458
// require the match to start at the start of a token.
439459
// (E.g. for 'ab_cd_ef', query could be 'ab_c' or 'cd_ef',
440460
// but not 'b_cd_ef'.)
441-
return emojiName.startsWith(_adjusted)
442-
|| emojiName.contains(_sepAdjusted);
461+
assert(!emojiName.startsWith(_adjusted)); // checked before calling this method
462+
return emojiName.contains(_sepAdjusted);
443463
}
444464

445465
/// A measure of the result's quality in the context of the query,
446466
/// ranked from 0 (best) to one less than [_numResultRanks].
447467
static int _rankResult(EmojiMatchQuality matchQuality) {
448-
// TODO(#1068): rank emoji results (popular, realm, other; exact match, prefix, other)
468+
// See also [EmojiStoreImpl._generateAllCandidates];
469+
// emoji which this function ranks equally
470+
// will appear in the order they were put in by that method.
471+
//
472+
// Compare sort_emojis in Zulip web:
473+
// https://github.com/zulip/zulip/blob/83a121c7e/web/shared/src/typeahead.ts#L322-L382
474+
//
475+
// Behavior differences we should or might copy, TODO(#1068):
476+
// * Web ranks popular emoji > custom emoji > others; we don't yet.
477+
// * Web ranks matches starting at a word boundary ahead of
478+
// other non-prefix matches; we don't yet.
479+
// * Web ranks each name of a Unicode emoji separately.
480+
//
481+
// Behavior differences that web should probably fix, TODO(web):
482+
// * Web starts with only case-sensitive exact matches ("perfect matches"),
483+
// and puts case-insensitive exact matches just ahead of prefix matches;
484+
// it also distinguishes prefix matches by case-sensitive vs. not.
485+
// We use case-insensitive matches throughout;
486+
// case seems unhelpful for emoji search.
487+
// * Web suppresses Unicode emoji names shadowed by a realm emoji
488+
// only if the latter is also a match for the query. That mostly works,
489+
// because emoji with the same name will mostly both match or both not;
490+
// but it breaks if the Unicode emoji was a literal match.
491+
449492
return switch (matchQuality) {
450-
EmojiMatchQuality.match => 0,
493+
EmojiMatchQuality.exact => 0,
494+
EmojiMatchQuality.prefix => 1,
495+
EmojiMatchQuality.other => 2,
451496
};
452497
}
453498

454499
/// The number of possible values returned by [_rankResult].
455-
static const _numResultRanks = 1;
500+
static const _numResultRanks = 3;
456501

457502
@override
458503
String toString() {

‎test/model/emoji_test.dart

Lines changed: 105 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,44 @@ void main() {
307307
check(view.results).single.which(
308308
isUnicodeResult(names: ['smile']));
309309
});
310+
311+
Future<Iterable<EmojiAutocompleteResult>> resultsOf(
312+
String query, {
313+
Map<String, String> realmEmoji = const {},
314+
Map<String, List<String>>? unicodeEmoji,
315+
}) async {
316+
final store = prepare(realmEmoji: realmEmoji, unicodeEmoji: unicodeEmoji);
317+
final view = EmojiAutocompleteView.init(store: store,
318+
query: EmojiAutocompleteQuery(query));
319+
bool done = false;
320+
view.addListener(() { done = true; });
321+
await Future(() {});
322+
check(done).isTrue();
323+
return view.results;
324+
}
325+
326+
test('results end-to-end', () async {
327+
final unicodeEmoji = {
328+
'1f4d3': ['notebook'], '1f516': ['bookmark'], '1f4d6': ['book']};
329+
330+
// Empty query -> base ordering.
331+
check(await resultsOf('', unicodeEmoji: unicodeEmoji)).deepEquals([
332+
isUnicodeResult(names: ['notebook']),
333+
isUnicodeResult(names: ['bookmark']),
334+
isUnicodeResult(names: ['book']),
335+
isZulipResult(),
336+
]);
337+
338+
// With query, exact match precedes prefix match precedes other.
339+
check(await resultsOf('book', unicodeEmoji: unicodeEmoji)).deepEquals([
340+
isUnicodeResult(names: ['book']),
341+
isUnicodeResult(names: ['bookmark']),
342+
isUnicodeResult(names: ['notebook']),
343+
]);
344+
});
310345
});
311346

312-
group('EmojiAutocompleteQuery.match', () {
347+
group('EmojiAutocompleteQuery', () {
313348
EmojiCandidate unicode(List<String> names, {String? emojiCode}) {
314349
emojiCode ??= '10ffff';
315350
return EmojiCandidate(emojiType: ReactionType.unicodeEmoji,
@@ -333,63 +368,71 @@ void main() {
333368
}
334369

335370
test('one-word query matches anywhere in name', () {
336-
check(matchOfName('', 'smile')).match;
337-
check(matchOfName('s', 'smile')).match;
338-
check(matchOfName('sm', 'smile')).match;
339-
check(matchOfName('smile', 'smile')).match;
340-
check(matchOfName('m', 'smile')).match;
341-
check(matchOfName('mile', 'smile')).match;
342-
check(matchOfName('e', 'smile')).match;
371+
check(matchOfName('', 'smile')).prefix;
372+
check(matchOfName('s', 'smile')).prefix;
373+
check(matchOfName('sm', 'smile')).prefix;
374+
check(matchOfName('smile', 'smile')).exact;
375+
check(matchOfName('m', 'smile')).other;
376+
check(matchOfName('mile', 'smile')).other;
377+
check(matchOfName('e', 'smile')).other;
343378

344379
check(matchOfName('smiley', 'smile')).none;
345380
check(matchOfName('a', 'smile')).none;
346381

347-
check(matchOfName('o', 'open_book')).match;
348-
check(matchOfName('open', 'open_book')).match;
349-
check(matchOfName('pe', 'open_book')).match;
350-
check(matchOfName('boo', 'open_book')).match;
351-
check(matchOfName('ok', 'open_book')).match;
382+
check(matchOfName('o', 'open_book')).prefix;
383+
check(matchOfName('open', 'open_book')).prefix;
384+
check(matchOfName('pe', 'open_book')).other;
385+
check(matchOfName('boo', 'open_book')).other;
386+
check(matchOfName('ok', 'open_book')).other;
352387
});
353388

354389
test('multi-word query matches from start of a word', () {
355-
check(matchOfName('open_', 'open_book')).match;
356-
check(matchOfName('open_b', 'open_book')).match;
357-
check(matchOfName('open_book', 'open_book')).match;
390+
check(matchOfName('open_', 'open_book')).prefix;
391+
check(matchOfName('open_b', 'open_book')).prefix;
392+
check(matchOfName('open_book', 'open_book')).exact;
358393

359394
check(matchOfName('pen_', 'open_book')).none;
360395
check(matchOfName('n_b', 'open_book')).none;
361396

362-
check(matchOfName('blue_dia', 'large_blue_diamond')).match;
397+
check(matchOfName('blue_dia', 'large_blue_diamond')).other;
363398
});
364399

365400
test('spaces in query behave as underscores', () {
366-
check(matchOfName('open ', 'open_book')).match;
367-
check(matchOfName('open b', 'open_book')).match;
368-
check(matchOfName('open book', 'open_book')).match;
401+
check(matchOfName('open ', 'open_book')).prefix;
402+
check(matchOfName('open b', 'open_book')).prefix;
403+
check(matchOfName('open book', 'open_book')).exact;
369404

370405
check(matchOfName('pen ', 'open_book')).none;
371406
check(matchOfName('n b', 'open_book')).none;
372407

373-
check(matchOfName('blue dia', 'large_blue_diamond')).match;
408+
check(matchOfName('blue dia', 'large_blue_diamond')).other;
374409
});
375410

376411
test('query is lower-cased', () {
377-
check(matchOfName('Smi', 'smile')).match;
412+
check(matchOfName('Smi', 'smile')).prefix;
378413
});
379414

380415
test('query matches aliases same way as primary name', () {
381-
check(matchOfNames('a', ['a', 'b'])).match;
382-
check(matchOfNames('b', ['a', 'b'])).match;
416+
check(matchOfNames('a', ['a', 'b'])).exact;
417+
check(matchOfNames('b', ['a', 'b'])).exact;
383418
check(matchOfNames('c', ['a', 'b'])).none;
384419

385-
check(matchOfNames('pe', ['x', 'open_book'])).match;
386-
check(matchOfNames('ok', ['x', 'open_book'])).match;
420+
check(matchOfNames('pe', ['x', 'open_book'])).other;
421+
check(matchOfNames('ok', ['x', 'open_book'])).other;
387422

388-
check(matchOfNames('open_', ['x', 'open_book'])).match;
389-
check(matchOfNames('open b', ['x', 'open_book'])).match;
423+
check(matchOfNames('open_', ['x', 'open_book'])).prefix;
424+
check(matchOfNames('open b', ['x', 'open_book'])).prefix;
390425
check(matchOfNames('pen_', ['x', 'open_book'])).none;
391426

392-
check(matchOfNames('Smi', ['x', 'smile'])).match;
427+
check(matchOfNames('Smi', ['x', 'smile'])).prefix;
428+
});
429+
430+
test('best match among name and aliases prevails', () {
431+
check(matchOfNames('a', ['ab', 'a', 'ba', 'x'])).exact;
432+
check(matchOfNames('a', ['ba', 'ab', 'x'])).prefix;
433+
check(matchOfNames('a', ['ba', 'ab'])).prefix;
434+
check(matchOfNames('a', ['ba', 'x'])).other;
435+
check(matchOfNames('a', ['x', 'y', 'z'])).none;
393436
});
394437

395438
test('query matches literal Unicode value', () {
@@ -403,13 +446,13 @@ void main() {
403446
check(matchOfLiteral('1f642', aka: '1f642', '1f642')).none;
404447

405448
// Matching the Unicode value the code describes does count…
406-
check(matchOfLiteral('🙂', aka: '\u{1f642}', '1f642')).match;
449+
check(matchOfLiteral('🙂', aka: '\u{1f642}', '1f642')).exact;
407450
// … and failing to match it doesn't make a match.
408451
check(matchOfLiteral('🙁', aka: '\u{1f641}', '1f642')).none;
409452

410453
// Multi-code-point emoji work fine.
411454
check(matchOfLiteral('🏳‍🌈', aka: '\u{1f3f3}\u{200d}\u{1f308}',
412-
'1f3f3-200d-1f308')).match;
455+
'1f3f3-200d-1f308')).exact;
413456
// Only exact matches count; no partial matches.
414457
check(matchOfLiteral('🏳', aka: '\u{1f3f3}',
415458
'1f3f3-200d-1f308')).none;
@@ -430,11 +473,11 @@ void main() {
430473
resolvedStillUrl: eg.realmUrl.resolve('/emoji/1-still.png')));
431474
}
432475

433-
check(matchOf('eqeq', realmCandidate('eqeq'))).match;
434-
check(matchOf('open_', realmCandidate('open_book'))).match;
476+
check(matchOf('eqeq', realmCandidate('eqeq'))).exact;
477+
check(matchOf('open_', realmCandidate('open_book'))).prefix;
435478
check(matchOf('n_b', realmCandidate('open_book'))).none;
436-
check(matchOf('blue dia', realmCandidate('large_blue_diamond'))).match;
437-
check(matchOf('Smi', realmCandidate('smile'))).match;
479+
check(matchOf('blue dia', realmCandidate('large_blue_diamond'))).other;
480+
check(matchOf('Smi', realmCandidate('smile'))).prefix;
438481
});
439482

440483
test('can match Zulip extra emoji', () {
@@ -446,11 +489,32 @@ void main() {
446489
emojiType: ReactionType.zulipExtraEmoji,
447490
emojiCode: 'zulip', emojiName: 'zulip'));
448491

449-
check(matchOf('z', zulipCandidate)).match;
450-
check(matchOf('Zulip', zulipCandidate)).match;
451-
check(matchOf('p', zulipCandidate)).match;
492+
check(matchOf('z', zulipCandidate)).prefix;
493+
check(matchOf('Zulip', zulipCandidate)).exact;
494+
check(matchOf('p', zulipCandidate)).other;
452495
check(matchOf('x', zulipCandidate)).none;
453496
});
497+
498+
int? rankOf(String query, EmojiCandidate candidate) {
499+
return EmojiAutocompleteQuery(query).testCandidate(candidate)?.rank;
500+
}
501+
502+
void checkPrecedes(String query, EmojiCandidate a, EmojiCandidate b) {
503+
check(rankOf(query, a)!).isLessThan(rankOf(query, b)!);
504+
}
505+
506+
test('ranks exact before prefix before other match', () {
507+
checkPrecedes('o', unicode(['o']), unicode(['onion']));
508+
checkPrecedes('o', unicode(['onion']), unicode(['book']));
509+
});
510+
511+
test('full list of ranks', () {
512+
check([
513+
rankOf('o', unicode(['o'])), // exact
514+
rankOf('o', unicode(['onion'])), // prefix
515+
rankOf('o', unicode(['book'])), // other
516+
]).deepEquals([0, 1, 2]);
517+
});
454518
});
455519
}
456520

@@ -476,7 +540,9 @@ extension EmojiCandidateChecks on Subject<EmojiCandidate> {
476540
}
477541

478542
extension EmojiMatchQualityChecks on Subject<EmojiMatchQuality?> {
479-
void get match => equals(EmojiMatchQuality.match);
543+
void get exact => equals(EmojiMatchQuality.exact);
544+
void get prefix => equals(EmojiMatchQuality.prefix);
545+
void get other => equals(EmojiMatchQuality.other);
480546
void get none => isNull();
481547
}
482548

0 commit comments

Comments
 (0)