Skip to content

autocomplete: Add code to recognize @-mention syntax in the content input #154

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

Merged
merged 6 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
119 changes: 113 additions & 6 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
@@ -1,10 +1,107 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';

import '../api/model/events.dart';
import '../api/model/model.dart';
import '../widgets/compose_box.dart';
import 'narrow.dart';
import 'store.dart';

extension Autocomplete on ContentTextEditingController {
AutocompleteIntent? autocompleteIntent() {
if (!selection.isValid || !selection.isNormalized) {
// We don't require [isCollapsed] to be true because we've seen that
// autocorrect and even backspace involve programmatically expanding the
// selection to the left. Once we know where the syntax starts, we can at
// least require that the selection doesn't extend leftward past that;
// see below.
return null;
}
final textUntilCursor = text.substring(0, selection.end);
for (
int position = selection.end - 1;
position >= 0 && (selection.end - position <= 30);
position--
) {
if (textUntilCursor[position] != '@') {
continue;
}
final match = mentionAutocompleteMarkerRegex.matchAsPrefix(textUntilCursor, position);
if (match == null) {
continue;
}
if (selection.start < position) {
// See comment about [TextSelection.isCollapsed] above.
return null;
}
Comment on lines +33 to +36
Copy link
Member

Choose a reason for hiding this comment

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

I think this part actually still belongs after the regexp match succeeds.

Otherwise we get a peculiar behavior where if you're in a situation like (in the tests' notation)
~@[email protected]^
and you start extending the selection left, the autocomplete state remains unchanged for a while:
~@someone@^example.com^
but then once you get past the @ that was in the middle of the query:
@someone^@example.com^
@^[email protected]^
the autocomplete goes away, even though you've still only selected text that was within the query.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, right! Thanks for the catch; I'll fix this and add test cases from those helpful examples you gave.

return AutocompleteIntent(
syntaxStart: position,
query: MentionAutocompleteQuery(match[2]!, silent: match[1]! == '_'),
textEditingValue: value);
}
return null;
}
}

final RegExp mentionAutocompleteMarkerRegex = (() {
// What's likely to come before an @-mention: the start of the string,
// whitespace, or punctuation. Letters are unlikely; in that case an email
// might be intended. (By punctuation, we mean *some* punctuation, like "(".
// We could refine this.)
const beforeAtSign = r'(?<=^|\s|\p{Punctuation})';

// Characters that would defeat searches in full_name and emails, since
// they're prohibited in both forms. These are all the characters prohibited
// in full_name except "@", which appears in emails. (For the form of
// full_name, find uses of UserProfile.NAME_INVALID_CHARS in zulip/zulip.)
const fullNameAndEmailCharExclusions = r'\*`\\>"\p{Other}';

return RegExp(
beforeAtSign
+ r'@(_?)' // capture, so we can distinguish silent mentions
+ r'(|'
// Reject on whitespace right after "@" or "@_". Emails can't start with
// it, and full_name can't either (it's run through Python's `.strip()`).
+ r'[^\s' + fullNameAndEmailCharExclusions + r']'
+ r'[^' + fullNameAndEmailCharExclusions + r']*'
+ r')$',
unicode: true);
})();

/// The content controller's recognition that the user might want autocomplete UI.
class AutocompleteIntent {
AutocompleteIntent({
required this.syntaxStart,
required this.query,
required this.textEditingValue,
});

/// At what index the intent's syntax starts. E.g., 3, in "Hi @chris".
///
/// May be used with [textEditingValue] to make a new [TextEditingValue] with
/// the autocomplete interaction's result: e.g., one that replaces "Hi @chris"
/// with "Hi @**Chris Bobbe** ". (Assume [textEditingValue.selection.end] is
/// the end of the syntax.)
///
/// Using this to index into something other than [textEditingValue] will give
/// undefined behavior and might cause a RangeError; it should be avoided.
// If a subclassed [TextEditingValue] could itself be the source of
// [syntaxStart], then the safe behavior would be accomplished more
// naturally, I think. But [TextEditingController] doesn't support subclasses
// that use a custom/subclassed [TextEditingValue], so that's not convenient.
final int syntaxStart;

final MentionAutocompleteQuery query; // TODO other autocomplete query types

/// The [TextEditingValue] whose text [syntaxStart] refers to.
final TextEditingValue textEditingValue;

@override
String toString() {
return '${objectRuntimeType(this, 'AutocompleteIntent')}(syntaxStart: $syntaxStart, query: $query, textEditingValue: $textEditingValue})';
}
}

