Skip to content

Commit 21883a0

Browse files
Cherry Pick: [SuperEditor] - Fix: Paste command remembers the nodes it inserted to play nice with undo/redo (Resolves #2173) (#2234)
1 parent 285101c commit 21883a0

File tree

6 files changed

+169
-35
lines changed

6 files changed

+169
-35
lines changed

super_editor/example/lib/main_super_editor.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ void main() {
1818
// editorImeDeltasLog,
1919
// editorIosFloatingCursorLog,
2020
// editorKeyLog,
21+
// editorEditsLog,
2122
// editorOpsLog,
2223
// editorLayoutLog,
2324
// editorDocLog,
@@ -178,7 +179,9 @@ class _EditorHistoryPanelState extends State<_EditorHistoryPanel> {
178179
titleTextStyle: TextStyle(
179180
fontSize: 16,
180181
),
181-
subtitle: Text("${history.changes.map((event) => event.describe()).join("\n")}"),
182+
subtitle: Text(
183+
"${history.commands.map((command) => command.describe()).join("\n\n")}\n-------------\n${history.changes.map((event) => event.describe()).join("\n\n")}",
184+
),
182185
subtitleTextStyle: TextStyle(
183186
color: Colors.white.withOpacity(0.5),
184187
fontSize: 10,

super_editor/lib/src/core/document_composer.dart

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,10 @@ class ChangeSelectionCommand extends EditCommand {
367367
void execute(EditContext context, CommandExecutor executor) {
368368
final composer = context.find<MutableDocumentComposer>(Editor.composerKey);
369369
final initialSelection = composer.selection;
370+
if (initialSelection == newSelection) {
371+
// Selection is already where it should be.
372+
return;
373+
}
370374

371375
composer.setSelectionWithReason(newSelection, reason);
372376

@@ -397,7 +401,24 @@ class SelectionChangeEvent extends EditEvent {
397401
final String reason;
398402

399403
@override
400-
String describe() => "Selection - ${changeType.name}, $reason";
404+
String describe() {
405+
final buffer = StringBuffer("Selection - ${changeType.name}, $reason");
406+
if (newSelection == null) {
407+
buffer.write(" (SELECTION REMOVED)");
408+
return buffer.toString();
409+
}
410+
411+
if (newSelection!.isCollapsed) {
412+
buffer.write(" (at ${newSelection!.extent.nodeId} - ${newSelection!.extent.nodePosition}");
413+
return buffer.toString();
414+
}
415+
416+
buffer
417+
..writeln("")
418+
..writeln(" - from: ${newSelection!.base.nodeId} - ${newSelection!.base.nodePosition}")
419+
..write(" - to: ${newSelection!.extent.nodeId} - ${newSelection!.extent.nodePosition}");
420+
return buffer.toString();
421+
}
401422

402423
@override
403424
String toString() => "[SelectionChangeEvent] - New selection: $newSelection, change type: $changeType";

super_editor/lib/src/core/editor.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,6 @@ class Editor implements RequestDispatcher {
410410
final changeEvents = <EditEvent>[];
411411
for (final commandTransaction in _history) {
412412
for (final command in commandTransaction.commands) {
413-
editorEditsLog.finer("Executing command: ${command.runtimeType}");
414413
// We re-run the commands without tracking changes and without running reactions
415414
// because any relevant reactions should have run the first time around, and already
416415
// submitted their commands.

super_editor/lib/src/default_editor/common_editor_operations.dart

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2265,11 +2265,28 @@ class PasteEditorCommand extends EditCommand {
22652265
final String _content;
22662266
final DocumentPosition _pastePosition;
22672267

2268+
// The [_content] as [DocumentNode]s so that we only generate node IDs one
2269+
// time. This is critical for undo behavior to work as expected.
2270+
List<DocumentNode>? _parsedContent;
2271+
22682272
@override
22692273
HistoryBehavior get historyBehavior => HistoryBehavior.undoable;
22702274

22712275
@override
22722276
void execute(EditContext context, CommandExecutor executor) {
2277+
// Only parse the content if we haven't done it already. This command
2278+
// might be run 2+ times if the user runs an undo operation.
2279+
_parsedContent ??= _parseContent();
2280+
2281+
// Assign locally so we don't have to use a "!" everywhere we reference it.
2282+
// Also, make a copy of the existing nodes so that when the document mutates,
2283+
// our local copy doesn't change.
2284+
final parsedContent = _parsedContent!.map((node) => node.copy()).toList();
2285+
if (parsedContent.isEmpty) {
2286+
// No content to paste.
2287+
return;
2288+
}
2289+
22732290
final document = context.document;
22742291
final composer = context.find<MutableDocumentComposer>(Editor.composerKey);
22752292
final currentNodeWithSelection = document.getNodeById(_pastePosition.nodeId);
@@ -2279,14 +2296,10 @@ class PasteEditorCommand extends EditCommand {
22792296

22802297
editorOpsLog.info("Pasting clipboard content in document.");
22812298

2282-
// Split the pasted content at newlines, and apply attributions based
2283-
// on inspection of the pasted content, e.g., link attributions.
2284-
final attributedLines = _inferAttributionsForLinesOfPastedText(_content);
2285-
22862299
final textNode = document.getNode(_pastePosition) as TextNode;
22872300
final pasteTextOffset = (_pastePosition.nodePosition as TextPosition).offset;
22882301

2289-
if (attributedLines.length > 1 && pasteTextOffset < textNode.endPosition.offset) {
2302+
if (parsedContent.length > 1 && pasteTextOffset < textNode.endPosition.offset) {
22902303
// There is more than 1 node of content being pasted. Therefore,
22912304
// new nodes will need to be added, which means that the currently
22922305
// selected text node will be split at the current text offset.
@@ -2303,20 +2316,20 @@ class PasteEditorCommand extends EditCommand {
23032316
);
23042317
}
23052318

2306-
// Paste the first piece of attributed content into the selected TextNode.
2307-
executor.executeCommand(
2308-
InsertAttributedTextCommand(
2309-
documentPosition: _pastePosition,
2310-
textToInsert: attributedLines.first,
2311-
),
2312-
);
2319+
if (parsedContent.first is TextNode) {
2320+
// Paste the first piece of attributed content into the existing selected TextNode.
2321+
executor.executeCommand(
2322+
InsertAttributedTextCommand(
2323+
documentPosition: _pastePosition,
2324+
textToInsert: (parsedContent.first as TextNode).text,
2325+
),
2326+
);
2327+
}
23132328

23142329
// The first line of pasted text was added to the selected paragraph.
2315-
// Now, create new nodes for each additional line of pasted text and
2316-
// insert those nodes.
2317-
final pastedContentNodes = _convertLinesToParagraphs(attributedLines.sublist(1));
2330+
// Now, add all remaining pasted nodes to the document..
23182331
DocumentNode previousNode = currentNodeWithSelection;
2319-
for (final pastedNode in pastedContentNodes) {
2332+
for (final pastedNode in parsedContent.sublist(1)) {
23202333
document.insertNodeAfter(
23212334
existingNode: previousNode,
23222335
newNode: pastedNode,
@@ -2334,17 +2347,10 @@ class PasteEditorCommand extends EditCommand {
23342347
executor.executeCommand(
23352348
ChangeSelectionCommand(
23362349
DocumentSelection.collapsed(
2337-
position: pastedContentNodes.isNotEmpty
2338-
? DocumentPosition(
2339-
nodeId: previousNode.id,
2340-
nodePosition: previousNode.endPosition,
2341-
)
2342-
: DocumentPosition(
2343-
nodeId: currentNodeWithSelection.id,
2344-
nodePosition: TextNodePosition(
2345-
offset: pasteTextOffset + attributedLines.first.text.length,
2346-
),
2347-
),
2350+
position: DocumentPosition(
2351+
nodeId: previousNode.id,
2352+
nodePosition: previousNode.endPosition,
2353+
),
23482354
),
23492355
SelectionChangeType.insertContent,
23502356
SelectionReason.userInteraction,
@@ -2354,6 +2360,13 @@ class PasteEditorCommand extends EditCommand {
23542360
editorOpsLog.fine('Done with paste command.');
23552361
}
23562362

2363+
List<DocumentNode> _parseContent() {
2364+
// Split the pasted content at newlines, and apply attributions based
2365+
// on inspection of the pasted content, e.g., link attributions.
2366+
final attributedLines = _inferAttributionsForLinesOfPastedText(_content);
2367+
return _convertLinesToParagraphs(attributedLines).toList();
2368+
}
2369+
23572370
/// Breaks the given [content] at each newline, then applies any inferred
23582371
/// attributions based on content analysis, e.g., surrounds URLs with
23592372
/// [LinkAttribution]s.

super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,11 +333,12 @@ class TextDeltasDocumentEditor {
333333
editorOpsLog.fine("Executing text insertion command.");
334334
editorOpsLog.finer("Text before insertion: '${insertionNode.text.text}'");
335335
editor.execute([
336-
ChangeSelectionRequest(
337-
DocumentSelection.collapsed(position: insertionPosition),
338-
SelectionChangeType.placeCaret,
339-
SelectionReason.userInteraction,
340-
),
336+
if (selection.value != DocumentSelection.collapsed(position: insertionPosition))
337+
ChangeSelectionRequest(
338+
DocumentSelection.collapsed(position: insertionPosition),
339+
SelectionChangeType.placeCaret,
340+
SelectionReason.userInteraction,
341+
),
341342
InsertTextRequest(
342343
documentPosition: insertionPosition,
343344
textToInsert: text,

super_editor/test/super_editor/supereditor_copy_and_paste_test.dart

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import 'package:flutter/foundation.dart';
2+
import 'package:flutter/services.dart';
23
import 'package:flutter_test/flutter_test.dart';
34
import 'package:flutter_test_robots/flutter_test_robots.dart';
5+
import 'package:flutter_test_runners/flutter_test_runners.dart';
46
import 'package:super_editor/super_editor.dart';
57
import 'package:super_editor/super_editor_test.dart';
68

@@ -90,5 +92,100 @@ This is the third paragraph''');
9092
expect((doc.getNodeAt(1)! as ParagraphNode).text.text, 'This is a second paragraph');
9193
expect((doc.getNodeAt(2)! as ParagraphNode).text.text, 'This is the third paragraph');
9294
});
95+
96+
testAllInputsOnAllPlatforms("paste retains node IDs when replayed during undo", (
97+
tester, {
98+
required TextInputSource inputSource,
99+
}) async {
100+
final testContext = await tester //
101+
.createDocument()
102+
.withSingleEmptyParagraph()
103+
.withInputSource(inputSource)
104+
.pump();
105+
106+
// Place the caret at the empty paragraph.
107+
await tester.placeCaretInParagraph('1', 0);
108+
109+
// Simulate pasting multiple lines.
110+
tester
111+
..simulateClipboard()
112+
..setSimulatedClipboardContent('''This is a paragraph
113+
This is a second paragraph
114+
This is the third paragraph''');
115+
if (defaultTargetPlatform == TargetPlatform.macOS) {
116+
await tester.pressCmdV();
117+
} else {
118+
await tester.pressCtlV();
119+
}
120+
121+
// Gather the current node IDs in the document.
122+
final originalNodeIds = testContext.document.toList().map((node) => node.id).toList();
123+
124+
// Pump enough time to separate the next text entry from the paste action.
125+
await tester.pump(const Duration(seconds: 2));
126+
127+
// Type some text.
128+
switch (inputSource) {
129+
case TextInputSource.keyboard:
130+
await tester.pressKey(LogicalKeyboardKey.keyA);
131+
case TextInputSource.ime:
132+
await tester.typeImeText("a");
133+
}
134+
135+
// Undo the text insertion (this causes the paste command re-run).
136+
testContext.editor.undo();
137+
138+
// Ensure that the node IDs in the document didn't change after re-running
139+
// the paste command.
140+
final newNodeIds = testContext.document.toList().map((node) => node.id).toList();
141+
expect(newNodeIds, originalNodeIds);
142+
});
143+
144+
testWidgetsOnMac("paste command content does not mutate when document changes", (tester) async {
145+
final testContext = await tester //
146+
.createDocument()
147+
.withSingleEmptyParagraph()
148+
.enableHistory(true)
149+
.pump();
150+
151+
// Place the caret at the empty paragraph.
152+
await tester.placeCaretInParagraph('1', 0);
153+
154+
// Simulate pasting multiple lines.
155+
tester
156+
..simulateClipboard()
157+
..setSimulatedClipboardContent('''This is a paragraph
158+
This is a second paragraph
159+
This is the third paragraph''');
160+
if (defaultTargetPlatform == TargetPlatform.macOS) {
161+
await tester.pressCmdV();
162+
} else {
163+
await tester.pressCtlV();
164+
}
165+
166+
// Pump enough time to separate the next text entry from the paste action.
167+
await tester.pump(const Duration(seconds: 2));
168+
await tester.typeImeText("a");
169+
170+
// Ensure that the "a" was inserted at the end of the final pasted paragraph.
171+
expect(
172+
(testContext.document.last as TextNode).text.text,
173+
"This is the third paragrapha",
174+
);
175+
176+
// Run undo.
177+
testContext.editor.undo();
178+
179+
// After undo, ensure that we no longer have the inserted "a".
180+
//
181+
// The undo operation works by replaying earlier commands, such as the paste command.
182+
// The paste command internally stores the content that it inserted. This test ensures
183+
// that the paste command's internal content wasn't mutated when we inserted the "a"
184+
// into the document. Such mutation was part of bug https://github.com/superlistapp/super_editor/issues/2173
185+
expect(
186+
(testContext.document.last as TextNode).text.text,
187+
"This is the third paragraph",
188+
);
189+
});
93190
});
94191
}

0 commit comments

Comments
 (0)