From 8b56dd1d57104cb6d95e068bf2e81ff2716fe665 Mon Sep 17 00:00:00 2001 From: Corentin Bazin Date: Tue, 12 Mar 2024 19:06:57 +0100 Subject: [PATCH 1/5] fix for https://github.com/superlistapp/super_editor/issues/1882 and https://github.com/superlistapp/super_editor/issues/1863 + more tests --- .../text_tokenizing/stable_tags.dart | 3 + .../default_editor/text_tokenizing/tags.dart | 71 ++--- .../text_entry/tagging/stable_tags_test.dart | 280 +++++++++++++++++- 3 files changed, 308 insertions(+), 46 deletions(-) diff --git a/super_editor/lib/src/default_editor/text_tokenizing/stable_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/stable_tags.dart index 8dd408227..3a250c1af 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/stable_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/stable_tags.dart @@ -1094,6 +1094,9 @@ class ComposingStableTag { @override int get hashCode => contentBounds.hashCode ^ token.hashCode; + + @override + String toString() => 'ComposingStableTag{contentBounds: $contentBounds, token: $token}'; } /// An [EditReaction] that prevents partial selection of a stable user tag. diff --git a/super_editor/lib/src/default_editor/text_tokenizing/tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/tags.dart index 201c42b0e..b93462008 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/tags.dart @@ -5,7 +5,6 @@ import 'package:characters/characters.dart'; import 'package:super_editor/src/core/document.dart'; import 'package:super_editor/src/core/document_selection.dart'; import 'package:super_editor/src/default_editor/text.dart'; -import 'package:super_editor/src/infrastructure/strings.dart'; /// A set of tools for finding tags within document text. class TagFinder { @@ -23,55 +22,51 @@ class TagFinder { return null; } - int tokenStartOffset = min(expansionPosition.offset - 1, rawText.length - 1); - tokenStartOffset = max(tokenStartOffset, 0); - if (tagRule.excludedCharacters.contains(rawText[tokenStartOffset])) { + int splitIndex = min(expansionPosition.offset, rawText.length); + splitIndex = max(splitIndex, 0); + + // Create 2 splits of characters to navigate upstream and downstream the caret position. + // ex: "this is a very|long string" + // -> split around the caret into charactersBefore="this is a very" and charactersAfter="long string" + final charactersBefore = rawText.substring(0, splitIndex).characters; + final iteratorUpstream = charactersBefore.iteratorAtEnd; + + final charactersAfter = rawText.substring(splitIndex).characters; + final iteratorDownstream = charactersAfter.iterator; + + if (charactersBefore.isNotEmpty && tagRule.excludedCharacters.contains(charactersBefore.last)) { // The character where we're supposed to begin our expansion is a // character that's not allowed in a tag. Therefore, no tag exists // around the search offset. return null; } - int tokenEndOffset = min(expansionPosition.offset - 1, rawText.length - 1); - tokenEndOffset = max(tokenEndOffset, 0); - - if (rawText[tokenStartOffset] != tagRule.trigger) { - while (tokenStartOffset > 0) { - final upstreamCharacterIndex = rawText.moveOffsetUpstreamByCharacter(tokenStartOffset)!; - final upstreamCharacter = rawText[upstreamCharacterIndex]; - if (tagRule.excludedCharacters.contains(upstreamCharacter)) { - // The upstream character isn't allowed to appear in a tag. Break before moving - // the starting character index any further upstream. - break; - } - - // Move the starting character index upstream. - tokenStartOffset = upstreamCharacterIndex; + // Move upstream until we find the trigger character or an excluded character. + while (iteratorUpstream.moveBack()) { + final currentCharacter = iteratorUpstream.current; + if (tagRule.excludedCharacters.contains(currentCharacter)) { + // The upstream character isn't allowed to appear in a tag. end the search. + return null; + } - if (upstreamCharacter == tagRule.trigger) { - // The character we just added to the token bounds is the trigger. - // We don't want to move the start any further upstream. - break; - } + if (currentCharacter == tagRule.trigger) { + // The character we are reading is the trigger. + // We move the iteratorUpstream one last time to include the trigger in the tokenRange and stop looking any further upstream + iteratorUpstream.moveBack(); + break; } } - while (tokenEndOffset < rawText.length - 1) { - final downstreamCharacterIndex = rawText.moveOffsetDownstreamByCharacter(tokenEndOffset)!; - final downstreamCharacter = rawText[downstreamCharacterIndex]; - if (downstreamCharacter != tagRule.trigger && tagRule.excludedCharacters.contains(downstreamCharacter)) { + // Move downstream the caret position until we find excluded character or reach the end of the text. + while (iteratorDownstream.moveNext()) { + final current = iteratorDownstream.current; + if (current != tagRule.trigger && tagRule.excludedCharacters.contains(current)) { break; } - - tokenEndOffset = downstreamCharacterIndex; } - // Make end off exclusive. - tokenEndOffset += 1; - final tokenRange = SpanRange(tokenStartOffset, tokenEndOffset); - if (tokenRange.end - tokenRange.start <= 0) { - return null; - } + final tokenStartOffset = splitIndex - iteratorUpstream.stringAfterLength; + final tokenRange = SpanRange(tokenStartOffset, splitIndex + iteratorDownstream.stringBeforeLength); final tagText = text.substringInRange(tokenRange); if (!tagText.startsWith(tagRule.trigger)) { @@ -83,7 +78,7 @@ class TagFinder { return null; } - final tagAroundPosition = TagAroundPosition( + return TagAroundPosition( indexedTag: IndexedTag( Tag(tagRule.trigger, tagText.substring(1)), nodeId, @@ -91,8 +86,6 @@ class TagFinder { ), searchOffset: expansionPosition.offset, ); - - return tagAroundPosition; } /// Finds and returns all tags in the given [textNode], which meet the given [rule]. diff --git a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart index 8c008c9ad..9421ab9dd 100644 --- a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart @@ -29,6 +29,25 @@ void main() { ); }); + testWidgetsOnAllPlatforms("can start with a single character", (tester) async { + await _pumpTestEditor( + tester, + singleParagraphEmptyDoc(), + ); + await tester.placeCaretInParagraph("1", 0); + + // Compose a stable tag. + await tester.typeImeText("@j"); + + // Ensure that the tag has a composing attribution. + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "@j"); + expect( + text.getAttributedRange({stableTagComposingAttribution}, 0), + const SpanRange(0, 1), + ); + }); + testWidgetsOnAllPlatforms("can start between words", (tester) async { await _pumpTestEditor( tester, @@ -100,7 +119,9 @@ void main() { ), ], ), - const TagRule(trigger: "@"), + plugin: StableTagPlugin( + tagRule: const TagRule(trigger: '@'), + ), ); // Place the caret at "before |" @@ -577,6 +598,48 @@ void main() { const SpanRange(7, 11), ); }); + + testWidgetsOnAllPlatforms("with consecutive triggers", (tester) async { + final plugin = StableTagPlugin(); + await _pumpTestEditor( + tester, + singleParagraphEmptyDoc(), + plugin: plugin, + ); + await tester.placeCaretInParagraph("1", 0); + + // Type two consecutive trigger characters. + await tester.typeImeText("@@"); + + final composingStableTag = plugin.tagIndex.composingStableTag.value!; + + // Ensure the composing tag is placed on the second @, with empty token. + expect( + composingStableTag, + const ComposingStableTag( + DocumentRange( + start: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 2), + ), + end: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 2), + ), + ), + '', + ), + ); + + final commitedTags = plugin.tagIndex.getCommittedTagsInTextNode('1'); + + expect(commitedTags.length, 1); + + final commitTag = commitedTags.first; + + // Ensure the committed tag is the first @ + expect(commitTag, const IndexedTag(Tag('@', ''), '1', 0)); + }); }); group("committed >", () { @@ -1042,19 +1105,222 @@ void main() { ); }); }); + + group("emoji >", () { + testWidgetsOnAllPlatforms("typed as first character of a paragraph", (tester) async { + // Ensure we can type an emoji as first character without crashing + // https://github.com/superlistapp/super_editor/issues/1863 + await _pumpTestEditor( + tester, + MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText(""), + ), + ], + ), + ); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("1", 0); + + // Type an emoji as first charactet πŸ’™ + await tester.typeImeText("πŸ’™"); + + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 2), + ), + ), + ); + + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "πŸ’™"); + }); + + testWidgetsOnAllPlatforms("after trigger", (tester) async { + final plugin = StableTagPlugin(); + await _pumpTestEditor( + tester, + MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText("πŸ’™"), + ), + ], + ), + plugin: plugin, + ); + + // Place the caret after the emoji: πŸ’™| + await tester.placeCaretInParagraph("1", 2); + + // Move the caret before the emoji: |πŸ’™ + await tester.pressLeftArrow(); + + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 0), + ), + ), + ); + + // Type @ and ensure the string is valid: @πŸ’™ + await tester.typeImeText("@"); + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "@πŸ’™"); + + // Ensure the composing tag is on the emoji + final composingStableTag = plugin.tagIndex.composingStableTag.value!; + expect( + composingStableTag, + const ComposingStableTag( + DocumentRange( + start: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 1), + ), + end: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 3), + ), + ), + 'πŸ’™', + ), + ); + + // Move the caret after the emoji: @πŸ’™| + await tester.pressRightArrow(); + + // Type a white space to commit the tag: @πŸ’™ | + await tester.typeImeText(" "); + + final commitedTags = plugin.tagIndex.getCommittedTagsInTextNode('1'); + + expect(commitedTags.length, 1); + + final commitTag = commitedTags.first; + + // Ensure the committed tag is the emoji and the composing tag is removed + expect(commitTag, const IndexedTag(Tag('@', 'πŸ’™'), '1', 0)); + expect(plugin.tagIndex.composingStableTag.value, isNull); + }); + + testWidgetsOnAllPlatforms("before trigger", (tester) async { + final plugin = StableTagPlugin(); + await _pumpTestEditor( + tester, + MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText("πŸ’™"), + ), + ], + ), + plugin: plugin, + ); + + // Place the caret after the emoji: πŸ’™| + await tester.placeCaretInParagraph("1", 2); + + // Type @ and ensure the string is valid: πŸ’™@ + await tester.typeImeText("@"); + + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "πŸ’™@"); + + // Ensure the composing tag is not null and empty + expect( + plugin.tagIndex.composingStableTag.value, + const ComposingStableTag( + DocumentRange( + start: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 3), + ), + end: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 3), + ), + ), + '', + ), + ); + }); + + testWidgetsOnAllPlatforms("in the middle of a tag", (tester) async { + final plugin = StableTagPlugin(); + await _pumpTestEditor( + tester, + MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText(""), + ), + ], + ), + plugin: plugin, + ); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("1", 0); + + // Start compsing a tag with an emoji in the middle + await tester.typeImeText("@FlutterπŸ’™SuperEditor"); + + // Ensure the composing tag is valid + final composingStableTag = plugin.tagIndex.composingStableTag.value!; + expect( + composingStableTag, + const ComposingStableTag( + DocumentRange( + start: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 1), + ), + end: DocumentPosition( + nodeId: '1', + nodePosition: TextNodePosition(offset: 21), + ), + ), + 'FlutterπŸ’™SuperEditor', + ), + ); + + // Commit the tag by typing a white space + await tester.typeImeText(" "); + + // Ensure the tag is commited and the composing tag is reset + final commitedTags = plugin.tagIndex.getCommittedTagsInTextNode('1'); + expect(commitedTags.length, 1); + expect(commitedTags.first, const IndexedTag(Tag('@', 'FlutterπŸ’™SuperEditor'), '1', 0)); + expect(plugin.tagIndex.composingStableTag.value, isNull); + }); + }); }); } Future _pumpTestEditor( WidgetTester tester, - MutableDocument document, [ - TagRule tagRule = userTagRule, -]) async { + MutableDocument document, { + SuperEditorPlugin? plugin, +}) async { return await tester .createDocument() .withCustomContent(document) - .withPlugin(StableTagPlugin( - tagRule: tagRule, - )) + .withPlugin(plugin ?? + StableTagPlugin( + tagRule: userTagRule, + )) .pump(); } From 26506cdcce1822ba38e9608079fceb9dc8ff749e Mon Sep 17 00:00:00 2001 From: Corentin Bazin Date: Tue, 12 Mar 2024 23:47:58 +0100 Subject: [PATCH 2/5] better test naming --- .../test/super_editor/text_entry/tagging/stable_tags_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart index 9421ab9dd..71c867f94 100644 --- a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart @@ -1107,7 +1107,8 @@ void main() { }); group("emoji >", () { - testWidgetsOnAllPlatforms("typed as first character of a paragraph", (tester) async { + testWidgetsOnAllPlatforms("can be typed as first character of a paragraph without crashing the editor", + (tester) async { // Ensure we can type an emoji as first character without crashing // https://github.com/superlistapp/super_editor/issues/1863 await _pumpTestEditor( From c09055d892b919a2e006d3e85b7a9f3cb7829898 Mon Sep 17 00:00:00 2001 From: Corentin Bazin Date: Wed, 13 Mar 2024 00:53:11 +0100 Subject: [PATCH 3/5] remove wrong condition in iteratorDownStream loop --- super_editor/lib/src/default_editor/text_tokenizing/tags.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/super_editor/lib/src/default_editor/text_tokenizing/tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/tags.dart index b93462008..7122ce72f 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/tags.dart @@ -60,7 +60,7 @@ class TagFinder { // Move downstream the caret position until we find excluded character or reach the end of the text. while (iteratorDownstream.moveNext()) { final current = iteratorDownstream.current; - if (current != tagRule.trigger && tagRule.excludedCharacters.contains(current)) { + if (tagRule.excludedCharacters.contains(current)) { break; } } From 7d997f7f2d6489d2bcd961b9b0438cb76871019e Mon Sep 17 00:00:00 2001 From: Corentin Bazin Date: Wed, 13 Mar 2024 01:21:40 +0100 Subject: [PATCH 4/5] naming --- .../text_entry/tagging/stable_tags_test.dart | 62 +++++++++++++++---- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart index 71c867f94..fbbb07101 100644 --- a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart @@ -1143,7 +1143,9 @@ void main() { expect(text.text, "πŸ’™"); }); - testWidgetsOnAllPlatforms("after trigger", (tester) async { + testWidgetsOnAllPlatforms("caret can move around emoji without breaking editor", (tester) async { + // We are doing this to ensure the plugin doesn't make the editor crash when moving caret around emoji. + final plugin = StableTagPlugin(); await _pumpTestEditor( tester, @@ -1158,10 +1160,23 @@ void main() { plugin: plugin, ); + // Place the caret before the emoji: |πŸ’™ + await tester.placeCaretInParagraph("1", 0); + // Place the caret after the emoji: πŸ’™| - await tester.placeCaretInParagraph("1", 2); + await tester.pressRightArrow(); - // Move the caret before the emoji: |πŸ’™ + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 2), + ), + ), + ); + + // Move the caret back to initial position, before the emoji: |πŸ’™. await tester.pressLeftArrow(); expect( @@ -1174,12 +1189,37 @@ void main() { ), ); - // Type @ and ensure the string is valid: @πŸ’™ - await tester.typeImeText("@"); + // Ensure the paragraph string is well formed: πŸ’™ + final text = SuperEditorInspector.findTextInComponent("1"); + expect(text.text, "πŸ’™"); + }); + + testWidgetsOnAllPlatforms("can be captured with trigger", (tester) async { + final plugin = StableTagPlugin(); + await _pumpTestEditor( + tester, + MutableDocument( + nodes: [ + ParagraphNode( + id: "1", + text: AttributedText(""), + ), + ], + ), + plugin: plugin, + ); + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph("1", 0); + + // Type @ to trigger a composing tag, followed by an emoji πŸ’™ + await tester.typeImeText("@πŸ’™"); + + // Ensure the paragraph string is well formed: @πŸ’™ final text = SuperEditorInspector.findTextInComponent("1"); expect(text.text, "@πŸ’™"); - // Ensure the composing tag is on the emoji + // Ensure the composing tag is wrapping the emoji final composingStableTag = plugin.tagIndex.composingStableTag.value!; expect( composingStableTag, @@ -1198,9 +1238,6 @@ void main() { ), ); - // Move the caret after the emoji: @πŸ’™| - await tester.pressRightArrow(); - // Type a white space to commit the tag: @πŸ’™ | await tester.typeImeText(" "); @@ -1215,7 +1252,7 @@ void main() { expect(plugin.tagIndex.composingStableTag.value, isNull); }); - testWidgetsOnAllPlatforms("before trigger", (tester) async { + testWidgetsOnAllPlatforms("can be used before a trigger", (tester) async { final plugin = StableTagPlugin(); await _pumpTestEditor( tester, @@ -1233,9 +1270,8 @@ void main() { // Place the caret after the emoji: πŸ’™| await tester.placeCaretInParagraph("1", 2); - // Type @ and ensure the string is valid: πŸ’™@ + // Type @ to trigger a composing tag and ensure the string is well formed: πŸ’™@ await tester.typeImeText("@"); - final text = SuperEditorInspector.findTextInComponent("1"); expect(text.text, "πŸ’™@"); @@ -1258,7 +1294,7 @@ void main() { ); }); - testWidgetsOnAllPlatforms("in the middle of a tag", (tester) async { + testWidgetsOnAllPlatforms("can be used in the middle of a tag", (tester) async { final plugin = StableTagPlugin(); await _pumpTestEditor( tester, From 3299552cfef8bd46b56cb5ba8e0aa36d5df86c6e Mon Sep 17 00:00:00 2001 From: Corentin Bazin Date: Mon, 15 Apr 2024 14:34:29 +0200 Subject: [PATCH 5/5] remove consecutive triggers test --- .../text_entry/tagging/stable_tags_test.dart | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart index fbbb07101..e86629f4a 100644 --- a/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/stable_tags_test.dart @@ -598,48 +598,6 @@ void main() { const SpanRange(7, 11), ); }); - - testWidgetsOnAllPlatforms("with consecutive triggers", (tester) async { - final plugin = StableTagPlugin(); - await _pumpTestEditor( - tester, - singleParagraphEmptyDoc(), - plugin: plugin, - ); - await tester.placeCaretInParagraph("1", 0); - - // Type two consecutive trigger characters. - await tester.typeImeText("@@"); - - final composingStableTag = plugin.tagIndex.composingStableTag.value!; - - // Ensure the composing tag is placed on the second @, with empty token. - expect( - composingStableTag, - const ComposingStableTag( - DocumentRange( - start: DocumentPosition( - nodeId: '1', - nodePosition: TextNodePosition(offset: 2), - ), - end: DocumentPosition( - nodeId: '1', - nodePosition: TextNodePosition(offset: 2), - ), - ), - '', - ), - ); - - final commitedTags = plugin.tagIndex.getCommittedTagsInTextNode('1'); - - expect(commitedTags.length, 1); - - final commitTag = commitedTags.first; - - // Ensure the committed tag is the first @ - expect(commitTag, const IndexedTag(Tag('@', ''), '1', 0)); - }); }); group("committed >", () {