Skip to content

Commit e4561f8

Browse files
Fix crash in linkification reaction, also add linkification to inline markdown parsing (Resolves #2074) (#2078)
1 parent bae3cbb commit e4561f8

File tree

7 files changed

+262
-10
lines changed

7 files changed

+262
-10
lines changed

super_editor/lib/src/default_editor/common_editor_operations.dart

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2391,10 +2391,19 @@ class PasteEditorCommand implements EditCommand {
23912391
looseUrl: true,
23922392
),
23932393
);
2394+
23942395
final int linkCount = extractedLinks.fold(0, (value, element) => element is UrlElement ? value + 1 : value);
23952396
if (linkCount == 1) {
23962397
// The word is a single URL. Linkify it.
2397-
final uri = parseLink(word);
2398+
late final Uri uri;
2399+
try {
2400+
uri = parseLink(word);
2401+
} catch (exception) {
2402+
// Something went wrong when trying to parse links. This can happen, for example,
2403+
// due to Markdown syntax around a link, e.g., [My Link](www.something.com). I'm
2404+
// not sure why that case throws, but it does. We ignore any URL that throws.
2405+
continue;
2406+
}
23982407

23992408
final startOffset = wordBoundary.start;
24002409
// -1 because TextPosition's offset indexes the character after the

super_editor/lib/src/default_editor/default_document_editor_reactions.dart

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,8 @@ class LinkifyReaction implements EditReaction {
561561
void react(EditContext editContext, RequestDispatcher requestDispatcher, List<EditEvent> edits) {
562562
final document = editContext.find<MutableDocument>(Editor.documentKey);
563563
final composer = editContext.find<MutableDocumentComposer>(Editor.composerKey);
564+
final selection = composer.selection;
565+
564566
bool didInsertSpace = false;
565567

566568
TextInsertionEvent? linkifyCandidate;
@@ -579,22 +581,25 @@ class LinkifyReaction implements EditReaction {
579581
continue;
580582
}
581583

582-
if (edit.newSelection == null) {
584+
if (selection == null) {
583585
// The editor doesn't have a selection. Don't linkify.
584586
linkifyCandidate = null;
585587
continue;
586-
} else if (!edit.newSelection!.isCollapsed) {
588+
}
589+
590+
if (!selection.isCollapsed) {
587591
// The selection is expanded. Don't linkify.
588592
linkifyCandidate = null;
589593
continue;
590594
}
591595

592-
final caretPosition = edit.newSelection!.extent;
596+
final caretPosition = selection.extent;
593597
if (caretPosition.nodeId != linkifyCandidate.nodeId) {
594598
// The selection moved to some other node. Don't linkify.
595599
linkifyCandidate = null;
596600
continue;
597601
}
602+
598603
// +1 for the inserted space
599604
if ((caretPosition.nodePosition as TextNodePosition).offset != linkifyCandidate.offset + 1) {
600605
// The caret isn't sitting directly after the space. Whatever
@@ -662,14 +667,23 @@ class LinkifyReaction implements EditReaction {
662667
),
663668
);
664669
final int linkCount = extractedLinks.fold(0, (value, element) => element is UrlElement ? value + 1 : value);
665-
if (linkCount == 1) {
666-
// The word is a single URL. Linkify it.
670+
if (linkCount != 1) {
671+
// There's either zero links, or more than one link. Either way we fizzle.
672+
return;
673+
}
674+
675+
// The word is a single URL. Linkify it.
676+
try {
677+
// Try to parse the word as a link.
667678
final uri = parseLink(word);
668679

669680
text.addAttribution(
670681
LinkAttribution.fromUri(uri),
671682
SpanRange(wordStartOffset, endOffset - 1),
672683
);
684+
} catch (exception) {
685+
// Something went wrong parsing the link. Fizzle.
686+
return;
673687
}
674688
}
675689

super_editor/lib/src/default_editor/multi_node_editing.dart

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,19 @@ class PasteStructuredContentEditorCommand implements EditCommand {
7575
textToInsert: (pastedNode as TextNode).text,
7676
),
7777
);
78+
executor.executeCommand(
79+
ChangeSelectionCommand(
80+
DocumentSelection.collapsed(
81+
position: DocumentPosition(
82+
nodeId: pastePosition.nodeId,
83+
nodePosition: TextNodePosition(
84+
offset: (pastePosition.nodePosition as TextNodePosition).offset + pastedNode.text.length),
85+
),
86+
),
87+
SelectionChangeType.insertContent,
88+
SelectionReason.userInteraction,
89+
),
90+
);
7891

7992
return;
8093
}

super_editor/test/super_editor/text_entry/links_test.dart

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,56 @@ void main() {
20352035
);
20362036
});
20372037

