Skip to content

Commit 4d82466

Browse files
gnpricegnprice
committed
emoji: Rank by quality of match (exact, prefix, other)
Fixes part of zulip#1068.
1 parent 04c44f6 commit 4d82466

File tree

2 files changed

+160
-56
lines changed

2 files changed

+160
-56
lines changed

‎lib/model/emoji.dart

+55-17
Original file line numberDiff line numberDiff line change
@@ -282,20 +282,30 @@ class EmojiStoreImpl with EmojiStore {
282282
}
283283

284284
/// The quality of an emoji's match to an autocomplete query.
285-
///
286-
/// (Rather vacuous for the moment; this structure will
287-
/// gain more substance in an upcoming commit.)
288285
enum EmojiMatchQuality {
289-
match;
286+
/// The query matches the whole emoji name (or the literal emoji itself).
287+
exact,
288+
289+
/// The query matches a prefix of the emoji name, but not the whole name.
290+
prefix,
291+
292+
/// The query matches somewhere in the emoji name, but not at the start.
293+
other;
290294

291295
/// The best possible quality of match.
292-
static const best = match;
296+
static const best = exact;
293297

294298
/// The better of the two given qualities of match,
295299
/// where null represents no match at all.
296300
static EmojiMatchQuality? bestOf(EmojiMatchQuality? a, EmojiMatchQuality? b) {
297301
if (b == null) return a;
298-
return b;
302+
if (a == null) return b;
303+
return compare(a, b) <= 0 ? a : b;
304+
}
305+
306+
/// Comparator that puts better matches first.
307+
static int compare(EmojiMatchQuality a, EmojiMatchQuality b) {
308+
return Enum.compareByIndex(a, b);
299309
}
300310
}
301311

@@ -365,11 +375,11 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
365375
// Compare get_emoji_matcher in Zulip web:shared/src/typeahead.ts .
366376
@visibleForTesting
367377
EmojiMatchQuality? match(EmojiCandidate candidate) {
368-
if (_adjusted == '') return EmojiMatchQuality.match;
378+
if (_adjusted == '') return EmojiMatchQuality.prefix;
369379

370380
if (candidate.emojiDisplay case UnicodeEmojiDisplay(:var emojiUnicode)) {
371381
if (_adjusted == emojiUnicode) {
372-
return EmojiMatchQuality.match;
382+
return EmojiMatchQuality.exact;
373383
}
374384
}
375385

@@ -382,13 +392,20 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
382392
}
383393

384394
EmojiMatchQuality? _matchName(String emojiName) {
385-
return _nameMatches(emojiName) ? EmojiMatchQuality.match : null;
386-
}
395+
// Compare query_matches_string_in_order in Zulip web:shared/src/typeahead.ts
396+
// for a Boolean version of this logic (match vs. no match),
397+
// and triage_raw in the same file web:shared/src/typeahead.ts
398+
// for the finer distinctions.
399+
// See also commentary in [_rankResult].
387400

388-
// Compare query_matches_string_in_order in Zulip web:shared/src/typeahead.ts .
389-
bool _nameMatches(String emojiName) {
390401
// TODO(#1067) this assumes emojiName is already lower-case (and no diacritics)
402+
if (emojiName == _adjusted) return EmojiMatchQuality.exact;
403+
if (emojiName.startsWith(_adjusted)) return EmojiMatchQuality.prefix;
404+
if (_nameMatches(emojiName)) return EmojiMatchQuality.other;
405+
return null;
406+
}
391407