/// A per-account manager for the view-models of autocomplete interactions.
///
/// There should be exactly one of these per PerAccountStore.
Expand Down Expand Up @@ -86,7 +183,9 @@ class MentionAutocompleteView extends ChangeNotifier {
@override
void dispose() {
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
// TODO cancel in-progress computations if possible
// We cancel in-progress computations by checking [hasListeners] between tasks.
// After [super.dispose] is called, [hasListeners] returns false.
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
super.dispose();
}

Expand Down Expand Up @@ -134,7 +233,7 @@ class MentionAutocompleteView extends ChangeNotifier {
}

if (newResults == null) {
// Query was old; new search is in progress.
// Query was old; new search is in progress. Or, no listeners to notify.
return;
}

Expand All @@ -152,7 +251,7 @@ class MentionAutocompleteView extends ChangeNotifier {
// CPU perf: End this task; enqueue a new one for resuming this work
await Future(() {});

if (query != _currentQuery) {
if (query != _currentQuery || !hasListeners) { // false if [dispose] has been called.
return null;
}

Expand All @@ -173,11 +272,14 @@ class MentionAutocompleteView extends ChangeNotifier {
}

class MentionAutocompleteQuery {
MentionAutocompleteQuery(this.raw)
MentionAutocompleteQuery(this.raw, {this.silent = false})
: _lowercaseWords = raw.toLowerCase().split(' ');

final String raw;

/// Whether the user wants a silent mention (@_query, vs. @query).
final bool silent;

final List<String> _lowercaseWords;

bool testUser(User user, AutocompleteDataCache cache) {
Expand All @@ -203,13 +305,18 @@ class MentionAutocompleteQuery {
}
}

@override
String toString() {
return '${objectRuntimeType(this, 'MentionAutocompleteQuery')}(raw: $raw, silent: $silent})';
}

@override
bool operator ==(Object other) {
return other is MentionAutocompleteQuery && other.raw == raw;
return other is MentionAutocompleteQuery && other.raw == raw && other.silent == silent;
}

@override
int get hashCode => Object.hash('MentionAutocompleteQuery', raw);
int get hashCode => Object.hash('MentionAutocompleteQuery', raw, silent);
}

class AutocompleteDataCache {
Expand Down
30 changes: 28 additions & 2 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:flutter/services.dart';
import 'package:image_picker/image_picker.dart';

import '../api/route/messages.dart';
import '../model/autocomplete.dart';
import '../model/narrow.dart';
import 'dialog.dart';
import 'store.dart';
Expand Down Expand Up @@ -152,12 +153,14 @@ class ContentTextEditingController extends TextEditingController {
/// The content input for StreamComposeBox.
class _StreamContentInput extends StatefulWidget {
const _StreamContentInput({
required this.narrow,
required this.streamId,
required this.controller,
required this.topicController,
required this.focusNode,
});

final Narrow narrow;
final int streamId;
final ContentTextEditingController controller;
final TopicTextEditingController topicController;
Expand All @@ -168,6 +171,8 @@ class _StreamContentInput extends StatefulWidget {
}

class _StreamContentInputState extends State<_StreamContentInput> {
MentionAutocompleteView? _mentionAutocompleteView; // TODO different autocomplete view types

late String _topicTextNormalized;

_topicChanged() {
Expand All @@ -176,16 +181,33 @@ class _StreamContentInputState extends State<_StreamContentInput> {
});
}

_changed() {
final newAutocompleteIntent = widget.controller.autocompleteIntent();
if (newAutocompleteIntent != null) {
final store = PerAccountStoreWidget.of(context);
_mentionAutocompleteView ??= MentionAutocompleteView.init(
store: store, narrow: widget.narrow);
_mentionAutocompleteView!.query = newAutocompleteIntent.query;
} else {
if (_mentionAutocompleteView != null) {
_mentionAutocompleteView!.dispose();
_mentionAutocompleteView = null;
}
}
}

@override
void initState() {
super.initState();
_topicTextNormalized = widget.topicController.textNormalized();
widget.topicController.addListener(_topicChanged);
widget.controller.addListener(_changed);
}

@override
void dispose() {
widget.topicController.removeListener(_topicChanged);
widget.controller.removeListener(_changed);
super.dispose();
}

Expand Down Expand Up @@ -572,7 +594,10 @@ class _StreamSendButtonState extends State<_StreamSendButton> {

/// The compose box for writing a stream message.
class StreamComposeBox extends StatefulWidget {
const StreamComposeBox({super.key, required this.streamId});
const StreamComposeBox({super.key, required this.narrow, required this.streamId});

/// The narrow on view in the message list.
final Narrow narrow;

final int streamId;

Expand Down Expand Up @@ -633,6 +658,7 @@ class _StreamComposeBoxState extends State<StreamComposeBox> {
topicInput,
const SizedBox(height: 8),
_StreamContentInput(
narrow: widget.narrow,
streamId: widget.streamId,
topicController: _topicController,
controller: _contentController,
Expand Down Expand Up @@ -666,7 +692,7 @@ class ComposeBox extends StatelessWidget {
Widget build(BuildContext context) {
final narrow = this.narrow;
if (narrow is StreamNarrow) {
return StreamComposeBox(streamId: narrow.streamId);
return StreamComposeBox(narrow: narrow, streamId: narrow.streamId);
} else if (narrow is TopicNarrow) {
return const SizedBox.shrink(); // TODO(#144): add a single-topic compose box
} else if (narrow is AllMessagesNarrow) {
Expand Down
10 changes: 10 additions & 0 deletions test/model/autocomplete_checks.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
import 'package:checks/checks.dart';
import 'package:zulip/model/autocomplete.dart';
import 'package:zulip/widgets/compose_box.dart';

extension ContentTextEditingControllerChecks on Subject<ContentTextEditingController> {
Subject<AutocompleteIntent?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent');
}

extension AutocompleteIntentChecks on Subject<AutocompleteIntent> {
Subject<int> get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart');
Subject<MentionAutocompleteQuery> get query => has((i) => i.query, 'query');
}

extension UserMentionAutocompleteResultChecks on Subject<UserMentionAutocompleteResult> {
Subject<int> get userId => has((r) => r.userId, 'userId');
Expand Down
Loading