2038+
testWidgetsOnAllPlatforms('plays nice with Markdown link when Markdown parsing is disabled', (tester) async {
2039+
// Based on bug #2074 - https://github.com/superlistapp/super_editor/issues/2074
2040+
await tester //
2041+
.createDocument()
2042+
.withSingleEmptyParagraph()
2043+
.withInputSource(TextInputSource.ime)
2044+
.pump();
2045+
2046+
await tester.placeCaretInParagraph("1", 0);
2047+
2048+
await tester.typeImeText("[google](www.google.com) ");
2049+
2050+
// Ensure that the Markdown was ignored and nothing was linkified.
2051+
final text = SuperEditorInspector.findTextInComponent("1");
2052+
expect(text.text, "[google](www.google.com) ");
2053+
expect(text.getAttributionSpansByFilter((a) => true), isEmpty);
2054+
});
2055+
2056+
testWidgetsOnMac('plays nice with Markdown link when pasting a Markdown link', (tester) async {
2057+
// Based on bug #2074 - https://github.com/superlistapp/super_editor/issues/2074
2058+
await tester //
2059+
.createDocument()
2060+
.withSingleEmptyParagraph()
2061+
.withInputSource(TextInputSource.ime)
2062+
.pump();
2063+
2064+
await tester.placeCaretInParagraph("1", 0);
2065+
2066+
// Simulate copying a Markdown link to the clipboard.
2067+
tester.simulateClipboard();
2068+
await tester.setSimulatedClipboardContent("Hello [google](www.google.com) ");
2069+
2070+
// Simulate pasting the Markdown link into the document.
2071+
await tester.pressCmdV();
2072+
2073+
// Ensure that the Markdown was ignored and nothing was linkified.
2074+
final text = SuperEditorInspector.findTextInComponent("1");
2075+
expect(text.text, "Hello [google](www.google.com) ");
2076+
expect(text.getAttributionSpansByFilter((a) => true), isEmpty);
2077+
expect(
2078+
SuperEditorInspector.findDocumentSelection(),
2079+
const DocumentSelection.collapsed(
2080+
position: DocumentPosition(
2081+
nodeId: "1",
2082+
nodePosition: TextNodePosition(offset: 31),
2083+
),
2084+
),
2085+
);
2086+
});
2087+
20382088
// TODO: once it's easier to configure task components (#1295), add a test that checks link attributions when inserting a new task
20392089
});
20402090
}