408+
bool _nameMatches(String emojiName) {
392409
if (!_adjusted.contains(_separator)) {
393410
// If the query is a single token (doesn't contain a separator),
394411
// the match can be anywhere in the string.
@@ -399,21 +416,42 @@ class EmojiAutocompleteQuery extends ComposeAutocompleteQuery {
399416
// require the match to start at the start of a token.
400417
// (E.g. for 'ab_cd_ef', query could be 'ab_c' or 'cd_ef',
401418
// but not 'b_cd_ef'.)
402-
return emojiName.startsWith(_adjusted)
403-
|| emojiName.contains(_sepAdjusted);
419+
assert(!emojiName.startsWith(_adjusted)); // checked before calling this method
420+
return emojiName.contains(_sepAdjusted);
404421
}
405422

406423
/// A measure of the result's quality in the context of the query,
407424
/// ranked from 0 (best) to one less than [_numResultRanks].
408425
static int _rankResult(EmojiMatchQuality matchQuality) {
409-
// TODO(#1068): rank emoji results (popular, realm, other; exact match, prefix, other)
426+
// Compare sort_emojis in Zulip web:
427+
// https://github.com/zulip/zulip/blob/83a121c7e/web/shared/src/typeahead.ts#L322-L382
428+
//
429+
// Behavior differences we should or might copy, TODO(#1068):
430+
// * Web ranks popular emoji > custom emoji > others; we don't yet.
431+
// * Web ranks matches starting at a word boundary ahead of
432+
// other non-prefix matches; we don't yet.
433+
// * Web ranks each name of a Unicode emoji separately.
434+
//
435+
// Behavior differences that web should probably fix, TODO(web):
436+
// * Web starts with only case-sensitive exact matches ("perfect matches"),
437+
// and puts case-insensitive exact matches just ahead of prefix matches;
438+
// it also distinguishes prefix matches by case-sensitive vs. not.
439+
// We use case-insensitive matches throughout;
440+
// case seems unhelpful for emoji search.
441+
// * Web suppresses Unicode emoji names shadowed by a realm emoji
442+
// only if the latter is also a match for the query. That mostly works,
443+
// because emoji with the same name will mostly both match or both not;
444+
// but it breaks if the Unicode emoji was a literal match.
445+
410446
return switch (matchQuality) {
411-
EmojiMatchQuality.match => 0,
447+
EmojiMatchQuality.exact => 0,
448+
EmojiMatchQuality.prefix => 1,
449+
EmojiMatchQuality.other => 2,
412450
};
413451
}
414452

415453
/// The number of possible values returned by [_rankResult].
416-
static const _numResultRanks = 1;
454+
static const _numResultRanks = 3;
417455

418456
@override
419457
String toString() {

‎test/model/emoji_test.dart

+105-39
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,44 @@ void main() {
270270
check(view.results).single.which(
271271
isUnicodeResult(names: ['smile']));
272272
});
273+
274+
Future<Iterable<EmojiAutocompleteResult>> resultsOf(
275+
String query, {
276+
Map<String, String> realmEmoji = const {},
277+
Map<String, List<String>>? unicodeEmoji,
278+
}) async {
279+
final store = prepare(realmEmoji: realmEmoji, unicodeEmoji: unicodeEmoji);
280+
final view = EmojiAutocompleteView.init(store: store,
281+
query: EmojiAutocompleteQuery(query));
282+
bool done = false;
283+
view.addListener(() { done = true; });
284+
await Future(() {});
285+
check(done).isTrue();
286+
return view.results;
287+
}
288+
289+
test('results end-to-end', () async {
290+
final unicodeEmoji = {
291+
'1f4d3': ['notebook'], '1f516': ['bookmark'], '1f4d6': ['book']};
292+
293+
// Empty query -> base ordering.
294+
check(await resultsOf('', unicodeEmoji: unicodeEmoji)).deepEquals([
295+
isUnicodeResult(names: ['notebook']),
296+
isUnicodeResult(names: ['bookmark']),
297+
isUnicodeResult(names: ['book']),
298+
isZulipResult(),
299+
]);
300+
301+
// With query, exact match precedes prefix match precedes other.
302+
check(await resultsOf('book', unicodeEmoji: unicodeEmoji)).deepEquals([
303+
isUnicodeResult(names: ['book']),
304+
isUnicodeResult(names: ['bookmark']),
305+
isUnicodeResult(names: ['notebook']),
306+
]);
307+
});
273308
});
274309