super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,7 @@ const defaultUpstreamInlineMarkdownParsers = [
5353
/// caret and converts it into attributions.
5454
///
5555
/// Inline Markdown syntax includes things like `**token**` for bold, `*token*` for
56-
/// italics, and `~token~` for strikethrough. Links aren't included because their complex
57-
/// syntax makes upstream parsing a poor strategy for identifying them. For linkification,
58-
/// consider using a batch Markdown parsing approach, or consider identifying URLs directly,
59-
/// without requiring any Markdown syntax.
56+
/// italics, `~token~` for strikethrough, and `[name](url)` for links.
6057
///
6158
/// When this reaction finds inline Markdown syntax, that syntax is removed when the corresponding
6259
/// attribution is applied. For example, "**bold**" becomes "bold" with a bold attribution
@@ -65,6 +62,10 @@ const defaultUpstreamInlineMarkdownParsers = [
6562
/// This reaction only identifies spans of Markdown styles within individual [TextNode]s, which
6663
/// immediately precedes the caret. For example, "Hello **bold**|" will apply the bold style,
6764
/// but "Hello **bold** wo|" won't apply bold.
65+
///
66+
/// Parsing of links is handled differently than all other upstream syntax. Links use a fairly
67+
/// complicated syntax, so they're identified with a regular expression. All other upstream
68+
/// inline syntaxes are parsed character by character, moving upstream from the caret position.
6869
class MarkdownInlineUpstreamSyntaxReaction implements EditReaction {
6970
const MarkdownInlineUpstreamSyntaxReaction(this._parsers);
7071

@@ -237,6 +238,14 @@ class _UpstreamInlineMarkdownParser {
237238
return null;
238239
}
239240

241+
// Run the special case of parsing a link. This is a special case because the syntax
242+
// is complicated enough that we don't want to try to accumulate characters as the
243+
// user types.
244+
final linkRun = _tryCreateLinkRun();
245+
if (linkRun != null) {
246+
return linkRun;
247+
}
248+
240249
int offset = caretOffset - 1;
241250

242251
// Start visiting upstream characters by visiting the first character
@@ -295,6 +304,66 @@ class _UpstreamInlineMarkdownParser {
295304
);
296305
}
297306

307+
_InlineMarkdownRun? _tryCreateLinkRun() {
308+
if (caretOffset < 2) {
309+
// There's not enough text before the caret to possibly hold a link. Fizzle.
310+
return null;
311+
}
312+
313+
final characterAtCaret = attributedText.text[caretOffset - 1]; // -1 because caret sits after character
314+
if (characterAtCaret != " ") {
315+
// Don't linkify unless the user just inserted a space after the token.
316+
return null;
317+
}
318+
319+
final endOfTokenOffset = caretOffset - 2;
320+
if (attributedText.text[endOfTokenOffset] != ")") {
321+
// All links end with a ")", therefore we know the upstream token
322+
// isn't a link. Short-circuit return.
323+
return null;
324+
}
325+
326+
final markdownLinkRegex = RegExp(r'\[([\w\s\d]+)]\(((?:|https?://)[\w\d./?=#]+)\)');
327+
final matches = markdownLinkRegex.allMatches(attributedText.text);
328+
if (matches.isEmpty) {
329+
// Didn't find any links.
330+
return null;
331+
}
332+
333+
// Found some links. See if any of them are immediately upstream.
334+
for (final match in matches) {
335+
if (match.end - 1 == endOfTokenOffset) {
336+
// We found a Markdown link immediately upstream. Return it.
337+
final linkName = match.group(1)!;
338+
final linkUrl = match.group(2)!;
339+
final linkAttribution = LinkAttribution(linkUrl);
340+
341+
return _InlineMarkdownRun(
342+
AttributedText(
343+
"$linkName ", // Explicitly add the trailing space so the caret stays after the space.
344+
AttributedSpans(attributions: [
345+
SpanMarker(
346+
attribution: linkAttribution,
347+
offset: 0,
348+
markerType: SpanMarkerType.start,
349+
),
350+
SpanMarker(
351+
attribution: linkAttribution,
352+
offset: linkName.length - 1,
353+
markerType: SpanMarkerType.end,
354+
),
355+
]),
356+
),
357+
match.start,
358+
match.end + 1, // +1 to include the space after the link so the caret stays in same place.
359+
);
360+
}
361+
}
362+
363+
// We found links, but none of them are immediately upstream.
364+
return null;
365+
}
366+
298367
void _updatePossibleSyntaxes(String character, int characterIndex) {
299368
// Update all existing possible syntaxes.
300369
for (int i = _possibleSyntaxes.length - 1; i >= 0; i -= 1) {

super_editor_markdown/test/markdown_inline_upstream_plugin_test.dart

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,68 @@ void main() {
476476
expect(SuperEditorInspector.findTextInComponent(nodeId).spans.markers, isEmpty);
477477
});
478478
});
479+
480+
group("parses Markdown link >", () {
481+
testWidgets("but only when a space follows the syntax", (tester) async {
482+
final (document, _) = await _pumpScaffold(tester);
483+
484+
final nodeId = document.nodes.first.id;
485+
await tester.placeCaretInParagraph(nodeId, 0);
486+
487+
// Enter a link syntax, but no characters after it.
488+
await tester.typeImeText("[google](www.google.com)");
489+
490+
// Ensure the syntax wasn't linkified.
491+
var text = SuperEditorInspector.findTextInComponent(nodeId);
492+
expect(text.text, "[google](www.google.com)");
493+
expect(text.getAttributionSpansByFilter((a) => true), isEmpty);
494+
495+
// Enter a non-space character.
496+
await tester.typeImeText("a");
497+
498+
// Ensure we still haven't linkified
499+
text = SuperEditorInspector.findTextInComponent(nodeId);
500+
expect(text.text, "[google](www.google.com)a");
501+
expect(text.getAttributionSpansByFilter((a) => true), isEmpty);
502+
503+
// Enter a space after the non-space character.
504+
await tester.typeImeText(" ");
505+
506+
// Ensure we still haven't linkified
507+
text = SuperEditorInspector.findTextInComponent(nodeId);
508+
expect(text.text, "[google](www.google.com)a ");
509+
expect(text.getAttributionSpansByFilter((a) => true), isEmpty);
510+
});
511+
512+
testWidgets("parses Markdown link syntax and plays nice with built-in linkification reaction", (tester) async {
513+
final (document, _) = await _pumpScaffold(tester);
514+
515+
final nodeId = document.nodes.first.id;
516+
await tester.placeCaretInParagraph(nodeId, 0);
517+
518+
await tester.typeImeText("[google](www.google.com) ");
519+
520+
// Ensure that the Markdown was parsed and replaced with a link.
521+
final text = SuperEditorInspector.findTextInComponent(nodeId);
522+
expect(text.text, "google ");
523+
expect(text.getAttributionSpansByFilter((a) => true), {
524+
const AttributionSpan(
525+
attribution: LinkAttribution("www.google.com"),
526+
start: 0,
527+
end: 5,
528+
),
529+
});
530+
expect(
531+
SuperEditorInspector.findDocumentSelection(),
532+
DocumentSelection.collapsed(
533+
position: DocumentPosition(
534+
nodeId: nodeId,
535+
nodePosition: const TextNodePosition(offset: 7),
536+
),
537+
),
538+
);
539+
});
540+
});
479541
});
480542
}
481543

super_editor_markdown/test/super_editor_markdown_pasting_test.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,41 @@ Phasellus sed sagittis urna.
351351
Aenean mattis ante justo, quis sollicitudin metus interdum id.''',
352352
);
353353
});
354+
355+
testWidgetsOnMac("can paste a link", (tester) async {
356+
final (_, document, composer) = await _pumpSuperEditor(
357+
tester,
358+
deserializeMarkdownToDocument(""),
359+
);
360+
361+
// Place the caret in empty paragraph.
362+
final paragraph = document.nodes.first as TextNode;
363+
await tester.placeCaretInParagraph(paragraph.id, 0);
364+
365+
// Simulate the user copying a markdown snippet.
366+
tester
367+
..simulateClipboard()
368+
..setSimulatedClipboardContent("Hello [link](www.google.com)");
369+
370+
// Paste the markdown content into the empty document.
371+
await tester.pressCmdV();
372+
373+
// Ensure that the Markdown link was linkified.
374+
expect(SuperEditorInspector.findTextInComponent(paragraph.id).text, "Hello link");
375+
const expectedAttribution = LinkAttribution("www.google.com");
376+
expect(SuperEditorInspector.findTextInComponent(paragraph.id).getAttributionSpansByFilter((a) => true), {
377+
const AttributionSpan(attribution: expectedAttribution, start: 6, end: 9),
378+
});
379+
expect(
380+
SuperEditorInspector.findDocumentSelection(),
381+
DocumentSelection.collapsed(
382+
position: DocumentPosition(
383+
nodeId: paragraph.id,
384+
nodePosition: const TextNodePosition(offset: 10),
385+
),
386+
),
387+
);
388+
});
354389
});
355390
}
356391

0 commit comments

Comments
 (0)