275-
group('EmojiAutocompleteQuery.match', () {
310+
group('EmojiAutocompleteQuery', () {
276311
EmojiCandidate unicode(List<String> names, {String? emojiCode}) {
277312
emojiCode ??= '10ffff';
278313
return EmojiCandidate(emojiType: ReactionType.unicodeEmoji,
@@ -296,63 +331,71 @@ void main() {
296331
}
297332

298333
test('one-word query matches anywhere in name', () {
299-
check(matchOfName('', 'smile')).match;
300-
check(matchOfName('s', 'smile')).match;
301-
check(matchOfName('sm', 'smile')).match;
302-
check(matchOfName('smile', 'smile')).match;
303-
check(matchOfName('m', 'smile')).match;
304-
check(matchOfName('mile', 'smile')).match;
305-
check(matchOfName('e', 'smile')).match;
334+
check(matchOfName('', 'smile')).prefix;
335+
check(matchOfName('s', 'smile')).prefix;
336+
check(matchOfName('sm', 'smile')).prefix;
337+
check(matchOfName('smile', 'smile')).exact;
338+
check(matchOfName('m', 'smile')).other;
339+
check(matchOfName('mile', 'smile')).other;
340+
check(matchOfName('e', 'smile')).other;
306341

307342
check(matchOfName('smiley', 'smile')).none;
308343
check(matchOfName('a', 'smile')).none;
309344

310-
check(matchOfName('o', 'open_book')).match;
311-
check(matchOfName('open', 'open_book')).match;
312-
check(matchOfName('pe', 'open_book')).match;
313-
check(matchOfName('boo', 'open_book')).match;
314-
check(matchOfName('ok', 'open_book')).match;
345+
check(matchOfName('o', 'open_book')).prefix;
346+
check(matchOfName('open', 'open_book')).prefix;
347+
check(matchOfName('pe', 'open_book')).other;
348+
check(matchOfName('boo', 'open_book')).other;
349+
check(matchOfName('ok', 'open_book')).other;
315350
});
316351

317352
test('multi-word query matches from start of a word', () {
318-
check(matchOfName('open_', 'open_book')).match;
319-
check(matchOfName('open_b', 'open_book')).match;
320-
check(matchOfName('open_book', 'open_book')).match;
353+
check(matchOfName('open_', 'open_book')).prefix;
354+
check(matchOfName('open_b', 'open_book')).prefix;
355+
check(matchOfName('open_book', 'open_book')).exact;
321356

322357
check(matchOfName('pen_', 'open_book')).none;
323358
check(matchOfName('n_b', 'open_book')).none;
324359

325-
check(matchOfName('blue_dia', 'large_blue_diamond')).match;
360+
check(matchOfName('blue_dia', 'large_blue_diamond')).other;
326361
});
327362

328363
test('spaces in query behave as underscores', () {
329-
check(matchOfName('open ', 'open_book')).match;
330-
check(matchOfName('open b', 'open_book')).match;
331-
check(matchOfName('open book', 'open_book')).match;
364+
check(matchOfName('open ', 'open_book')).prefix;
365+
check(matchOfName('open b', 'open_book')).prefix;
366+
check(matchOfName('open book', 'open_book')).exact;
332367

333368
check(matchOfName('pen ', 'open_book')).none;
334369
check(matchOfName('n b', 'open_book')).none;
335370

336-
check(matchOfName('blue dia', 'large_blue_diamond')).match;
371+
check(matchOfName('blue dia', 'large_blue_diamond')).other;
337372
});
338373

339374
test('query is lower-cased', () {
340-
check(matchOfName('Smi', 'smile')).match;
375+
check(matchOfName('Smi', 'smile')).prefix;
341376
});
342377

343378
test('query matches aliases same way as primary name', () {
344-
check(matchOfNames('a', ['a', 'b'])).match;
345-
check(matchOfNames('b', ['a', 'b'])).match;
379+
check(matchOfNames('a', ['a', 'b'])).exact;
380+
check(matchOfNames('b', ['a', 'b'])).exact;
346381
check(matchOfNames('c', ['a', 'b'])).none;
347382

348-
check(matchOfNames('pe', ['x', 'open_book'])).match;
349-
check(matchOfNames('ok', ['x', 'open_book'])).match;
383+
check(matchOfNames('pe', ['x', 'open_book'])).other;
384+
check(matchOfNames('ok', ['x', 'open_book'])).other;
350385

351-
check(matchOfNames('open_', ['x', 'open_book'])).match;
352-
check(matchOfNames('open b', ['x', 'open_book'])).match;
386+
check(matchOfNames('open_', ['x', 'open_book'])).prefix;
387+
check(matchOfNames('open b', ['x', 'open_book'])).prefix;
353388
check(matchOfNames('pen_', ['x', 'open_book'])).none;
354389

355-
check(matchOfNames('Smi', ['x', 'smile'])).match;
390+
check(matchOfNames('Smi', ['x', 'smile'])).prefix;
391+
});
392+
393+
test('best match among name and aliases prevails', () {
394+
check(matchOfNames('a', ['ab', 'a', 'ba', 'x'])).exact;
395+
check(matchOfNames('a', ['ba', 'ab', 'x'])).prefix;
396+
check(matchOfNames('a', ['ba', 'ab'])).prefix;
397+
check(matchOfNames('a', ['ba', 'x'])).other;
398+
check(matchOfNames('a', ['x', 'y', 'z'])).none;
356399
});
357400

358401
test('query matches literal Unicode value', () {
@@ -366,13 +409,13 @@ void main() {
366409
check(matchOfLiteral('1f642', aka: '1f642', '1f642')).none;
367410

368411
// Matching the Unicode value the code describes does count…
369-
check(matchOfLiteral('🙂', aka: '\u{1f642}', '1f642')).match;
412+
check(matchOfLiteral('🙂', aka: '\u{1f642}', '1f642')).exact;
370413
// … and failing to match it doesn't make a match.
371414
check(matchOfLiteral('🙁', aka: '\u{1f641}', '1f642')).none;
372415

373416
// Multi-code-point emoji work fine.
374417
check(matchOfLiteral('🏳‍🌈', aka: '\u{1f3f3}\u{200d}\u{1f308}',
375-
'1f3f3-200d-1f308')).match;
418+
'1f3f3-200d-1f308')).exact;
376419
// Only exact matches count; no partial matches.
377420
check(matchOfLiteral('🏳', aka: '\u{1f3f3}',
378421
'1f3f3-200d-1f308')).none;
@@ -393,11 +436,11 @@ void main() {
393436
resolvedStillUrl: eg.realmUrl.resolve('/emoji/1-still.png')));
394437
}
395438

396-
check(matchOf('eqeq', realmCandidate('eqeq'))).match;
397-
check(matchOf('open_', realmCandidate('open_book'))).match;
439+
check(matchOf('eqeq', realmCandidate('eqeq'))).exact;
440+
check(matchOf('open_', realmCandidate('open_book'))).prefix;
398441
check(matchOf('n_b', realmCandidate('open_book'))).none;
399-
check(matchOf('blue dia', realmCandidate('large_blue_diamond'))).match;
400-
check(matchOf('Smi', realmCandidate('smile'))).match;
442+
check(matchOf('blue dia', realmCandidate('large_blue_diamond'))).other;
443+
check(matchOf('Smi', realmCandidate('smile'))).prefix;
401444
});
402445

403446
test('can match Zulip extra emoji', () {
@@ -409,11 +452,32 @@ void main() {
409452
emojiType: ReactionType.zulipExtraEmoji,
410453
emojiCode: 'zulip', emojiName: 'zulip'));
411454

412-
check(matchOf('z', zulipCandidate)).match;
413-
check(matchOf('Zulip', zulipCandidate)).match;
414-
check(matchOf('p', zulipCandidate)).match;
455+
check(matchOf('z', zulipCandidate)).prefix;
456+
check(matchOf('Zulip', zulipCandidate)).exact;
457+
check(matchOf('p', zulipCandidate)).other;
415458
check(matchOf('x', zulipCandidate)).none;
416459
});
460+
461+
int? rankOf(String query, EmojiCandidate candidate) {
462+
return EmojiAutocompleteQuery(query).testCandidate(candidate)?.rank;
463+
}
464+
465+
void checkPrecedes(String query, EmojiCandidate a, EmojiCandidate b) {
466+
check(rankOf(query, a)!).isLessThan(rankOf(query, b)!);
467+
}
468+
469+
test('ranks exact before prefix before other match', () {
470+
checkPrecedes('o', unicode(['o']), unicode(['onion']));
471+
checkPrecedes('o', unicode(['onion']), unicode(['book']));
472+
});
473+
474+
test('full list of ranks', () {
475+
check([
476+
rankOf('o', unicode(['o'])), // exact
477+
rankOf('o', unicode(['onion'])), // prefix
478+
rankOf('o', unicode(['book'])), // other
479+
]).deepEquals([0, 1, 2]);
480+
});
417481
});
418482
}
419483

@@ -439,7 +503,9 @@ extension EmojiCandidateChecks on Subject<EmojiCandidate> {
439503
}
440504

441505
extension EmojiMatchQualityChecks on Subject<EmojiMatchQuality?> {
442-
void get match => equals(EmojiMatchQuality.match);
506+
void get exact => equals(EmojiMatchQuality.exact);
507+
void get prefix => equals(EmojiMatchQuality.prefix);
508+
void get other => equals(EmojiMatchQuality.other);
443509
void get none => isNull();
444510
}
445511

0 commit comments

Comments
 (0)