From 8c89c7d1777eade3697ad751bfcbcd69707aee8f Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Tue, 19 Mar 2024 13:59:59 -0700 Subject: [PATCH 01/19] WIP: Undo/redo using snapshots with one-way commands on top. --- .../demos/in_the_lab/feature_action_tags.dart | 2 +- super_editor/lib/src/core/document.dart | 2 + .../lib/src/core/document_composer.dart | 37 +- .../lib/src/core/document_layout.dart | 5 +- super_editor/lib/src/core/editor.dart | 150 ++++++++- .../lib/src/default_editor/blockquote.dart | 6 + .../lib/src/default_editor/box_component.dart | 3 + .../common_editor_operations.dart | 9 + .../document_ime/document_delta_editing.dart | 37 +- .../src/default_editor/horizontal_rule.dart | 5 + .../lib/src/default_editor/image.dart | 11 + .../lib/src/default_editor/list_items.dart | 29 ++ .../default_editor/multi_node_editing.dart | 12 + .../lib/src/default_editor/paragraph.dart | 23 ++ .../lib/src/default_editor/super_editor.dart | 5 + .../lib/src/default_editor/tasks.dart | 22 ++ super_editor/lib/src/default_editor/text.dart | 26 ++ .../text_tokenizing/action_tags.dart | 6 + .../text_tokenizing/pattern_tags.dart | 5 + .../text_tokenizing/stable_tags.dart | 13 + super_editor/lib/src/undo_redo.dart | 43 +++ .../infrastructure/editor_test.dart | 3 + .../super_editor_undo_redo_test.dart | 318 ++++++++++++++++++ 23 files changed, 747 insertions(+), 25 deletions(-) create mode 100644 super_editor/lib/src/undo_redo.dart create mode 100644 super_editor/test/super_editor/super_editor_undo_redo_test.dart diff --git a/super_editor/example/lib/demos/in_the_lab/feature_action_tags.dart b/super_editor/example/lib/demos/in_the_lab/feature_action_tags.dart index f2a615cfd..071b7d1d5 100644 --- a/super_editor/example/lib/demos/in_the_lab/feature_action_tags.dart +++ b/super_editor/example/lib/demos/in_the_lab/feature_action_tags.dart @@ -298,7 +298,7 @@ class ConvertSelectedTextNodeRequest implements EditRequest { int get hashCode => newType.hashCode; } -class ConvertSelectedTextNodeCommand implements EditCommand { +class ConvertSelectedTextNodeCommand extends EditCommand { ConvertSelectedTextNodeCommand(this.newType); final TextNodeType newType; diff --git a/super_editor/lib/src/core/document.dart b/super_editor/lib/src/core/document.dart index 8c3f37642..deb206ab3 100644 --- a/super_editor/lib/src/core/document.dart +++ b/super_editor/lib/src/core/document.dart @@ -381,6 +381,8 @@ abstract class DocumentNode implements ChangeNotifier { /// Returns a copy of this node's metadata. Map copyMetadata() => Map.from(_metadata); + DocumentNode copy(); + @override bool operator ==(Object other) => identical(this, other) || diff --git a/super_editor/lib/src/core/document_composer.dart b/super_editor/lib/src/core/document_composer.dart index 30aedad38..affdc899c 100644 --- a/super_editor/lib/src/core/document_composer.dart +++ b/super_editor/lib/src/core/document_composer.dart @@ -108,6 +108,7 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { bool _isInTransaction = false; bool _didChangeSelectionDuringTransaction = false; + bool _didReset = false; /// Sets the current [selection] for a [Document]. /// @@ -117,6 +118,8 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { _didChangeSelectionDuringTransaction = true; } + print("Changing selection in composer to:"); + print("$newSelection"); _latestSelectionChange = DocumentSelectionChange( selection: newSelection, reason: reason, @@ -140,9 +143,7 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { @override void onTransactionStart() { _selectionNotifier.pauseNotifications(); - _composingRegion.pauseNotifications(); - _isInInteractionMode.pauseNotifications(); _isInTransaction = true; @@ -157,10 +158,27 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { if (_latestSelectionChange != null && _didChangeSelectionDuringTransaction) { _streamController.sink.add(_latestSelectionChange!); } - _composingRegion.resumeNotifications(); - _isInInteractionMode.resumeNotifications(); + + if (_didReset) { + print("Force notifying composer listeners. Current selection is:"); + print("${_selectionNotifier.value}"); + // Our state was reset (possibly for to undo an operation). Anything may have changed. + // Force notify all listeners. + _didReset = false; + _selectionNotifier.notifyListeners(); + _composingRegion.notifyListeners(); + _isInInteractionMode.notifyListeners(); + } + } + + @override + void reset() { + _selectionNotifier.value = null; + _latestSelectionChange = null; + _composingRegion.value = null; + _didReset = true; } } @@ -343,11 +361,16 @@ class ChangeSelectionCommand implements EditCommand { final String reason; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final composer = context.find(Editor.composerKey); final initialSelection = composer.selection; + print("ChangeSelectionCommand - Changing selection to:"); + print("$newSelection"); composer.setSelectionWithReason(newSelection, reason); executor.logChanges([ @@ -480,6 +503,9 @@ class ChangeComposingRegionCommand implements EditCommand { final DocumentRange? composingRegion; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final composer = context.find(Editor.composerKey); @@ -525,6 +551,9 @@ class ChangeInteractionModeCommand implements EditCommand { final bool isInteractionModeDesired; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.nonHistorical; + @override void execute(EditContext context, CommandExecutor executor) { context.find(Editor.composerKey).setIsInteractionMode(isInteractionModeDesired); diff --git a/super_editor/lib/src/core/document_layout.dart b/super_editor/lib/src/core/document_layout.dart index 84823ec77..9b2f6df88 100644 --- a/super_editor/lib/src/core/document_layout.dart +++ b/super_editor/lib/src/core/document_layout.dart @@ -24,11 +24,14 @@ class DocumentLayoutEditable implements Editable { DocumentLayout get documentLayout => _documentLayoutResolver(); + @override + void onTransactionStart() {} + @override void onTransactionEnd(List edits) {} @override - void onTransactionStart() {} + void reset() {} } /// Abstract representation of a document layout. diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index e95933e35..df67a886f 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -76,6 +76,9 @@ class Editor implements RequestDispatcher { /// Executes [EditCommand]s and collects a list of changes. late final _DocumentEditorCommandExecutor _commandExecutor; + final _commandHistory = []; + final _commandFuture = []; + /// A pipeline of objects that receive change-lists from command execution /// and get the first opportunity to spawn additional commands before the /// change list is dispatched to regular listeners. @@ -126,10 +129,10 @@ class Editor implements RequestDispatcher { /// of [EditEvent]s. @override void execute(List requests) { - // print("Request execution:"); - // for (final request in requests) { - // print(" - ${request.runtimeType}"); - // } + print("Request execution:"); + for (final request in requests) { + print(" - ${request.runtimeType}"); + } // print(StackTrace.current); if (_activeCommandCount == 0) { @@ -142,11 +145,22 @@ class Editor implements RequestDispatcher { _activeChangeList ??= []; _activeCommandCount += 1; + final undoableCommands = []; for (final request in requests) { // Execute the given request. final command = _findCommandForRequest(request); final commandChanges = _executeCommand(command); _activeChangeList!.addAll(commandChanges); + + if (command.historyBehavior == HistoryBehavior.undoable) { + undoableCommands.add(command); + } + } + + if (undoableCommands.isNotEmpty) { + _commandHistory.add( + CommandTransaction(undoableCommands), + ); } // Run reactions and notify listeners, but only do it once per batch of executions. @@ -219,6 +233,84 @@ class Editor implements RequestDispatcher { } } + void undo() { + print("Running undo()"); + if (_commandHistory.isEmpty) { + return; + } + + print("History before undo:"); + for (final transaction in _commandHistory) { + print(" - transaction"); + for (final command in transaction.commands) { + print(" - ${command.runtimeType}"); + } + } + print("---"); + + // Move the latest command from the history to the future. + _commandFuture.add(_commandHistory.removeLast()); + + for (final editable in context._resources.values) { + // Don't let editables notify listeners during undo. + editable.onTransactionStart(); + + // Revert all editables to the last snapshot. + editable.reset(); + } + + // Replay all history except for the most recent command transaction. + final changeEvents = []; + for (final commandTransaction in _commandHistory) { + print("Starting a command transaction."); + for (final command in commandTransaction.commands) { + print("Executing command: ${command.runtimeType}"); + // We re-run the commands without tracking changes and without running reactions + // because any relevant reactions should have run the first time around, and already + // submitted their commands. + final commandChanges = _executeCommand(command); + changeEvents.addAll(commandChanges); + } + print("Ending a command transaction."); + } + + for (final editable in context._resources.values) { + // Let editables start notifying listeners again. + editable.onTransactionEnd(changeEvents); + } + } + + void redo() { + print("Running redo()"); + if (_commandFuture.isEmpty) { + return; + } + + print("Future transaction:"); + for (final command in _commandFuture.last.commands) { + print(" - ${command.runtimeType}"); + } + + for (final editable in context._resources.values) { + // Don't let editables notify listeners during redo. + editable.onTransactionStart(); + } + + final commandTransaction = _commandFuture.removeLast(); + final edits = []; + for (final command in commandTransaction.commands) { + final commandEdits = _executeCommand(command); + edits.addAll(commandEdits); + } + _commandHistory.add(commandTransaction); + print("Done with redo()"); + + for (final editable in context._resources.values) { + // Let editables start notifying listeners again. + editable.onTransactionEnd(edits); + } + } + void _notifyListeners() { final changeList = List.from(_activeChangeList!, growable: false); for (final listener in _changeListeners) { @@ -229,6 +321,12 @@ class Editor implements RequestDispatcher { } } +class CommandTransaction { + const CommandTransaction(this.commands); + + final List commands; +} + /// An implementation of [CommandExecutor], designed for [Editor]. class _DocumentEditorCommandExecutor implements CommandExecutor { _DocumentEditorCommandExecutor(this._context); @@ -284,6 +382,8 @@ abstract mixin class Editable { /// A transaction that was previously started with [onTransactionStart] has now ended, this /// [Editable] should notify interested parties of changes. void onTransactionEnd(List edits) {} + + void reset() {} } /// An object that processes [EditRequest]s. @@ -296,6 +396,23 @@ abstract class RequestDispatcher { abstract class EditCommand { /// Executes this command and logs all changes with the [executor]. void execute(EditContext context, CommandExecutor executor); + + /// The desired "undo" behavior of this command. + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; +} + +/// The way a command interacts with the history ledger, AKA "undo". +enum HistoryBehavior { + /// The command can be undone and redone. + /// + /// For example: inserting text into a paragraph. + undoable, + + /// The command has no impact on history. + /// + /// For example: entering and exiting interaction mode, (possibly) activating and + /// deactivating bold/italics in the composer. + nonHistorical, } /// All resources that are available when executing [EditCommand]s, such as a document, @@ -496,6 +613,8 @@ class MutableDocument implements Document, Editable { List? nodes, }) : _nodes = nodes ?? [] { _refreshNodeIdCaches(); + + _latestNodesSnapshot = _nodes.map((node) => node.copy()).toList(); } /// Creates an [Document] with a single [ParagraphNode]. @@ -516,6 +635,9 @@ class MutableDocument implements Document, Editable { _listeners.clear(); } + late final List _latestNodesSnapshot; + bool _didReset = false; + final List _nodes; @override @@ -731,9 +853,10 @@ class MutableDocument implements Document, Editable { @override void onTransactionEnd(List edits) { final documentChanges = edits.whereType().map((edit) => edit.change).toList(); - if (documentChanges.isEmpty) { + if (documentChanges.isEmpty && !_didReset) { return; } + _didReset = false; final changeLog = DocumentChangeLog(documentChanges); for (final listener in _listeners) { @@ -741,6 +864,23 @@ class MutableDocument implements Document, Editable { } } + @override + void reset() { + print("Resetting the MutableDocument"); + + _nodes + ..clear() + ..addAll(_latestNodesSnapshot.map((node) => node.copy()).toList()); + _refreshNodeIdCaches(); + + _didReset = true; + + for (final node in _nodes) { + print("$node"); + } + print(""); + } + /// Updates all the maps which use the node id as the key. /// /// All the maps are cleared and re-populated. diff --git a/super_editor/lib/src/default_editor/blockquote.dart b/super_editor/lib/src/default_editor/blockquote.dart index 4a19e103f..73a717648 100644 --- a/super_editor/lib/src/default_editor/blockquote.dart +++ b/super_editor/lib/src/default_editor/blockquote.dart @@ -245,6 +245,9 @@ class ConvertBlockquoteToParagraphCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -335,6 +338,9 @@ class SplitBlockquoteCommand implements EditCommand { final TextPosition splitPosition; final String newNodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/box_component.dart b/super_editor/lib/src/default_editor/box_component.dart index c9784b256..b98e35de2 100644 --- a/super_editor/lib/src/default_editor/box_component.dart +++ b/super_editor/lib/src/default_editor/box_component.dart @@ -315,6 +315,9 @@ class DeleteUpstreamAtBeginningOfBlockNodeCommand implements EditCommand { final DocumentNode node; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index 47fc7a388..617e87b94 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -2257,6 +2257,9 @@ class PasteEditorCommand implements EditCommand { final DocumentPosition _pastePosition; final DocumentComposer _composer; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -2417,6 +2420,9 @@ class DeleteUpstreamCharacterRequest implements EditRequest { class DeleteUpstreamCharacterCommand implements EditCommand { const DeleteUpstreamCharacterCommand(); + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -2468,6 +2474,9 @@ class DeleteDownstreamCharacterRequest implements EditRequest { class DeleteDownstreamCharacterCommand implements EditCommand { const DeleteDownstreamCharacterCommand(); + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart index 223300452..13fd9f43c 100644 --- a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart +++ b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart @@ -97,11 +97,14 @@ class TextDeltasDocumentEditor { // the IME composing region. editorImeLog.fine("After applying all deltas, converting the final composing region to a document range."); editorImeLog.fine("Raw IME delta composing region: ${textEditingDeltas.last.composing}"); - editor.execute([ - ChangeComposingRegionRequest( - _serializedDoc.imeToDocumentRange(textEditingDeltas.last.composing), - ), - ]); + final newComposingRegion = _serializedDoc.imeToDocumentRange(textEditingDeltas.last.composing); + if (newComposingRegion != composingRegion.value) { + editor.execute([ + ChangeComposingRegionRequest( + _serializedDoc.imeToDocumentRange(textEditingDeltas.last.composing), + ), + ]); + } editorImeLog.fine("Document composing region: ${composingRegion.value}"); _nextImeValue = null; @@ -282,17 +285,17 @@ class TextDeltasDocumentEditor { editorImeLog .fine("Updating the Document Composer's selection to place caret at insertion offset:\n$insertionSelection"); final selectionBeforeInsertion = selection.value; - editor.execute([ - ChangeSelectionRequest( - insertionSelection, - SelectionChangeType.placeCaret, - SelectionReason.userInteraction, - ), - ]); + // editor.execute([ + // ChangeSelectionRequest( + // insertionSelection, + // SelectionChangeType.placeCaret, + // SelectionReason.userInteraction, + // ), + // ]); editorImeLog.fine("Inserting the text at the Document Composer's selection"); final didInsert = _insertPlainText( - insertionSelection.extent, + insertionSelection, textInserted, ); editorImeLog.fine("Insertion successful? $didInsert"); @@ -310,9 +313,10 @@ class TextDeltasDocumentEditor { } bool _insertPlainText( - DocumentPosition insertionPosition, + DocumentSelection insertionSelection, String text, ) { + DocumentPosition insertionPosition = insertionSelection.extent; editorOpsLog.fine('Attempting to insert "$text" at position: $insertionPosition'); DocumentNode? insertionNode = document.getNodeById(insertionPosition.nodeId); @@ -340,6 +344,11 @@ class TextDeltasDocumentEditor { editorOpsLog.fine("Executing text insertion command."); editorOpsLog.finer("Text before insertion: '${insertionNode.text.text}'"); editor.execute([ + ChangeSelectionRequest( + insertionSelection, + SelectionChangeType.placeCaret, + SelectionReason.userInteraction, + ), InsertTextRequest( documentPosition: insertionPosition, textToInsert: text, diff --git a/super_editor/lib/src/default_editor/horizontal_rule.dart b/super_editor/lib/src/default_editor/horizontal_rule.dart index 88bee9497..896390c93 100644 --- a/super_editor/lib/src/default_editor/horizontal_rule.dart +++ b/super_editor/lib/src/default_editor/horizontal_rule.dart @@ -32,6 +32,11 @@ class HorizontalRuleNode extends BlockNode with ChangeNotifier { return other is HorizontalRuleNode; } + @override + HorizontalRuleNode copy() { + return HorizontalRuleNode(id: id); + } + @override bool operator ==(Object other) => identical(this, other) || other is HorizontalRuleNode && runtimeType == other.runtimeType && id == other.id; diff --git a/super_editor/lib/src/default_editor/image.dart b/super_editor/lib/src/default_editor/image.dart index 74bea8b23..b27e399a5 100644 --- a/super_editor/lib/src/default_editor/image.dart +++ b/super_editor/lib/src/default_editor/image.dart @@ -81,6 +81,17 @@ class ImageNode extends BlockNode with ChangeNotifier { return other is ImageNode && imageUrl == other.imageUrl && altText == other.altText; } + @override + ImageNode copy() { + return ImageNode( + id: id, + imageUrl: imageUrl, + expectedBitmapSize: expectedBitmapSize, + altText: altText, + metadata: Map.from(metadata), + ); + } + @override bool operator ==(Object other) => identical(this, other) || diff --git a/super_editor/lib/src/default_editor/list_items.dart b/super_editor/lib/src/default_editor/list_items.dart index 1e7f26973..f573acece 100644 --- a/super_editor/lib/src/default_editor/list_items.dart +++ b/super_editor/lib/src/default_editor/list_items.dart @@ -81,6 +81,17 @@ class ListItemNode extends TextNode { return other is ListItemNode && type == other.type && indent == other.indent && text == other.text; } + @override + ListItemNode copy() { + return ListItemNode( + id: id, + text: text.copyText(0), + itemType: type, + indent: indent, + metadata: Map.from(metadata), + ); + } + @override bool operator ==(Object other) => identical(this, other) || @@ -521,6 +532,9 @@ class IndentListItemCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -556,6 +570,9 @@ class UnIndentListItemCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -600,6 +617,9 @@ class ConvertListItemToParagraphCommand implements EditCommand { final String nodeId; final Map? paragraphMetadata; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -644,6 +664,9 @@ class ConvertParagraphToListItemCommand implements EditCommand { final String nodeId; final ListItemType type; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -684,6 +707,9 @@ class ChangeListItemTypeCommand implements EditCommand { final String nodeId; final ListItemType newType; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -727,6 +753,9 @@ class SplitListItemCommand implements EditCommand { final TextPosition splitPosition; final String newNodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/multi_node_editing.dart b/super_editor/lib/src/default_editor/multi_node_editing.dart index 4fea9d552..133895f95 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -38,6 +38,9 @@ class PasteStructuredContentEditorCommand implements EditCommand { final List _content; final DocumentPosition _pastePosition; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { if (_content.isEmpty) { @@ -607,6 +610,9 @@ class ReplaceNodeWithEmptyParagraphWithCaretCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -660,6 +666,9 @@ class DeleteContentCommand implements EditCommand { final DocumentRange documentRange; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { _log.log('DeleteSelectionCommand', 'DocumentEditor: deleting selection: $documentRange'); @@ -1037,6 +1046,9 @@ class DeleteNodeCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { _log.log('DeleteNodeCommand', 'DocumentEditor: deleting node: $nodeId'); diff --git a/super_editor/lib/src/default_editor/paragraph.dart b/super_editor/lib/src/default_editor/paragraph.dart index 4e9bf85dd..3b1526632 100644 --- a/super_editor/lib/src/default_editor/paragraph.dart +++ b/super_editor/lib/src/default_editor/paragraph.dart @@ -33,6 +33,11 @@ class ParagraphNode extends TextNode { putMetadataValue("blockType", paragraphAttribution); } } + + @override + ParagraphNode copy() { + return ParagraphNode(id: id, text: text.copyText(0), metadata: Map.from(metadata)); + } } class ParagraphComponentBuilder implements ComponentBuilder { @@ -230,6 +235,9 @@ class ChangeParagraphAlignmentCommand implements EditCommand { final String nodeId; final TextAlign alignment; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -293,6 +301,9 @@ class ChangeParagraphBlockTypeCommand implements EditCommand { final String nodeId; final Attribution? blockType; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -336,6 +347,9 @@ class CombineParagraphsCommand implements EditCommand { final String firstNodeId; final String secondNodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { editorDocLog.info('Executing CombineParagraphsCommand'); @@ -446,6 +460,9 @@ class SplitParagraphCommand implements EditCommand { // TODO: remove the attribution filter and move the decision to an EditReaction in #1296 final AttributionFilter attributionsToExtendToNewParagraph; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { editorDocLog.info('Executing SplitParagraphCommand'); @@ -563,6 +580,9 @@ class DeleteUpstreamAtBeginningOfParagraphCommand implements EditCommand { final DocumentNode node; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { if (node is! ParagraphNode) { @@ -771,6 +791,9 @@ class DeleteParagraphCommand implements EditCommand { final String nodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { editorDocLog.info('Executing DeleteParagraphCommand'); diff --git a/super_editor/lib/src/default_editor/super_editor.dart b/super_editor/lib/src/default_editor/super_editor.dart index 42d424f3a..8cadb8a63 100644 --- a/super_editor/lib/src/default_editor/super_editor.dart +++ b/super_editor/lib/src/default_editor/super_editor.dart @@ -32,6 +32,7 @@ import 'package:super_editor/src/infrastructure/platforms/mac/mac_ime.dart'; import 'package:super_editor/src/infrastructure/platforms/platform.dart'; import 'package:super_editor/src/infrastructure/signal_notifier.dart'; import 'package:super_editor/src/infrastructure/text_input.dart'; +import 'package:super_editor/src/undo_redo.dart'; import 'package:super_text_layout/super_text_layout.dart'; import '../infrastructure/document_gestures_interaction_overrides.dart'; @@ -1177,6 +1178,8 @@ final defaultKeyboardActions = [ pasteWhenCmdVIsPressed, copyWhenCmdCIsPressed, cutWhenCmdXIsPressed, + undoWhenCmdZOrCtrlZIsPressed, + redoWhenCmdShiftZOrCtrlShiftZIsPressed, collapseSelectionWhenEscIsPressed, selectAllWhenCmdAIsPressed, moveLeftAndRightWithArrowKeys, @@ -1218,6 +1221,8 @@ final defaultImeKeyboardActions = [ pasteWhenCmdVIsPressed, copyWhenCmdCIsPressed, cutWhenCmdXIsPressed, + undoWhenCmdZOrCtrlZIsPressed, + redoWhenCmdShiftZOrCtrlShiftZIsPressed, selectAllWhenCmdAIsPressed, cmdBToToggleBold, cmdIToToggleItalics, diff --git a/super_editor/lib/src/default_editor/tasks.dart b/super_editor/lib/src/default_editor/tasks.dart index 44d17af5c..f121792ee 100644 --- a/super_editor/lib/src/default_editor/tasks.dart +++ b/super_editor/lib/src/default_editor/tasks.dart @@ -46,6 +46,16 @@ class TaskNode extends TextNode { return other is TaskNode && isComplete == other.isComplete && text == other.text; } + @override + TaskNode copy() { + return TaskNode( + id: id, + text: text.copyText(0), + metadata: Map.from(metadata), + isComplete: isComplete, + ); + } + @override bool operator ==(Object other) => identical(this, other) || @@ -382,6 +392,9 @@ class ChangeTaskCompletionCommand implements EditCommand { final String nodeId; final bool isComplete; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final taskNode = context.find(Editor.documentKey).getNodeById(nodeId); @@ -429,6 +442,9 @@ class ConvertParagraphToTaskCommand implements EditCommand { final String nodeId; final bool isComplete; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -460,6 +476,9 @@ class ConvertTaskToParagraphCommand implements EditCommand { final String nodeId; final Map? paragraphMetadata; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -506,6 +525,9 @@ class SplitExistingTaskCommand implements EditCommand { final int splitOffset; final String? newNodeId; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext editContext, CommandExecutor executor) { final document = editContext.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 0d0f5bbb3..bd187b805 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -164,6 +164,11 @@ class TextNode extends DocumentNode with ChangeNotifier { return other is TextNode && text == other.text && super.hasEquivalentContent(other); } + @override + TextNode copy() { + return TextNode(id: id, text: text.copyText(0), metadata: Map.from(metadata)); + } + @override String toString() => '[TextNode] - text: $text, metadata: ${copyMetadata()}'; @@ -1197,6 +1202,9 @@ class AddTextAttributionsCommand implements EditCommand { final Set attributions; final bool autoMerge; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { editorDocLog.info('Executing AddTextAttributionsCommand'); @@ -1308,6 +1316,9 @@ class RemoveTextAttributionsCommand implements EditCommand { final DocumentRange documentRange; final Set attributions; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { editorDocLog.info('Executing RemoveTextAttributionsCommand'); @@ -1422,6 +1433,9 @@ class ToggleTextAttributionsCommand implements EditCommand { final DocumentRange documentRange; final Set attributions; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + // TODO: The structure of this command looks nearly identical to the two other attribution // commands above. We collect nodes and then we loop through them to apply an operation. // Try to de-dup this code. Maybe use a private base class called ChangeTextAttributionsCommand @@ -1544,6 +1558,9 @@ class ChangeSingleColumnLayoutComponentStylesCommand implements EditCommand { final String nodeId; final SingleColumnLayoutComponentStyles styles; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -1582,6 +1599,9 @@ class InsertTextCommand implements EditCommand { final String textToInsert; final Set attributions; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -1592,6 +1612,8 @@ class InsertTextCommand implements EditCommand { return; } + print("Inserting text: '$textToInsert'"); + final textPosition = documentPosition.nodePosition as TextPosition; final textOffset = textPosition.offset; textNode.text = textNode.text.insertString( @@ -1599,6 +1621,7 @@ class InsertTextCommand implements EditCommand { startOffset: textOffset, applyAttributions: attributions, ); + print("Updated node text: '${textNode.text}'"); executor.logChanges([ DocumentEdit( @@ -1734,6 +1757,9 @@ class InsertAttributedTextCommand implements EditCommand { final DocumentPosition documentPosition; final AttributedText textToInsert; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart index 9fd89db91..ba7e75645 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart @@ -113,6 +113,9 @@ class SubmitComposingActionTagRequest implements EditRequest { } class SubmitComposingActionTagCommand implements EditCommand { + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -188,6 +191,9 @@ class CancelComposingActionTagCommand implements EditCommand { final TagRule _tagRule; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart index e5bea1a5d..a79b8ba11 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart @@ -173,6 +173,11 @@ class PatternTagIndex with ChangeNotifier implements Editable { notifyListeners(); } } + + @override + void reset() { + _tags.clear(); + } } /// An [EditReaction] that creates, updates, and removes pattern tags. 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..eced33d20 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 @@ -171,6 +171,9 @@ class FillInComposingUserTagCommand implements EditCommand { final String _tag; final TagRule _tagRule; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -283,6 +286,9 @@ class CancelComposingStableTagCommand implements EditCommand { final TagRule _tagRule; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -1076,6 +1082,13 @@ class StableTagIndex with ChangeNotifier implements Editable { notifyListeners(); } } + + @override + void reset() { + _composingStableTag.value = null; + _committedTags.clear(); + _cancelledTags.clear(); + } } class ComposingStableTag { diff --git a/super_editor/lib/src/undo_redo.dart b/super_editor/lib/src/undo_redo.dart new file mode 100644 index 000000000..de6002c52 --- /dev/null +++ b/super_editor/lib/src/undo_redo.dart @@ -0,0 +1,43 @@ +import 'package:flutter/services.dart'; +import 'package:super_editor/src/core/edit_context.dart'; +import 'package:super_editor/src/infrastructure/keyboard.dart'; + +/// Undoes the most recent change within the [Editor]. +ExecutionInstruction undoWhenCmdZOrCtrlZIsPressed({ + required SuperEditorContext editContext, + required KeyEvent keyEvent, +}) { + if (keyEvent is! KeyDownEvent && keyEvent is! KeyRepeatEvent) { + return ExecutionInstruction.continueExecution; + } + + if (keyEvent.logicalKey != LogicalKeyboardKey.keyZ || + !keyEvent.isPrimaryShortcutKeyPressed || + HardwareKeyboard.instance.isShiftPressed) { + return ExecutionInstruction.continueExecution; + } + + editContext.editor.undo(); + + return ExecutionInstruction.haltExecution; +} + +/// Re-runs the most recently undone change within the [Editor]. +ExecutionInstruction redoWhenCmdShiftZOrCtrlShiftZIsPressed({ + required SuperEditorContext editContext, + required KeyEvent keyEvent, +}) { + if (keyEvent is! KeyDownEvent && keyEvent is! KeyRepeatEvent) { + return ExecutionInstruction.continueExecution; + } + + if (keyEvent.logicalKey != LogicalKeyboardKey.keyZ || + !keyEvent.isPrimaryShortcutKeyPressed || + !HardwareKeyboard.instance.isShiftPressed) { + return ExecutionInstruction.continueExecution; + } + + editContext.editor.redo(); + + return ExecutionInstruction.haltExecution; +} diff --git a/super_editor/test/super_editor/infrastructure/editor_test.dart b/super_editor/test/super_editor/infrastructure/editor_test.dart index 189b743b1..4912550ed 100644 --- a/super_editor/test/super_editor/infrastructure/editor_test.dart +++ b/super_editor/test/super_editor/infrastructure/editor_test.dart @@ -695,6 +695,9 @@ class _ExpandingCommand implements EditCommand { final _ExpandingCommandRequest request; + @override + HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart new file mode 100644 index 000000000..06f97b6ae --- /dev/null +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -0,0 +1,318 @@ +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:super_editor/super_editor.dart'; +import 'package:super_editor/super_editor_test.dart'; +import 'package:super_editor_markdown/super_editor_markdown.dart'; + +import 'supereditor_test_tools.dart'; + +void main() { + group("Super Editor > undo redo >", () { + testWidgets("insert a word", (widgetTester) async { + final document = deserializeMarkdownToDocument("Hello world"); + final composer = MutableDocumentComposer(); + final editor = createDefaultDocumentEditor(document: document, composer: composer); + final paragraphId = document.nodes.first.id; + + editor.execute([ + ChangeSelectionRequest( + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 6), + ), + ), + SelectionChangeType.placeCaret, + SelectionReason.userInteraction, + ) + ]); + + editor.execute([ + InsertTextRequest( + documentPosition: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 6), + ), + textToInsert: "another", + attributions: {}, + ), + ]); + + expect(serializeDocumentToMarkdown(document), "Hello another world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 13), + ), + ), + ); + + // Undo the event. + editor.undo(); + + expect(serializeDocumentToMarkdown(document), "Hello world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 6), + ), + ), + ); + + // Redo the event. + editor.redo(); + + expect(serializeDocumentToMarkdown(document), "Hello another world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 13), + ), + ), + ); + }); + + testWidgetsOnMac("type by character", (widgetTester) async { + final editContext = await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + final editor = editContext.editor; + + await widgetTester.placeCaretInParagraph("1", 0); + + // Type characters. + await widgetTester.typeImeText("Hello"); + // editor.execute([ + // InsertTextRequest( + // documentPosition: const DocumentPosition( + // nodeId: "1", + // nodePosition: TextNodePosition(offset: 0), + // ), + // textToInsert: "H", + // attributions: {}, + // ), + // ]); + // await widgetTester.pump(); + + // editor.execute([ + // InsertTextRequest( + // documentPosition: const DocumentPosition( + // nodeId: "1", + // nodePosition: TextNodePosition(offset: 1), + // ), + // textToInsert: "e", + // attributions: {}, + // ), + // ]); + // await widgetTester.pump(); + // + // editor.execute([ + // InsertTextRequest( + // documentPosition: const DocumentPosition( + // nodeId: "1", + // nodePosition: TextNodePosition(offset: 2), + // ), + // textToInsert: "l", + // attributions: {}, + // ), + // ]); + // await widgetTester.pump(); + // + // editor.execute([ + // InsertTextRequest( + // documentPosition: const DocumentPosition( + // nodeId: "1", + // nodePosition: TextNodePosition(offset: 3), + // ), + // textToInsert: "l", + // attributions: {}, + // ), + // ]); + // await widgetTester.pump(); + // + // editor.execute([ + // InsertTextRequest( + // documentPosition: const DocumentPosition( + // nodeId: "1", + // nodePosition: TextNodePosition(offset: 4), + // ), + // textToInsert: "o", + // attributions: {}, + // ), + // ]); + // await widgetTester.pump(); + + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 5), + ), + ), + ); + + print("------ STARTING UNDOS --------"); + + // Undo the event. + await widgetTester.pressCmdZ(); // Undo composing region. + await widgetTester.pressCmdZ(); // Undo text. + + expect(SuperEditorInspector.findTextInComponent("1").text, "Hell"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 4), + ), + ), + ); + + await widgetTester.pressCmdZ(); // Undo composing region. + await widgetTester.pressCmdZ(); // Undo text. + + expect(SuperEditorInspector.findTextInComponent("1").text, "Hel"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 3), + ), + ), + ); + + await widgetTester.pressCmdZ(); // Undo composing region. + await widgetTester.pressCmdZ(); // Undo text. + + expect(SuperEditorInspector.findTextInComponent("1").text, "He"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 2), + ), + ), + ); + + await widgetTester.pressCmdZ(); // Undo composing region. + await widgetTester.pressCmdZ(); // Undo text. + + expect(SuperEditorInspector.findTextInComponent("1").text, "H"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 1), + ), + ), + ); + + await widgetTester.pressCmdZ(); // Undo composing region. + await widgetTester.pressCmdZ(); // Undo text. + + expect(SuperEditorInspector.findTextInComponent("1").text, ""); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 0), + ), + ), + ); + + print("---------- STARTING REDOS -----------"); + + await widgetTester.pressCmdShiftZ(); // Insert text + await widgetTester.pressCmdShiftZ(); // Change composing region + _expectDocumentWithCaret("H", "1", 1); + print("--------"); + + await widgetTester.pressCmdShiftZ(); // Insert text + await widgetTester.pressCmdShiftZ(); // Change composing region + _expectDocumentWithCaret("He", "1", 2); + print("--------"); + + await widgetTester.pressCmdShiftZ(); // Insert text + await widgetTester.pressCmdShiftZ(); // Change composing region + _expectDocumentWithCaret("Hel", "1", 3); + print("--------"); + + await widgetTester.pressCmdShiftZ(); // Insert text + await widgetTester.pressCmdShiftZ(); // Change composing region + _expectDocumentWithCaret("Hell", "1", 4); + + await widgetTester.pressCmdShiftZ(); // Insert text + await widgetTester.pressCmdShiftZ(); // Change composing region + _expectDocumentWithCaret("Hello", "1", 5); + }); + }); +} + +void _expectDocumentWithCaret(String documentContent, String caretNodeId, int caretOffset) { + expect(serializeDocumentToMarkdown(SuperEditorInspector.findDocument()!), documentContent); + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: caretNodeId, + nodePosition: TextNodePosition(offset: caretOffset), + ), + ), + ); +} + +extension on WidgetTester { + Future pressCmdZ() async { + await sendKeyDownEvent(LogicalKeyboardKey.metaLeft); + await sendKeyEvent(LogicalKeyboardKey.keyZ); + await sendKeyUpEvent(LogicalKeyboardKey.metaLeft); + + await pump(); + } + + Future pressCmdShiftZ() async { + await sendKeyDownEvent(LogicalKeyboardKey.metaLeft); + await sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + + await sendKeyEvent(LogicalKeyboardKey.keyZ); + + await sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + await sendKeyUpEvent(LogicalKeyboardKey.metaLeft); + + await pump(); + } + + Future pressCtrlZ() async { + await sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await sendKeyEvent(LogicalKeyboardKey.keyZ); + await sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + + await pump(); + } + + Future pressCtrlShiftZ() async { + await sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + + await sendKeyEvent(LogicalKeyboardKey.keyZ); + + await sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + await sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + + await pump(); + } +} From d5f7dda7957eb3dba58f0e962506c3a463357aba Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Tue, 19 Mar 2024 19:55:41 -0700 Subject: [PATCH 02/19] Added ability to start/end Editor transactions across multiple calls to execute() so that delta text insertions can execute the delta and then later adjust the composing region and have the whole thing considered to be a single transaction. --- super_editor/lib/src/core/editor.dart | 57 +++++++++++- .../document_ime/document_delta_editing.dart | 7 ++ .../super_editor_undo_redo_test.dart | 92 +++---------------- 3 files changed, 72 insertions(+), 84 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index df67a886f..e71cd34c3 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -123,6 +123,41 @@ class Editor implements RequestDispatcher { /// Tracks the number of request executions that are in the process of running. int _activeCommandCount = 0; + bool _isInTransaction = false; + CommandTransaction? _openTransaction; + + /// Starts a transaction that runs across multiple calls to [execute], until [endTransaction] + /// is called. + /// + /// Typically, a transaction only includes the [EditRequest]s that are passed to a single + /// call to [execute]. That's useful in the nominal case where editing code knows everything + /// that needs to execute at one time. However, sometimes the later [EditRequest] within a + /// single transaction can't be configured until the editing code inspects the [Document] + /// after some earlier [EditRequest]. In this situation, the editing code needs to be able + /// to run [execute] multiple times while having all [EditRequest]s still considered to be + /// part of the same transaction. + /// + /// Does nothing if a transaction is already in-progress. + void startTransaction() { + if (_isInTransaction) { + return; + } + + _isInTransaction = true; + } + + /// Ends a transaction that was started with a call to [startTransaction]. + /// + /// Does nothing if a transaction is not in-progress. + void endTransaction() { + if (!_isInTransaction) { + return; + } + + _isInTransaction = false; + _openTransaction = null; + } + /// Executes the given [requests]. /// /// Any changes that result from the given [requests] are reported to listeners as a series @@ -158,9 +193,25 @@ class Editor implements RequestDispatcher { } if (undoableCommands.isNotEmpty) { - _commandHistory.add( - CommandTransaction(undoableCommands), - ); + if (_isInTransaction) { + if (_openTransaction == null) { + // We're in a multi-execution transaction, but this is the first list of undoable + // commands that have been run for this transaction. Create the transaction object + // and remember it for future calls to `execute()`. + _openTransaction = CommandTransaction(undoableCommands); + _commandHistory.add(_openTransaction!); + } else { + // We're in a multi-execution transaction, and that transaction already contains + // some number of commands. Add these commands to it. + _openTransaction!.commands.addAll(undoableCommands); + } + } else { + // We're not running a multi-execution transaction. Therefore, the list of commands + // run within this call constitutes a full transaction. Add it to the history. + _commandHistory.add( + CommandTransaction(undoableCommands), + ); + } } // Run reactions and notify listeners, but only do it once per batch of executions. diff --git a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart index 13fd9f43c..0655bc6b6 100644 --- a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart +++ b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart @@ -70,6 +70,10 @@ class TextDeltasDocumentEditor { composing: _serializedDoc.documentToImeRange(_serializedDoc.composingRegion), ); + // Start an editor transaction so that all changes made during this delta + // application is considered a single undo-able change. + editor.startTransaction(); + for (final delta in textEditingDeltas) { editorImeLog.info("---------------------------------------------------"); @@ -107,6 +111,9 @@ class TextDeltasDocumentEditor { } editorImeLog.fine("Document composing region: ${composingRegion.value}"); + // End the editor transaction for all deltas in this call. + editor.endTransaction(); + _nextImeValue = null; } diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index 06f97b6ae..c93501d3b 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -80,75 +80,15 @@ void main() { }); testWidgetsOnMac("type by character", (widgetTester) async { - final editContext = await widgetTester // + await widgetTester // .createDocument() .withSingleEmptyParagraph() .pump(); - final editor = editContext.editor; await widgetTester.placeCaretInParagraph("1", 0); // Type characters. await widgetTester.typeImeText("Hello"); - // editor.execute([ - // InsertTextRequest( - // documentPosition: const DocumentPosition( - // nodeId: "1", - // nodePosition: TextNodePosition(offset: 0), - // ), - // textToInsert: "H", - // attributions: {}, - // ), - // ]); - // await widgetTester.pump(); - - // editor.execute([ - // InsertTextRequest( - // documentPosition: const DocumentPosition( - // nodeId: "1", - // nodePosition: TextNodePosition(offset: 1), - // ), - // textToInsert: "e", - // attributions: {}, - // ), - // ]); - // await widgetTester.pump(); - // - // editor.execute([ - // InsertTextRequest( - // documentPosition: const DocumentPosition( - // nodeId: "1", - // nodePosition: TextNodePosition(offset: 2), - // ), - // textToInsert: "l", - // attributions: {}, - // ), - // ]); - // await widgetTester.pump(); - // - // editor.execute([ - // InsertTextRequest( - // documentPosition: const DocumentPosition( - // nodeId: "1", - // nodePosition: TextNodePosition(offset: 3), - // ), - // textToInsert: "l", - // attributions: {}, - // ), - // ]); - // await widgetTester.pump(); - // - // editor.execute([ - // InsertTextRequest( - // documentPosition: const DocumentPosition( - // nodeId: "1", - // nodePosition: TextNodePosition(offset: 4), - // ), - // textToInsert: "o", - // attributions: {}, - // ), - // ]); - // await widgetTester.pump(); expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); expect( @@ -164,8 +104,7 @@ void main() { print("------ STARTING UNDOS --------"); // Undo the event. - await widgetTester.pressCmdZ(); // Undo composing region. - await widgetTester.pressCmdZ(); // Undo text. + await widgetTester.pressCmdZ(); expect(SuperEditorInspector.findTextInComponent("1").text, "Hell"); expect( @@ -178,8 +117,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); // Undo composing region. - await widgetTester.pressCmdZ(); // Undo text. + await widgetTester.pressCmdZ(); expect(SuperEditorInspector.findTextInComponent("1").text, "Hel"); expect( @@ -192,8 +130,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); // Undo composing region. - await widgetTester.pressCmdZ(); // Undo text. + await widgetTester.pressCmdZ(); expect(SuperEditorInspector.findTextInComponent("1").text, "He"); expect( @@ -206,8 +143,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); // Undo composing region. - await widgetTester.pressCmdZ(); // Undo text. + await widgetTester.pressCmdZ(); expect(SuperEditorInspector.findTextInComponent("1").text, "H"); expect( @@ -220,8 +156,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); // Undo composing region. - await widgetTester.pressCmdZ(); // Undo text. + await widgetTester.pressCmdZ(); expect(SuperEditorInspector.findTextInComponent("1").text, ""); expect( @@ -236,27 +171,22 @@ void main() { print("---------- STARTING REDOS -----------"); - await widgetTester.pressCmdShiftZ(); // Insert text - await widgetTester.pressCmdShiftZ(); // Change composing region + await widgetTester.pressCmdShiftZ(); _expectDocumentWithCaret("H", "1", 1); print("--------"); - await widgetTester.pressCmdShiftZ(); // Insert text - await widgetTester.pressCmdShiftZ(); // Change composing region + await widgetTester.pressCmdShiftZ(); _expectDocumentWithCaret("He", "1", 2); print("--------"); - await widgetTester.pressCmdShiftZ(); // Insert text - await widgetTester.pressCmdShiftZ(); // Change composing region + await widgetTester.pressCmdShiftZ(); _expectDocumentWithCaret("Hel", "1", 3); print("--------"); - await widgetTester.pressCmdShiftZ(); // Insert text - await widgetTester.pressCmdShiftZ(); // Change composing region + await widgetTester.pressCmdShiftZ(); _expectDocumentWithCaret("Hell", "1", 4); - await widgetTester.pressCmdShiftZ(); // Insert text - await widgetTester.pressCmdShiftZ(); // Change composing region + await widgetTester.pressCmdShiftZ(); _expectDocumentWithCaret("Hello", "1", 5); }); }); From eba2f323f54cb716675bb7f90362354c47634642 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Fri, 22 Mar 2024 12:08:03 -0700 Subject: [PATCH 03/19] WIP: Editor transactions --- super_editor/lib/src/core/editor.dart | 58 +++++++----- .../default_document_editor_reactions.dart | 42 ++++++--- .../super_editor_undo_redo_test.dart | 91 +++++++------------ 3 files changed, 97 insertions(+), 94 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index e71cd34c3..54b1d5bfd 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -139,23 +139,29 @@ class Editor implements RequestDispatcher { /// /// Does nothing if a transaction is already in-progress. void startTransaction() { + print("START TRANSACTION"); if (_isInTransaction) { return; } _isInTransaction = true; + + _onTransactionStart(); } /// Ends a transaction that was started with a call to [startTransaction]. /// /// Does nothing if a transaction is not in-progress. void endTransaction() { + print("END TRANSACTION"); if (!_isInTransaction) { return; } _isInTransaction = false; _openTransaction = null; + + _onTransactionEnd(); } /// Executes the given [requests]. @@ -170,11 +176,8 @@ class Editor implements RequestDispatcher { } // print(StackTrace.current); - if (_activeCommandCount == 0) { - // This is the start of a new transaction. - for (final editable in context._resources.values) { - editable.onTransactionStart(); - } + if (_activeCommandCount == 0 && !_isInTransaction) { + _onTransactionStart(); } _activeChangeList ??= []; @@ -219,24 +222,8 @@ class Editor implements RequestDispatcher { // would also run the reactions and notify listeners. At best this would result in // many superfluous calls, but in practice it would probably break lots of features // by notifying listeners too early, and running the same reactions over and over. - if (_activeCommandCount == 1) { - if (_activeChangeList!.isNotEmpty) { - // Run all reactions. These reactions will likely call `execute()` again, with - // their own requests, to make additional changes. - _reactToChanges(); - - // Notify all listeners that care about changes, but won't spawn additional requests. - _notifyListeners(); - - // This is the end of a transaction. - for (final editable in context._resources.values) { - editable.onTransactionEnd(_activeChangeList!); - } - } else { - editorOpsLog.warning("We have an empty change list after processing one or more requests: $requests"); - } - - _activeChangeList = null; + if (_activeCommandCount == 1 && !_isInTransaction) { + _onTransactionEnd(); } _activeCommandCount -= 1; @@ -276,6 +263,31 @@ class Editor implements RequestDispatcher { return changeList; } + void _onTransactionStart() { + for (final editable in context._resources.values) { + editable.onTransactionStart(); + } + } + + void _onTransactionEnd() { + if (_activeChangeList!.isNotEmpty) { + // TODO: We need to determine at this point whether to process reactios as a new + // transaction or as part of this existing transaction. + // Run all reactions. These reactions will likely call `execute()` again, with + // their own requests, to make additional changes. + _reactToChanges(); + + // Notify all listeners that care about changes, but won't spawn additional requests. + _notifyListeners(); + } + + for (final editable in context._resources.values) { + editable.onTransactionEnd(_activeChangeList!); + } + + _activeChangeList = null; + } + void _reactToChanges() { for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index a2a954f1a..145833e5c 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -260,10 +260,12 @@ class HorizontalRuleConversionReaction implements EditReaction { @override void react(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) { + print("HR Reaction"); if (changeList.length < 2) { // This reaction requires at least an insertion event and a selection change event. // There are less than two events in the the change list, therefore this reaction // shouldn't apply. Fizzle. + print("Less than 2 changes in change list - fizzling."); return; } @@ -271,10 +273,12 @@ class HorizontalRuleConversionReaction implements EditReaction { final didTypeSpace = EditInspector.didTypeSpace(document, changeList); if (!didTypeSpace) { + print("User didn't type a space - fizzling"); return; } - final edit = changeList[changeList.length - 2] as DocumentEdit; + // final edit = changeList[changeList.length - 2] as DocumentEdit; + final edit = changeList.reversed.firstWhere((edit) => edit is DocumentEdit) as DocumentEdit; if (edit.change is! TextInsertionEvent) { // This reaction requires that the two last events are an insertion event // followed by a selection change event. @@ -286,9 +290,11 @@ class HorizontalRuleConversionReaction implements EditReaction { final paragraph = document.getNodeById(textInsertionEvent.nodeId) as TextNode; final match = _hrPattern.firstMatch(paragraph.text.text)?.group(0); if (match == null) { + print("No HR pattern match. Fizzling."); return; } + print("User inserted HR pattern. Replacing text with HR."); // The user typed a horizontal rule pattern at the beginning of a paragraph. // - Remove the dashes and the space. // - Insert a horizontal rule before the paragraph. @@ -935,19 +941,33 @@ class EditInspector { return false; } - // If the user typed a space, then the last event should be a selection change. - final selectionEvent = edits[edits.length - 1]; - if (selectionEvent is! SelectionChangeEvent) { + // If the user typed a space, then the final document edit should be a text + // insertion event with a space " ". + DocumentEdit? lastDocumentEditEvent; + SelectionChangeEvent? lastSelectionChangeEvent; + print("Inspecting ${edits.length} edits"); + for (int i = edits.length - 1; i >= 0; i -= 1) { + print("Index: $i"); + if (edits[i] is DocumentEdit) { + lastDocumentEditEvent = edits[i] as DocumentEdit; + } else if (lastSelectionChangeEvent == null && edits[i] is SelectionChangeEvent) { + lastSelectionChangeEvent = edits[i] as SelectionChangeEvent; + } + + if (lastDocumentEditEvent != null) { + break; + } + } + if (lastDocumentEditEvent == null) { + print("There was no document edit. Fizzling."); return false; } - - // If the user typed a space, then the second to last event should be a text - // insertion event with a space " ". - final edit = edits[edits.length - 2]; - if (edit is! DocumentEdit) { + if (lastSelectionChangeEvent == null) { + print("There was no selection change after inserting space. Fizzling."); return false; } - final textInsertionEvent = edit.change; + + final textInsertionEvent = lastDocumentEditEvent.change; if (textInsertionEvent is! TextInsertionEvent) { return false; } @@ -955,7 +975,7 @@ class EditInspector { return false; } - if (selectionEvent.newSelection!.extent.nodeId != textInsertionEvent.nodeId) { + if (lastSelectionChangeEvent.newSelection!.extent.nodeId != textInsertionEvent.nodeId) { return false; } diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index c93501d3b..b6a0f0f0a 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -1,5 +1,5 @@ -import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_test_robots/flutter_test_robots.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; import 'package:super_editor/super_editor.dart'; import 'package:super_editor/super_editor_test.dart'; @@ -101,10 +101,8 @@ void main() { ), ); - print("------ STARTING UNDOS --------"); - // Undo the event. - await widgetTester.pressCmdZ(); + await widgetTester.pressCmdZ(widgetTester); expect(SuperEditorInspector.findTextInComponent("1").text, "Hell"); expect( @@ -117,7 +115,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); + await widgetTester.pressCmdZ(widgetTester); expect(SuperEditorInspector.findTextInComponent("1").text, "Hel"); expect( @@ -130,7 +128,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); + await widgetTester.pressCmdZ(widgetTester); expect(SuperEditorInspector.findTextInComponent("1").text, "He"); expect( @@ -143,7 +141,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); + await widgetTester.pressCmdZ(widgetTester); expect(SuperEditorInspector.findTextInComponent("1").text, "H"); expect( @@ -156,7 +154,7 @@ void main() { ), ); - await widgetTester.pressCmdZ(); + await widgetTester.pressCmdZ(widgetTester); expect(SuperEditorInspector.findTextInComponent("1").text, ""); expect( @@ -169,26 +167,41 @@ void main() { ), ); - print("---------- STARTING REDOS -----------"); - - await widgetTester.pressCmdShiftZ(); + await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("H", "1", 1); - print("--------"); - await widgetTester.pressCmdShiftZ(); + await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("He", "1", 2); - print("--------"); - await widgetTester.pressCmdShiftZ(); + await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("Hel", "1", 3); - print("--------"); - await widgetTester.pressCmdShiftZ(); + await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("Hell", "1", 4); - await widgetTester.pressCmdShiftZ(); + await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("Hello", "1", 5); }); + + testWidgetsOnMac("convert to horizontal rule", (widgetTester) async { + final editContext = await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + await widgetTester.typeImeText("--- "); + expect(editContext.document.nodes.first, isA()); + + print(""); + print("------- UNDO ------"); + print(""); + await widgetTester.pressCmdZ(widgetTester); + + expect(editContext.document.nodes.first, isA()); + expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "--- "); + }); }); } @@ -204,45 +217,3 @@ void _expectDocumentWithCaret(String documentContent, String caretNodeId, int ca ), ); } - -extension on WidgetTester { - Future pressCmdZ() async { - await sendKeyDownEvent(LogicalKeyboardKey.metaLeft); - await sendKeyEvent(LogicalKeyboardKey.keyZ); - await sendKeyUpEvent(LogicalKeyboardKey.metaLeft); - - await pump(); - } - - Future pressCmdShiftZ() async { - await sendKeyDownEvent(LogicalKeyboardKey.metaLeft); - await sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); - - await sendKeyEvent(LogicalKeyboardKey.keyZ); - - await sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); - await sendKeyUpEvent(LogicalKeyboardKey.metaLeft); - - await pump(); - } - - Future pressCtrlZ() async { - await sendKeyDownEvent(LogicalKeyboardKey.controlLeft); - await sendKeyEvent(LogicalKeyboardKey.keyZ); - await sendKeyUpEvent(LogicalKeyboardKey.controlLeft); - - await pump(); - } - - Future pressCtrlShiftZ() async { - await sendKeyDownEvent(LogicalKeyboardKey.controlLeft); - await sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); - - await sendKeyEvent(LogicalKeyboardKey.keyZ); - - await sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); - await sendKeyUpEvent(LogicalKeyboardKey.controlLeft); - - await pump(); - } -} From b690e568911ff78d8f465593ddad46d1f53dff33 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sun, 14 Apr 2024 21:55:49 -0700 Subject: [PATCH 04/19] WIP: Debugging reactions --- super_editor/example_docs/pubspec.lock | 66 ++++++++++++++++---------- super_editor/lib/src/core/editor.dart | 16 ++++++- 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/super_editor/example_docs/pubspec.lock b/super_editor/example_docs/pubspec.lock index 48318ee4f..83961bfa6 100644 --- a/super_editor/example_docs/pubspec.lock +++ b/super_editor/example_docs/pubspec.lock @@ -136,6 +136,14 @@ packages: url: "https://pub.dev" source: hosted version: "7.0.0" + fixnum: + dependency: transitive + description: + name: fixnum + sha256: "25517a4deb0c03aa0f32fd12db525856438902d9c16536311e76cdc57b31d7d1" + url: "https://pub.dev" + source: hosted + version: "1.1.0" flutter: dependency: "direct main" description: flutter @@ -243,26 +251,26 @@ packages: dependency: transitive description: name: leak_tracker - sha256: "7f0df31977cb2c0b88585095d168e689669a2cc9b97c309665e3386f3e9d341a" + sha256: "78eb209deea09858f5269f5a5b02be4049535f568c07b275096836f01ea323fa" url: "https://pub.dev" source: hosted - version: "10.0.4" + version: "10.0.0" leak_tracker_flutter_testing: dependency: transitive description: name: leak_tracker_flutter_testing - sha256: "06e98f569d004c1315b991ded39924b21af84cf14cc94791b8aea337d25b57f8" + sha256: b46c5e37c19120a8a01918cfaf293547f47269f7cb4b0058f21531c2465d6ef0 url: "https://pub.dev" source: hosted - version: "3.0.3" + version: "2.0.1" leak_tracker_testing: dependency: transitive description: name: leak_tracker_testing - sha256: "6ba465d5d76e67ddf503e1161d1f4a6bc42306f9d66ca1e8f079a47290fb06d3" + sha256: a597f72a664dbd293f3bfc51f9ba69816f84dcd403cdac7066cb3f6003f3ab47 url: "https://pub.dev" source: hosted - version: "3.0.1" + version: "2.0.1" linkify: dependency: transitive description: @@ -291,10 +299,10 @@ packages: dependency: transitive description: name: markdown - sha256: "01512006c8429f604eb10f9848717baeaedf99e991d14a50d540d9beff08e5c6" + sha256: "39caf989ccc72c63e87b961851a74257141938599ed2db45fbd9403fee0db423" url: "https://pub.dev" source: hosted - version: "4.0.1" + version: "5.0.0" matcher: dependency: transitive description: @@ -315,10 +323,10 @@ packages: dependency: transitive description: name: meta - sha256: "7687075e408b093f36e6bbf6c91878cc0d4cd10f409506f7bc996f68220b9136" + sha256: d584fa6707a52763a52446f02cc621b077888fb63b93bbcb1143a7be5a0c0c04 url: "https://pub.dev" source: hosted - version: "1.12.0" + version: "1.11.0" mime: dependency: transitive description: @@ -336,13 +344,13 @@ packages: source: hosted version: "2.0.2" overlord: - dependency: transitive + dependency: "direct main" description: name: overlord - sha256: "311b50446ec227beafc114968101ae623046cf27887f43c916fa7c5131a145b6" + sha256: "576256bc9ce3fb0ae3042bbb26eed67bdb26a5045dd7e3c851aae65b0bbab2f5" url: "https://pub.dev" source: hosted - version: "0.0.3+4" + version: "0.0.3+5" package_config: dependency: transitive description: @@ -500,6 +508,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.10.0" + sprintf: + dependency: transitive + description: + name: sprintf + sha256: "1fc9ffe69d4df602376b52949af107d8f5703b77cda567c4d7d86a0693120f23" + url: "https://pub.dev" + source: hosted + version: "7.0.0" stack_trace: dependency: transitive description: @@ -557,26 +573,26 @@ packages: dependency: transitive description: name: test - sha256: "7ee446762c2c50b3bd4ea96fe13ffac69919352bd3b4b17bac3f3465edc58073" + sha256: a1f7595805820fcc05e5c52e3a231aedd0b72972cb333e8c738a8b1239448b6f url: "https://pub.dev" source: hosted - version: "1.25.2" + version: "1.24.9" test_api: dependency: transitive description: name: test_api - sha256: "9955ae474176f7ac8ee4e989dadfb411a58c30415bcfb648fa04b2b8a03afa7f" + sha256: "5c2f730018264d276c20e4f1503fd1308dfbbae39ec8ee63c5236311ac06954b" url: "https://pub.dev" source: hosted - version: "0.7.0" + version: "0.6.1" test_core: dependency: transitive description: name: test_core - sha256: "2bc4b4ecddd75309300d8096f781c0e3280ca1ef85beda558d33fcbedc2eead4" + sha256: a757b14fc47507060a162cc2530d9a4a2f92f5100a952c7443b5cad5ef5b106a url: "https://pub.dev" source: hosted - version: "0.6.0" + version: "0.5.9" typed_data: dependency: transitive description: @@ -653,10 +669,10 @@ packages: dependency: transitive description: name: uuid - sha256: "648e103079f7c64a36dc7d39369cabb358d377078a051d6ae2ad3aa539519313" + sha256: "814e9e88f21a176ae1359149021870e87f7cddaf633ab678a5d2b0bff7fd1ba8" url: "https://pub.dev" source: hosted - version: "3.0.7" + version: "4.4.0" vector_math: dependency: transitive description: @@ -669,10 +685,10 @@ packages: dependency: transitive description: name: vm_service - sha256: e7d5ecd604e499358c5fe35ee828c0298a320d54455e791e9dcf73486bc8d9f0 + sha256: b3d56ff4341b8f182b96aceb2fa20e3dcb336b9f867bc0eafc0de10f1048e957 url: "https://pub.dev" source: hosted - version: "14.1.0" + version: "13.0.0" watcher: dependency: transitive description: @@ -730,5 +746,5 @@ packages: source: hosted version: "3.1.2" sdks: - dart: ">=3.3.0 <4.0.0" - flutter: ">=3.18.0-18.0.pre.54" + dart: ">=3.3.0-279.1.beta <4.0.0" + flutter: ">=3.16.0" diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 311de8422..d60cbff3a 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -264,27 +264,40 @@ class Editor implements RequestDispatcher { } void _onTransactionStart() { + print("_onTransactionStart()"); for (final editable in context._resources.values) { editable.onTransactionStart(); } } void _onTransactionEnd() { + print("_onTransactionEnd() - active change list: $_activeChangeList"); if (_activeChangeList!.isNotEmpty) { + print("_onTransactionEnd() - active change is non-empty - calling _reactToChanges()"); // TODO: We need to determine at this point whether to process reactios as a new // transaction or as part of this existing transaction. // Run all reactions. These reactions will likely call `execute()` again, with // their own requests, to make additional changes. - _reactToChanges(); + try { + _reactToChanges(); + } catch (exception, stacktrace) { + print("Error running _reactToChanges"); + print(exception); + print("$stacktrace"); + rethrow; + } // Notify all listeners that care about changes, but won't spawn additional requests. _notifyListeners(); } + print("_onTransactionEnd() - done reacting to changes"); for (final editable in context._resources.values) { + print("_onTransactionEnd() - ending transaction on editable: $editable"); editable.onTransactionEnd(_activeChangeList!); } + print("Null'ing out the active change list after ending the transaction"); _activeChangeList = null; } @@ -292,6 +305,7 @@ class Editor implements RequestDispatcher { for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. + print("_reactToChanges() - processing reaction. Active change list: $_activeChangeList"); reaction.react(context, this, _activeChangeList!); } } From 1fed142da0084addf5add60fdfb0b130bc4230ae Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sun, 21 Apr 2024 17:48:35 -0700 Subject: [PATCH 05/19] WIP: Trying to get HR conversion to work again after altering when transactions are applied. --- super_editor/lib/src/core/editor.dart | 113 +++++++++--------- .../common_editor_operations.dart | 3 +- .../text_tokenizing/pattern_tags.dart | 7 ++ .../super_editor_undo_redo_test.dart | 63 ++-------- .../text_entry/tagging/pattern_tags_test.dart | 5 +- 5 files changed, 78 insertions(+), 113 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index d60cbff3a..f170950a1 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -124,7 +124,11 @@ class Editor implements RequestDispatcher { int _activeCommandCount = 0; bool _isInTransaction = false; - CommandTransaction? _openTransaction; + bool _isImplicitTransaction = false; + CommandTransaction? _transaction; + + /// Whether the editor is currently running reactions for the current transaction. + bool _isReacting = false; /// Starts a transaction that runs across multiple calls to [execute], until [endTransaction] /// is called. @@ -139,29 +143,46 @@ class Editor implements RequestDispatcher { /// /// Does nothing if a transaction is already in-progress. void startTransaction() { - print("START TRANSACTION"); if (_isInTransaction) { return; } + print("STARTING TRANSACTION"); _isInTransaction = true; + _activeChangeList = []; + // ignore: prefer_const_constructors + _transaction = CommandTransaction([]); _onTransactionStart(); + print("TRANSACTION WAS STARTED"); } /// Ends a transaction that was started with a call to [startTransaction]. /// /// Does nothing if a transaction is not in-progress. void endTransaction() { - print("END TRANSACTION"); if (!_isInTransaction) { return; } + if (_transaction!.commands.isNotEmpty) { + _commandHistory.add(_transaction!); + } + + // Now that an atomic set of changes have completed, let the reactions followup + // with more changes, such as auto-correction, tagging, etc. + _reactToChanges(); + _isInTransaction = false; - _openTransaction = null; + _isImplicitTransaction = false; + _transaction = null; + // Note: The transaction isn't fully considered over until after the reactions run. + // This is because the reactions need access to the change list from the previous + // transaction. _onTransactionEnd(); + + print("TRANSACTION WAS ENDED"); } /// Executes the given [requests]. @@ -170,17 +191,25 @@ class Editor implements RequestDispatcher { /// of [EditEvent]s. @override void execute(List requests) { + if (requests.isEmpty) { + // No changes were requested. Don't waste time starting and ending transactions, etc. + print("No requests were given"); + print(StackTrace.current); + return; + } + print("Request execution:"); for (final request in requests) { print(" - ${request.runtimeType}"); } - // print(StackTrace.current); if (_activeCommandCount == 0 && !_isInTransaction) { - _onTransactionStart(); + // No transaction was explicitly requested, but all changes exist in a transaction. + // Automatically start one, and then end the transaction after the current changes. + _isImplicitTransaction = true; + startTransaction(); } - _activeChangeList ??= []; _activeCommandCount += 1; final undoableCommands = []; @@ -196,34 +225,11 @@ class Editor implements RequestDispatcher { } if (undoableCommands.isNotEmpty) { - if (_isInTransaction) { - if (_openTransaction == null) { - // We're in a multi-execution transaction, but this is the first list of undoable - // commands that have been run for this transaction. Create the transaction object - // and remember it for future calls to `execute()`. - _openTransaction = CommandTransaction(undoableCommands); - _commandHistory.add(_openTransaction!); - } else { - // We're in a multi-execution transaction, and that transaction already contains - // some number of commands. Add these commands to it. - _openTransaction!.commands.addAll(undoableCommands); - } - } else { - // We're not running a multi-execution transaction. Therefore, the list of commands - // run within this call constitutes a full transaction. Add it to the history. - _commandHistory.add( - CommandTransaction(undoableCommands), - ); - } + _transaction!.commands.addAll(undoableCommands); } - // Run reactions and notify listeners, but only do it once per batch of executions. - // If we reacted and notified listeners on every execution, then every sub-request - // would also run the reactions and notify listeners. At best this would result in - // many superfluous calls, but in practice it would probably break lots of features - // by notifying listeners too early, and running the same reactions over and over. - if (_activeCommandCount == 1 && !_isInTransaction) { - _onTransactionEnd(); + if (_activeCommandCount == 1 && _isImplicitTransaction && !_isReacting) { + endTransaction(); } _activeCommandCount -= 1; @@ -271,27 +277,6 @@ class Editor implements RequestDispatcher { } void _onTransactionEnd() { - print("_onTransactionEnd() - active change list: $_activeChangeList"); - if (_activeChangeList!.isNotEmpty) { - print("_onTransactionEnd() - active change is non-empty - calling _reactToChanges()"); - // TODO: We need to determine at this point whether to process reactios as a new - // transaction or as part of this existing transaction. - // Run all reactions. These reactions will likely call `execute()` again, with - // their own requests, to make additional changes. - try { - _reactToChanges(); - } catch (exception, stacktrace) { - print("Error running _reactToChanges"); - print(exception); - print("$stacktrace"); - rethrow; - } - - // Notify all listeners that care about changes, but won't spawn additional requests. - _notifyListeners(); - } - print("_onTransactionEnd() - done reacting to changes"); - for (final editable in context._resources.values) { print("_onTransactionEnd() - ending transaction on editable: $editable"); editable.onTransactionEnd(_activeChangeList!); @@ -302,12 +287,26 @@ class Editor implements RequestDispatcher { } void _reactToChanges() { + if (_activeChangeList!.isEmpty) { + return; + } + + print("${DateTime.now().microsecondsSinceEpoch} _reactToChanges()"); + _isReacting = true; for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. - print("_reactToChanges() - processing reaction. Active change list: $_activeChangeList"); + print( + "${DateTime.now().microsecondsSinceEpoch} Running reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); reaction.react(context, this, _activeChangeList!); } + print("${DateTime.now().microsecondsSinceEpoch} DONE _reactToChanges()"); + + // FIXME: try removing this notify listeners + // Notify all listeners that care about changes, but won't spawn additional requests. + _notifyListeners(); + + _isReacting = false; } void undo() { @@ -327,6 +326,10 @@ class Editor implements RequestDispatcher { // Move the latest command from the history to the future. _commandFuture.add(_commandHistory.removeLast()); + print("The commands being undone are:"); + for (final command in _commandHistory.last.commands) { + print(" - ${command.runtimeType}"); + } for (final editable in context._resources.values) { // Don't let editables notify listeners during undo. diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index 617e87b94..590c7b1be 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -1129,7 +1129,6 @@ class CommonEditorOperations { } else { editor.execute([const DeleteUpstreamCharacterRequest()]); return true; - // return _deleteUpstreamCharacter(); } } @@ -2448,6 +2447,8 @@ class DeleteUpstreamCharacterCommand implements EditCommand { final previousCharacterOffset = getCharacterStartBounds(textNode.text.text, currentTextOffset); // Delete the selected content. + print( + "Deleting character at $previousCharacterOffset. Moving selection to: ${textNode.selectionAt(previousCharacterOffset)}"); executor ..executeCommand( DeleteContentCommand( diff --git a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart index a79b8ba11..660964e01 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart @@ -360,6 +360,8 @@ class PatternTagReaction implements EditReaction { editorPatternTagsLog.fine( "Found a pattern tag around caret: '${tagAroundCaret.indexedTag.tag}' - surrounding it with an attribution: ${tagAroundCaret.indexedTag.startOffset} -> ${tagAroundCaret.indexedTag.endOffset}"); + print( + "FOUND A TAG: ${selectedNode.text.substring(tagAroundCaret.indexedTag.startOffset, tagAroundCaret.indexedTag.endOffset)}"); requestDispatcher.execute([ // Remove the old pattern tag attribution(s). RemoveTextAttributionsRequest( @@ -470,6 +472,11 @@ class PatternTagReaction implements EditReaction { } } + if (spanRemovals.isEmpty) { + // We didn't find any tags to break up. No need to submit change requests. + return; + } + // Execute the attribution removals and additions. requestDispatcher.execute([ // Remove the original multi-tag attribution spans. diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index b6a0f0f0a..e04a33cce 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -101,72 +101,23 @@ void main() { ), ); - // Undo the event. + // --- Undo character insertions --- await widgetTester.pressCmdZ(widgetTester); - - expect(SuperEditorInspector.findTextInComponent("1").text, "Hell"); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 4), - ), - ), - ); + _expectDocumentWithCaret("Hell", "1", 4); await widgetTester.pressCmdZ(widgetTester); - - expect(SuperEditorInspector.findTextInComponent("1").text, "Hel"); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 3), - ), - ), - ); + _expectDocumentWithCaret("Hel", "1", 3); await widgetTester.pressCmdZ(widgetTester); - - expect(SuperEditorInspector.findTextInComponent("1").text, "He"); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 2), - ), - ), - ); + _expectDocumentWithCaret("He", "1", 2); await widgetTester.pressCmdZ(widgetTester); - - expect(SuperEditorInspector.findTextInComponent("1").text, "H"); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 1), - ), - ), - ); + _expectDocumentWithCaret("H", "1", 1); await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("", "1", 0); - expect(SuperEditorInspector.findTextInComponent("1").text, ""); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 0), - ), - ), - ); - + //----- Redo Changes ---- await widgetTester.pressCmdShiftZ(widgetTester); _expectDocumentWithCaret("H", "1", 1); diff --git a/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart index c1071fa4c..ed9c7f0a6 100644 --- a/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart @@ -453,7 +453,7 @@ void main() { group("editing >", () { testWidgetsOnAllPlatforms("user can delete pieces of tags", (tester) async { - await _pumpTestEditor( + final context = await _pumpTestEditor( tester, singleParagraphEmptyDoc(), ); @@ -463,14 +463,17 @@ void main() { await tester.typeImeText("#abcdefghij "); // Delete part of the end. + print("-------- DELETING AT END ---------"); await tester.placeCaretInParagraph("1", 11); await tester.pressBackspace(); // Delete part of the middle. + print("-------- DELETING IN MIDDLE ---------"); await tester.placeCaretInParagraph("1", 6); await tester.pressBackspace(); // Delete part of the beginning. + print("-------- DELETING AT BEGINNING ---------"); await tester.placeCaretInParagraph("1", 2); await tester.pressBackspace(); From 858399d552b1dbd6210d98ddcee1fc3ecd47b0f0 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Tue, 14 May 2024 00:46:54 -0700 Subject: [PATCH 06/19] Fixed dash_conversion_test by loosening the guard conditions in the dash conversion reaction. --- super_editor/lib/src/core/editor.dart | 5 ++ .../default_document_editor_reactions.dart | 56 +++++++++---------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index f170950a1..cd06c80fd 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -292,6 +292,11 @@ class Editor implements RequestDispatcher { } print("${DateTime.now().microsecondsSinceEpoch} _reactToChanges()"); + for (final change in _activeChangeList!) { + print(" - ${change.runtimeType}"); + } + print(""); + _isReacting = true; for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index 145833e5c..dcadd9d97 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -857,40 +857,36 @@ class DashConversionReaction implements EditReaction { return; } - final selectionEvent = changeList.last; - if (selectionEvent is! SelectionChangeEvent) { - // This reaction requires that the two last events are an insertion event - // followed by a selection change event. - // The last event isn't a selection event, therefore this reaction - // shouldn't apply. Fizzle. - return; - } - - final documentEdit = changeList[changeList.length - 2]; - if (documentEdit is! DocumentEdit || documentEdit.change is! TextInsertionEvent) { - // This reaction requires that the two last events are an insertion event - // followed by a selection change event. - // The second to last event isn't a text insertion event, therefore this reaction - // shouldn't apply. Fizzle. - return; - } + TextInsertionEvent? dashInsertionEvent; + for (final event in changeList) { + if (event is! DocumentEdit) { + continue; + } - final insertionEvent = documentEdit.change as TextInsertionEvent; + final change = event.change; + if (change is! TextInsertionEvent) { + continue; + } + if (change.text.text != "-") { + continue; + } - if (insertionEvent.text.text != '-') { - // The text that was inserted wasn't a dash. The only character that triggers a - // conversion is a dash. Fizzle. + dashInsertionEvent = change; + break; + } + if (dashInsertionEvent == null) { + // The user didn't type a dash. return; } - if (insertionEvent.offset < 1) { - // The reaction needs at least two characters before the caret, but there's less than two. Fizzle. + if (dashInsertionEvent.offset == 0) { + // There's nothing upstream from this dash, therefore it can't + // be a 2nd dash. return; } - final insertionNode = document.getNodeById(insertionEvent.nodeId) as TextNode; - - final upstreamCharacter = insertionNode.text.text[insertionEvent.offset - 1]; + final insertionNode = document.getNodeById(dashInsertionEvent.nodeId) as TextNode; + final upstreamCharacter = insertionNode.text.text[dashInsertionEvent.offset - 1]; if (upstreamCharacter != '-') { return; } @@ -901,16 +897,16 @@ class DashConversionReaction implements EditReaction { DeleteContentRequest( documentRange: DocumentRange( start: DocumentPosition( - nodeId: insertionNode.id, nodePosition: TextNodePosition(offset: insertionEvent.offset - 1)), + nodeId: insertionNode.id, nodePosition: TextNodePosition(offset: dashInsertionEvent.offset - 1)), end: DocumentPosition( - nodeId: insertionNode.id, nodePosition: TextNodePosition(offset: insertionEvent.offset + 1)), + nodeId: insertionNode.id, nodePosition: TextNodePosition(offset: dashInsertionEvent.offset + 1)), ), ), InsertTextRequest( documentPosition: DocumentPosition( nodeId: insertionNode.id, nodePosition: TextNodePosition( - offset: insertionEvent.offset - 1, + offset: dashInsertionEvent.offset - 1, ), ), textToInsert: SpecialCharacters.emDash, @@ -920,7 +916,7 @@ class DashConversionReaction implements EditReaction { DocumentSelection.collapsed( position: DocumentPosition( nodeId: insertionNode.id, - nodePosition: TextNodePosition(offset: insertionEvent.offset), + nodePosition: TextNodePosition(offset: dashInsertionEvent.offset), ), ), SelectionChangeType.placeCaret, From 6217fe811e8494e09b9a13feee535e01427c3938 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 16 May 2024 11:30:36 -0700 Subject: [PATCH 07/19] WIP: Getting tests to pass again - mostly dealing with how adding composing region changes to the change list broke reactions --- .../default_document_editor_reactions.dart | 84 +++++++++++++++---- .../super_editor/text_entry/links_test.dart | 10 ++- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index dcadd9d97..035eddc24 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -535,6 +535,7 @@ class LinkifyReaction implements EditReaction { @override void react(EditContext editContext, RequestDispatcher requestDispatcher, List edits) { + print("(MAYBE) Running linkify reaction"); final document = editContext.find(Editor.documentKey); final composer = editContext.find(Editor.composerKey); bool didInsertSpace = false; @@ -609,7 +610,7 @@ class LinkifyReaction implements EditReaction { if (!didInsertSpace) { // We didn't linkify any text. Check if we need to update an URL. - _tryUpdateLinkAttribution(document, composer, edits); + _tryUpdateLinkAttribution(requestDispatcher, document, composer, edits); } } @@ -680,7 +681,9 @@ class LinkifyReaction implements EditReaction { } /// Update or remove the link attributions if edits happen at the middle of a link. - void _tryUpdateLinkAttribution(Document document, MutableDocumentComposer composer, List changeList) { + void _tryUpdateLinkAttribution(RequestDispatcher requestDispatcher, Document document, + MutableDocumentComposer composer, List changeList) { + print("_tryUpdateLinkAttribution"); if (!const [LinkUpdatePolicy.remove, LinkUpdatePolicy.update].contains(updatePolicy)) { // We are configured to NOT change the attributions. Fizzle. return; @@ -700,26 +703,28 @@ class LinkifyReaction implements EditReaction { // event. The only situation where a URL would change with a single // event is a deletion event. Therefore, we don't need to change a URL. // Fizzle. + print("There's only one change event and its not a deletion. Fizzling."); return; } insertionOrDeletionEvent = editEvent.change as NodeChangeEvent; } else { - final selectionEvent = changeList.last; - if (selectionEvent is! SelectionChangeEvent) { - // The last event isn't a selection event. We expect a URL change + final lastSelectionEventIndex = changeList.lastIndexWhere((change) => change is SelectionChangeEvent); + if (lastSelectionEventIndex < 1) { + // There's no selection change event. We expect a URL change // to consist of an insertion or a deletion followed by a selection // change. This event list doesn't fit the pattern. Fizzle. + print("There's no selection change event. Fizzling"); return; } - final edit = changeList[changeList.length - 2]; + final edit = changeList[lastSelectionEventIndex - 1]; if (edit is! DocumentEdit || // (edit.change is! TextInsertionEvent && edit.change is! TextDeletedEvent)) { - // The second to last event isn't an insertion or deletion. We - // expect a URL change to consist of an insertion or a deletion - // followed by a selection change. This event list doesn't fit - // the pattern. Fizzle. + // The event before the selection change isn't an insertion or deletion. We + // expect a URL change to consist of an insertion or a deletion followed by + // a selection change. This event list doesn't fit the pattern. Fizzle. + print("The selection change event isn't preceded by an insertion or deletion. Fizzling."); return; } @@ -791,10 +796,10 @@ class LinkifyReaction implements EditReaction { attributionFilter: (attr) => attr is LinkAttribution, range: rangeToUpdate, ); - for (final attributionSpan in attributionsToRemove) { - changedNodeText.removeAttribution(attributionSpan.attribution, attributionSpan.range); - composer.preferences.removeStyle(attributionSpan.attribution); - } + // for (final attributionSpan in attributionsToRemove) { + // changedNodeText.removeAttribution(attributionSpan.attribution, attributionSpan.range); + // composer.preferences.removeStyle(attributionSpan.attribution); + // } // A URL was changed and we have now removed the original link. Removing // the original link was a necessary step for both `LinkUpdatePolicy.remove` @@ -803,12 +808,55 @@ class LinkifyReaction implements EditReaction { // If the policy is `LinkUpdatePolicy.update` then we need to add a new // link attribution that reflects the edited URL text. We do that below. if (updatePolicy == LinkUpdatePolicy.update) { - changedNodeText.addAttribution( - LinkAttribution( - url: _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), + // TODO: don't change text directly!! + // changedNodeText.addAttribution( + // LinkAttribution( + // url: _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), + // ), + // rangeToUpdate, + // ); + + final linkRange = DocumentRange( + start: DocumentPosition( + nodeId: changedNodeId, + nodePosition: TextNodePosition(offset: rangeToUpdate.start), + ), + end: DocumentPosition( + nodeId: changedNodeId, + nodePosition: TextNodePosition(offset: rangeToUpdate.end + 1), ), - rangeToUpdate, ); + + print("Before executing requests..."); + print("Node spans:"); + print("${(document.getNodeById(changedNodeId) as TextNode).text.spans}"); + + print("Requesting removal of attribution:"); + print("${attributionsToRemove.first.attribution}"); + requestDispatcher.execute([ + // Switch out the old link attribution for the new one. + RemoveTextAttributionsRequest( + documentRange: linkRange, + attributions: {attributionsToRemove.first.attribution}, + ), + AddTextAttributionsRequest( + documentRange: linkRange, + attributions: { + LinkAttribution( + url: _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), + ) + }, + ), + // When the caret is in the middle of a link then the composer will automatically + // apply that style to the next character. Remove the current link style + // from the composer's preferences, so that as the user types, he doesn't + // immediately add the link attribution we just deleted. + RemoveComposerPreferenceStylesRequest(attributionsToRemove.first.attribution), + ]); + + print("After executing requests..."); + print("Node spans:"); + print("${(document.getNodeById(changedNodeId) as TextNode).text.spans}"); } } diff --git a/super_editor/test/super_editor/text_entry/links_test.dart b/super_editor/test/super_editor/text_entry/links_test.dart index f322faa82..620710610 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -1767,15 +1767,19 @@ void main() { await tester.doubleTapInParagraph(doc.nodes.first.id, 5); // Replace "google" with "duckduckgo". - await tester.typeImeText('duckduckgo'); + // await tester.typeImeText('duckduckgo'); + await tester.typeImeText('du'); // Ensure the text and the link were updated. final text = SuperEditorInspector.findTextInComponent(doc.nodes.first.id); - expect(text.text, "www.duckduckgo.com"); + // expect(text.text, "www.duckduckgo.com"); + expect(text.text, "www.du.com"); + print("Actual text spans: ${text.spans}"); expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.duckduckgo.com")), + LinkAttribution(url: Uri.parse("https://www.du.com")), + // LinkAttribution(url: Uri.parse("https://www.duckduckgo.com")), }, range: SpanRange(0, text.length - 1), ), From f4b22455e2512b25c22b9af4c2a598c9d343f9f3 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 25 May 2024 18:46:00 -0700 Subject: [PATCH 08/19] Got link reaction tests passing again --- .../lib/src/core/document_composer.dart | 21 +++++ .../lib/src/core/document_selection.dart | 16 ++++ .../default_document_editor.dart | 3 + .../default_document_editor_reactions.dart | 80 ++++++++----------- 4 files changed, 72 insertions(+), 48 deletions(-) diff --git a/super_editor/lib/src/core/document_composer.dart b/super_editor/lib/src/core/document_composer.dart index 0eb91a190..dd2c95244 100644 --- a/super_editor/lib/src/core/document_composer.dart +++ b/super_editor/lib/src/core/document_composer.dart @@ -563,3 +563,24 @@ class ChangeInteractionModeCommand implements EditCommand { context.find(Editor.composerKey).setIsInteractionMode(isInteractionModeDesired); } } + +class RemoveComposerPreferenceStylesRequest implements EditRequest { + const RemoveComposerPreferenceStylesRequest(this.stylesToRemove); + + final Set stylesToRemove; +} + +class RemoveComposerPreferenceStylesCommand implements EditCommand { + RemoveComposerPreferenceStylesCommand(this._stylesToRemove); + + final Set _stylesToRemove; + + @override + final historyBehavior = HistoryBehavior.undoable; + + @override + void execute(EditContext context, CommandExecutor executor) { + final composer = context.find(Editor.composerKey); + composer.preferences.removeStyles(_stylesToRemove); + } +} diff --git a/super_editor/lib/src/core/document_selection.dart b/super_editor/lib/src/core/document_selection.dart index ac957c836..82fa89022 100644 --- a/super_editor/lib/src/core/document_selection.dart +++ b/super_editor/lib/src/core/document_selection.dart @@ -1,5 +1,7 @@ import 'dart:ui'; +import 'package:super_editor/src/default_editor/text.dart'; + import 'document.dart'; /// A selection within a [Document]. @@ -86,6 +88,20 @@ class DocumentSelection extends DocumentRange { @override String toString() { + if (base.nodeId == extent.nodeId) { + final basePosition = base.nodePosition; + final extentPosition = extent.nodePosition; + if (basePosition is TextNodePosition && extentPosition is TextNodePosition) { + if (basePosition.offset == extentPosition.offset) { + return "[Selection] - ${base.nodeId}: ${extentPosition.offset}"; + } + + return "[Selection] - ${base.nodeId}: [${basePosition.offset}, ${extentPosition.offset}]"; + } + + return "[Selection] - ${base.nodeId}: [${base.nodePosition}, ${extent.nodePosition}]"; + } + return '[DocumentSelection] - \n base: ($base),\n extent: ($extent)'; } diff --git a/super_editor/lib/src/default_editor/default_document_editor.dart b/super_editor/lib/src/default_editor/default_document_editor.dart index 049f079ab..aa66907a1 100644 --- a/super_editor/lib/src/default_editor/default_document_editor.dart +++ b/super_editor/lib/src/default_editor/default_document_editor.dart @@ -52,6 +52,9 @@ final defaultRequestHandlers = List.unmodifiable([ (request) => request is ChangeInteractionModeRequest // ? ChangeInteractionModeCommand(isInteractionModeDesired: request.isInteractionModeDesired) : null, + (request) => request is RemoveComposerPreferenceStylesRequest // + ? RemoveComposerPreferenceStylesCommand(request.stylesToRemove) + : null, (request) => request is InsertTextRequest ? InsertTextCommand( documentPosition: request.documentPosition, diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index 035eddc24..45fddf2cb 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -683,7 +683,6 @@ class LinkifyReaction implements EditReaction { /// Update or remove the link attributions if edits happen at the middle of a link. void _tryUpdateLinkAttribution(RequestDispatcher requestDispatcher, Document document, MutableDocumentComposer composer, List changeList) { - print("_tryUpdateLinkAttribution"); if (!const [LinkUpdatePolicy.remove, LinkUpdatePolicy.update].contains(updatePolicy)) { // We are configured to NOT change the attributions. Fizzle. return; @@ -703,7 +702,6 @@ class LinkifyReaction implements EditReaction { // event. The only situation where a URL would change with a single // event is a deletion event. Therefore, we don't need to change a URL. // Fizzle. - print("There's only one change event and its not a deletion. Fizzling."); return; } @@ -714,7 +712,6 @@ class LinkifyReaction implements EditReaction { // There's no selection change event. We expect a URL change // to consist of an insertion or a deletion followed by a selection // change. This event list doesn't fit the pattern. Fizzle. - print("There's no selection change event. Fizzling"); return; } @@ -724,7 +721,6 @@ class LinkifyReaction implements EditReaction { // The event before the selection change isn't an insertion or deletion. We // expect a URL change to consist of an insertion or a deletion followed by // a selection change. This event list doesn't fit the pattern. Fizzle. - print("The selection change event isn't preceded by an insertion or deletion. Fizzling."); return; } @@ -796,10 +792,24 @@ class LinkifyReaction implements EditReaction { attributionFilter: (attr) => attr is LinkAttribution, range: rangeToUpdate, ); - // for (final attributionSpan in attributionsToRemove) { - // changedNodeText.removeAttribution(attributionSpan.attribution, attributionSpan.range); - // composer.preferences.removeStyle(attributionSpan.attribution); - // } + + final linkRange = DocumentRange( + start: DocumentPosition( + nodeId: changedNodeId, + nodePosition: TextNodePosition(offset: rangeToUpdate.start), + ), + end: DocumentPosition( + nodeId: changedNodeId, + nodePosition: TextNodePosition(offset: rangeToUpdate.end + 1), + ), + ); + + final linkChangeRequests = [ + RemoveTextAttributionsRequest( + documentRange: linkRange, + attributions: {attributionsToRemove.first.attribution}, + ), + ]; // A URL was changed and we have now removed the original link. Removing // the original link was a necessary step for both `LinkUpdatePolicy.remove` @@ -808,37 +818,8 @@ class LinkifyReaction implements EditReaction { // If the policy is `LinkUpdatePolicy.update` then we need to add a new // link attribution that reflects the edited URL text. We do that below. if (updatePolicy == LinkUpdatePolicy.update) { - // TODO: don't change text directly!! - // changedNodeText.addAttribution( - // LinkAttribution( - // url: _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), - // ), - // rangeToUpdate, - // ); - - final linkRange = DocumentRange( - start: DocumentPosition( - nodeId: changedNodeId, - nodePosition: TextNodePosition(offset: rangeToUpdate.start), - ), - end: DocumentPosition( - nodeId: changedNodeId, - nodePosition: TextNodePosition(offset: rangeToUpdate.end + 1), - ), - ); - - print("Before executing requests..."); - print("Node spans:"); - print("${(document.getNodeById(changedNodeId) as TextNode).text.spans}"); - - print("Requesting removal of attribution:"); - print("${attributionsToRemove.first.attribution}"); - requestDispatcher.execute([ + linkChangeRequests.add( // Switch out the old link attribution for the new one. - RemoveTextAttributionsRequest( - documentRange: linkRange, - attributions: {attributionsToRemove.first.attribution}, - ), AddTextAttributionsRequest( documentRange: linkRange, attributions: { @@ -847,17 +828,20 @@ class LinkifyReaction implements EditReaction { ) }, ), - // When the caret is in the middle of a link then the composer will automatically - // apply that style to the next character. Remove the current link style - // from the composer's preferences, so that as the user types, he doesn't - // immediately add the link attribution we just deleted. - RemoveComposerPreferenceStylesRequest(attributionsToRemove.first.attribution), - ]); - - print("After executing requests..."); - print("Node spans:"); - print("${(document.getNodeById(changedNodeId) as TextNode).text.spans}"); + ); } + + linkChangeRequests.add( + // When the caret is in the middle of a link then the composer will automatically + // apply that style to the next character. Remove the current link style + // from the composer's preferences, so that as the user types, he doesn't + // immediately add the link attribution we just deleted. + RemoveComposerPreferenceStylesRequest( + attributionsToRemove.map((span) => span.attribution).toSet(), + ), + ); + + requestDispatcher.execute(linkChangeRequests); } /// Parses the [text] as [Uri], prepending "https://" if it doesn't start From 87f8c1b93d24d17e1088c35862f133b8e9fcdf36 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 25 May 2024 19:48:39 -0700 Subject: [PATCH 09/19] Got paragraph_conversions_test.dart to pass again --- .../default_document_editor_reactions.dart | 103 ++++++++++-------- 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index 45fddf2cb..c879c3fa4 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:attributed_text/attributed_text.dart'; import 'package:characters/characters.dart'; import 'package:collection/collection.dart'; +import 'package:flutter/cupertino.dart'; import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; import 'package:linkify/linkify.dart'; @@ -70,6 +71,7 @@ class HeaderConversionReaction extends ParagraphPrefixConversionReaction { ParagraphNode paragraph, String match, ) { + print("FOUND PREFIX MATCH!"); final prefixLength = match.length - 1; // -1 for the space on the end late Attribution headerAttribution = _getHeaderAttributionForLevel(prefixLength); @@ -346,20 +348,27 @@ abstract class ParagraphPrefixConversionReaction implements EditReaction { @override void react(EditContext editContext, RequestDispatcher requestDispatcher, List changeList) { + print("Running paragraph prefix matcher for pattern: ${pattern.pattern}"); final document = editContext.find(Editor.documentKey); - final didTypeSpaceAtEnd = EditInspector.didTypeSpaceAtEndOfNode(document, changeList); - if (_requireSpaceInsertion && !didTypeSpaceAtEnd) { + final typedText = EditInspector.findLastTextUserTyped(document, changeList); + if (typedText == null) { + print("User didn't type any text. Fizzling."); + return; + } + if (_requireSpaceInsertion && !typedText.text.text.endsWith(" ")) { + print("User didn't end with a space. Fizzling."); return; } - final edit = changeList[changeList.length - 2] as DocumentEdit; - final textInsertionEvent = edit.change as TextInsertionEvent; - final paragraph = document.getNodeById(textInsertionEvent.nodeId); + final paragraph = document.getNodeById(typedText.nodeId); if (paragraph is! ParagraphNode) { + print("Not a ParagraphNode. Fizzling."); return; } + final match = pattern.firstMatch(paragraph.text.text)?.group(0); if (match == null) { + print("Didn't find a match for the pattern. Fizzling."); return; } @@ -1016,58 +1025,62 @@ class EditInspector { return true; } - /// Returns `true` if the given [edits] end with the user typing a space at the end of - /// a [TextNode], e.g., typing a " " at the end of a paragraph. - static bool didTypeSpaceAtEndOfNode(Document document, List edits) { - if (edits.length < 2) { - // This reaction requires at least an insertion event and a selection change event. - // There are less than two events in the the change list, therefore this reaction - // shouldn't apply. Fizzle. - return false; + /// Finds and returns the last text the user typed within the given [edit]s, or `null` if + /// no text was typed. + static UserTypedText? findLastTextUserTyped(Document document, List edits) { + final lastSpaceInsertion = edits.whereType().lastWhereOrNull( + (edit) => edit.change is TextInsertionEvent && (edit.change as TextInsertionEvent).text.text.endsWith(" ")); + if (lastSpaceInsertion == null) { + // The user didn't insert any text segment that ended with a space. + return null; } - // If the user typed a space, then the last event should be a selection change. - final selectionEvent = edits[edits.length - 1]; - if (selectionEvent is! SelectionChangeEvent) { - return false; + final spaceInsertionChangeIndex = edits.indexWhere((edit) => edit == lastSpaceInsertion); + final selectionAfterInsertionIndex = + edits.indexWhere((edit) => edit is SelectionChangeEvent, spaceInsertionChangeIndex); + if (selectionAfterInsertionIndex < 0) { + // The text insertion wasn't followed by a selection change. It's not clear what this + // means, but we can't say with confidence that the user typed the space. Perhaps the + // space was injected by some other means. + return null; } - // If the user typed a space, then the second to last event should be a text - // insertion event with a space " ". - final edit = edits[edits.length - 2]; - if (edit is! DocumentEdit) { - return false; + final newSelection = (edits[selectionAfterInsertionIndex] as SelectionChangeEvent).newSelection; + if (newSelection == null) { + // There's no selection, which indicates something other than the user typing. + return null; } - final textInsertionEvent = edit.change; - if (textInsertionEvent is! TextInsertionEvent) { - return false; - } - if (textInsertionEvent.text.text != " ") { - return false; + if (!newSelection.isCollapsed) { + // The selection is expanded, which indicates something other than the user typing. + return null; } - if (selectionEvent.oldSelection == null || selectionEvent.newSelection == null) { - return false; - } - if (selectionEvent.newSelection!.extent.nodeId != textInsertionEvent.nodeId) { - return false; + final textInsertionEvent = lastSpaceInsertion.change as TextInsertionEvent; + if (textInsertionEvent.nodeId != newSelection.extent.nodeId) { + // The selection is in a different node than where tex was inserted. This indicates + // something other than a user typing. + return null; } - final editedNode = document.getNodeById(textInsertionEvent.nodeId)!; - if (editedNode is! TextNode) { - return false; + final newCaretOffset = (newSelection.extent.nodePosition as TextNodePosition).offset; + if (textInsertionEvent.offset + textInsertionEvent.text.length != newCaretOffset) { + return null; } - final caretPosition = selectionEvent.newSelection!.extent.nodePosition as TextNodePosition; - final editedText = editedNode.text.text; - if (caretPosition.offset != editedText.length) { - return false; - } - - // The inserted text was a space, and the caret now sits at the end of - // the edited text. We assume this means that the user just typed a space. - return true; + return UserTypedText( + textInsertionEvent.nodeId, + textInsertionEvent.offset, + textInsertionEvent.text, + ); } EditInspector._(); } + +class UserTypedText { + const UserTypedText(this.nodeId, this.offset, this.text); + + final String nodeId; + final int offset; + final AttributedText text; +} From 9c14a8dcf6899458b2add04629e8c3ca5b21f37a Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 25 May 2024 20:54:49 -0700 Subject: [PATCH 10/19] All tests pass - next we need reactions to be able to put their changes in a new transaction, also need checkpoint serialization for editables to avoid full history playback --- .../lib/src/core/document_composer.dart | 11 +++++++---- .../lib/src/core/document_selection.dart | 16 +++++++++++++++- super_editor/lib/src/core/editor.dart | 18 ++++++++++++++---- .../lib/src/default_editor/blockquote.dart | 4 ++-- .../lib/src/default_editor/box_component.dart | 2 +- .../common_editor_operations.dart | 6 +++--- .../lib/src/default_editor/list_items.dart | 12 ++++++------ .../src/default_editor/multi_node_editing.dart | 14 ++++++++++---- .../lib/src/default_editor/paragraph.dart | 12 ++++++------ super_editor/lib/src/default_editor/tasks.dart | 8 ++++---- super_editor/lib/src/default_editor/text.dart | 16 ++++++++++------ .../text_tokenizing/action_tags.dart | 4 ++-- .../text_tokenizing/stable_tags.dart | 4 ++-- .../infrastructure/editor_test.dart | 2 +- .../super_editor_undo_redo_test.dart | 4 +++- 15 files changed, 86 insertions(+), 47 deletions(-) diff --git a/super_editor/lib/src/core/document_composer.dart b/super_editor/lib/src/core/document_composer.dart index dd2c95244..52344f86e 100644 --- a/super_editor/lib/src/core/document_composer.dart +++ b/super_editor/lib/src/core/document_composer.dart @@ -343,7 +343,7 @@ class ChangeSelectionRequest implements EditRequest { /// An [EditCommand] that changes the [DocumentSelection] in the [DocumentComposer] /// to the [newSelection]. -class ChangeSelectionCommand implements EditCommand { +class ChangeSelectionCommand extends EditCommand { const ChangeSelectionCommand( this.newSelection, this.changeType, @@ -364,6 +364,9 @@ class ChangeSelectionCommand implements EditCommand { @override HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override + String describe() => "Change selection ($changeType): $newSelection"; + @override void execute(EditContext context, CommandExecutor executor) { final composer = context.find(Editor.composerKey); @@ -502,7 +505,7 @@ class ChangeComposingRegionRequest implements EditRequest { int get hashCode => composingRegion.hashCode; } -class ChangeComposingRegionCommand implements EditCommand { +class ChangeComposingRegionCommand extends EditCommand { ChangeComposingRegionCommand(this.composingRegion); final DocumentRange? composingRegion; @@ -548,7 +551,7 @@ class ChangeInteractionModeRequest implements EditRequest { int get hashCode => isInteractionModeDesired.hashCode; } -class ChangeInteractionModeCommand implements EditCommand { +class ChangeInteractionModeCommand extends EditCommand { ChangeInteractionModeCommand({ required this.isInteractionModeDesired, }); @@ -570,7 +573,7 @@ class RemoveComposerPreferenceStylesRequest implements EditRequest { final Set stylesToRemove; } -class RemoveComposerPreferenceStylesCommand implements EditCommand { +class RemoveComposerPreferenceStylesCommand extends EditCommand { RemoveComposerPreferenceStylesCommand(this._stylesToRemove); final Set _stylesToRemove; diff --git a/super_editor/lib/src/core/document_selection.dart b/super_editor/lib/src/core/document_selection.dart index 82fa89022..a4435ac2c 100644 --- a/super_editor/lib/src/core/document_selection.dart +++ b/super_editor/lib/src/core/document_selection.dart @@ -299,7 +299,21 @@ class DocumentRange { @override String toString() { - return '[DocumentRange] - start: ($start), end: ($end)'; + if (start.nodeId == end.nodeId) { + final startPosition = start.nodePosition; + final endPosition = end.nodePosition; + if (startPosition is TextNodePosition && endPosition is TextNodePosition) { + if (startPosition.offset == endPosition.offset) { + return "[Range] - ${start.nodeId}: ${endPosition.offset}"; + } + + return "[Range] - ${start.nodeId}: [${startPosition.offset}, ${endPosition.offset}]"; + } + + return "[Range] - ${start.nodeId}: [${start.nodePosition}, ${end.nodePosition}]"; + } + + return '[Range] - \n start: ($start),\n end: ($end)'; } } diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index cd06c80fd..7f7bc9265 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -324,18 +324,20 @@ class Editor implements RequestDispatcher { for (final transaction in _commandHistory) { print(" - transaction"); for (final command in transaction.commands) { - print(" - ${command.runtimeType}"); + print(" - ${command.runtimeType}: ${command.describe()}"); } } print("---"); // Move the latest command from the history to the future. - _commandFuture.add(_commandHistory.removeLast()); + final transactionToUndo = _commandHistory.removeLast(); + _commandFuture.add(transactionToUndo); print("The commands being undone are:"); - for (final command in _commandHistory.last.commands) { - print(" - ${command.runtimeType}"); + for (final command in transactionToUndo.commands) { + print(" - ${command.runtimeType}: ${command.describe()}"); } + print("Resetting all editables to their last checkpoint..."); for (final editable in context._resources.values) { // Don't let editables notify listeners during undo. editable.onTransactionStart(); @@ -345,6 +347,7 @@ class Editor implements RequestDispatcher { } // Replay all history except for the most recent command transaction. + print("Replaying all command history except for the most recent transaction..."); final changeEvents = []; for (final commandTransaction in _commandHistory) { print("Starting a command transaction."); @@ -359,10 +362,13 @@ class Editor implements RequestDispatcher { print("Ending a command transaction."); } + print("Ending the transaction for all Editables..."); for (final editable in context._resources.values) { // Let editables start notifying listeners again. editable.onTransactionEnd(changeEvents); } + + print("DONE WITH UNDO"); } void redo() { @@ -479,11 +485,15 @@ abstract class RequestDispatcher { /// A command that alters something in a [Editor]. abstract class EditCommand { + const EditCommand(); + /// Executes this command and logs all changes with the [executor]. void execute(EditContext context, CommandExecutor executor); /// The desired "undo" behavior of this command. HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + + String describe() => toString(); } /// The way a command interacts with the history ledger, AKA "undo". diff --git a/super_editor/lib/src/default_editor/blockquote.dart b/super_editor/lib/src/default_editor/blockquote.dart index 73a717648..694758fcc 100644 --- a/super_editor/lib/src/default_editor/blockquote.dart +++ b/super_editor/lib/src/default_editor/blockquote.dart @@ -238,7 +238,7 @@ class BlockquoteComponent extends StatelessWidget { } } -class ConvertBlockquoteToParagraphCommand implements EditCommand { +class ConvertBlockquoteToParagraphCommand extends EditCommand { ConvertBlockquoteToParagraphCommand({ required this.nodeId, }); @@ -327,7 +327,7 @@ ExecutionInstruction splitBlockquoteWhenEnterPressed({ return didSplit ? ExecutionInstruction.haltExecution : ExecutionInstruction.continueExecution; } -class SplitBlockquoteCommand implements EditCommand { +class SplitBlockquoteCommand extends EditCommand { SplitBlockquoteCommand({ required this.nodeId, required this.splitPosition, diff --git a/super_editor/lib/src/default_editor/box_component.dart b/super_editor/lib/src/default_editor/box_component.dart index b98e35de2..4bfd4572f 100644 --- a/super_editor/lib/src/default_editor/box_component.dart +++ b/super_editor/lib/src/default_editor/box_component.dart @@ -310,7 +310,7 @@ class SelectableBox extends StatelessWidget { } } -class DeleteUpstreamAtBeginningOfBlockNodeCommand implements EditCommand { +class DeleteUpstreamAtBeginningOfBlockNodeCommand extends EditCommand { DeleteUpstreamAtBeginningOfBlockNodeCommand(this.node); final DocumentNode node; diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index 590c7b1be..acb7944f1 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -2243,7 +2243,7 @@ class PasteEditorRequest implements EditRequest { final DocumentComposer composer; } -class PasteEditorCommand implements EditCommand { +class PasteEditorCommand extends EditCommand { PasteEditorCommand({ required String content, required DocumentPosition pastePosition, @@ -2416,7 +2416,7 @@ class DeleteUpstreamCharacterRequest implements EditRequest { const DeleteUpstreamCharacterRequest(); } -class DeleteUpstreamCharacterCommand implements EditCommand { +class DeleteUpstreamCharacterCommand extends EditCommand { const DeleteUpstreamCharacterCommand(); @override @@ -2472,7 +2472,7 @@ class DeleteDownstreamCharacterRequest implements EditRequest { const DeleteDownstreamCharacterRequest(); } -class DeleteDownstreamCharacterCommand implements EditCommand { +class DeleteDownstreamCharacterCommand extends EditCommand { const DeleteDownstreamCharacterCommand(); @override diff --git a/super_editor/lib/src/default_editor/list_items.dart b/super_editor/lib/src/default_editor/list_items.dart index f573acece..a0ed83e97 100644 --- a/super_editor/lib/src/default_editor/list_items.dart +++ b/super_editor/lib/src/default_editor/list_items.dart @@ -525,7 +525,7 @@ class IndentListItemRequest implements EditRequest { final String nodeId; } -class IndentListItemCommand implements EditCommand { +class IndentListItemCommand extends EditCommand { IndentListItemCommand({ required this.nodeId, }); @@ -563,7 +563,7 @@ class UnIndentListItemRequest implements EditRequest { final String nodeId; } -class UnIndentListItemCommand implements EditCommand { +class UnIndentListItemCommand extends EditCommand { UnIndentListItemCommand({ required this.nodeId, }); @@ -608,7 +608,7 @@ class ConvertListItemToParagraphRequest implements EditRequest { final Map? paragraphMetadata; } -class ConvertListItemToParagraphCommand implements EditCommand { +class ConvertListItemToParagraphCommand extends EditCommand { ConvertListItemToParagraphCommand({ required this.nodeId, this.paragraphMetadata, @@ -655,7 +655,7 @@ class ConvertParagraphToListItemRequest implements EditRequest { final ListItemType type; } -class ConvertParagraphToListItemCommand implements EditCommand { +class ConvertParagraphToListItemCommand extends EditCommand { ConvertParagraphToListItemCommand({ required this.nodeId, required this.type, @@ -698,7 +698,7 @@ class ChangeListItemTypeRequest implements EditRequest { final ListItemType newType; } -class ChangeListItemTypeCommand implements EditCommand { +class ChangeListItemTypeCommand extends EditCommand { ChangeListItemTypeCommand({ required this.nodeId, required this.newType, @@ -742,7 +742,7 @@ class SplitListItemRequest implements EditRequest { final String newNodeId; } -class SplitListItemCommand implements EditCommand { +class SplitListItemCommand extends EditCommand { SplitListItemCommand({ required this.nodeId, required this.splitPosition, diff --git a/super_editor/lib/src/default_editor/multi_node_editing.dart b/super_editor/lib/src/default_editor/multi_node_editing.dart index 133895f95..bca467b0c 100644 --- a/super_editor/lib/src/default_editor/multi_node_editing.dart +++ b/super_editor/lib/src/default_editor/multi_node_editing.dart @@ -28,7 +28,7 @@ class PasteStructuredContentEditorRequest implements EditRequest { /// Inserts given structured content, in the form of a `List` of [DocumentNode]s at a /// given paste position within the document. -class PasteStructuredContentEditorCommand implements EditCommand { +class PasteStructuredContentEditorCommand extends EditCommand { PasteStructuredContentEditorCommand({ required List content, required DocumentPosition pastePosition, @@ -274,6 +274,9 @@ class InsertNodeAtIndexCommand extends EditCommand { final int nodeIndex; final DocumentNode newNode; + @override + String describe() => "Insert node at index $nodeIndex: $newNode"; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -603,7 +606,7 @@ class ReplaceNodeWithEmptyParagraphWithCaretRequest implements EditRequest { int get hashCode => nodeId.hashCode; } -class ReplaceNodeWithEmptyParagraphWithCaretCommand implements EditCommand { +class ReplaceNodeWithEmptyParagraphWithCaretCommand extends EditCommand { ReplaceNodeWithEmptyParagraphWithCaretCommand({ required this.nodeId, }); @@ -659,7 +662,7 @@ class DeleteContentRequest implements EditRequest { final DocumentRange documentRange; } -class DeleteContentCommand implements EditCommand { +class DeleteContentCommand extends EditCommand { DeleteContentCommand({ required this.documentRange, }); @@ -669,6 +672,9 @@ class DeleteContentCommand implements EditCommand { @override HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override + String describe() => "Delete content within range: $documentRange"; + @override void execute(EditContext context, CommandExecutor executor) { _log.log('DeleteSelectionCommand', 'DocumentEditor: deleting selection: $documentRange'); @@ -1039,7 +1045,7 @@ class DeleteNodeRequest implements EditRequest { final String nodeId; } -class DeleteNodeCommand implements EditCommand { +class DeleteNodeCommand extends EditCommand { DeleteNodeCommand({ required this.nodeId, }); diff --git a/super_editor/lib/src/default_editor/paragraph.dart b/super_editor/lib/src/default_editor/paragraph.dart index 3b1526632..986e3ee03 100644 --- a/super_editor/lib/src/default_editor/paragraph.dart +++ b/super_editor/lib/src/default_editor/paragraph.dart @@ -226,7 +226,7 @@ class ChangeParagraphAlignmentRequest implements EditRequest { int get hashCode => nodeId.hashCode ^ alignment.hashCode; } -class ChangeParagraphAlignmentCommand implements EditCommand { +class ChangeParagraphAlignmentCommand extends EditCommand { const ChangeParagraphAlignmentCommand({ required this.nodeId, required this.alignment, @@ -292,7 +292,7 @@ class ChangeParagraphBlockTypeRequest implements EditRequest { int get hashCode => nodeId.hashCode ^ blockType.hashCode; } -class ChangeParagraphBlockTypeCommand implements EditCommand { +class ChangeParagraphBlockTypeCommand extends EditCommand { const ChangeParagraphBlockTypeCommand({ required this.nodeId, required this.blockType, @@ -338,7 +338,7 @@ class CombineParagraphsRequest implements EditRequest { /// in reverse order, the command fizzles. /// /// If both nodes are not `ParagraphNode`s, the command fizzles. -class CombineParagraphsCommand implements EditCommand { +class CombineParagraphsCommand extends EditCommand { CombineParagraphsCommand({ required this.firstNodeId, required this.secondNodeId, @@ -444,7 +444,7 @@ final _defaultAttributionsToExtend = { /// given `splitPosition`, placing all text after `splitPosition` in a /// new `ParagraphNode` with the given `newNodeId`, inserted after the /// original node. -class SplitParagraphCommand implements EditCommand { +class SplitParagraphCommand extends EditCommand { SplitParagraphCommand({ required this.nodeId, required this.splitPosition, @@ -575,7 +575,7 @@ class SplitParagraphCommand implements EditCommand { } } -class DeleteUpstreamAtBeginningOfParagraphCommand implements EditCommand { +class DeleteUpstreamAtBeginningOfParagraphCommand extends EditCommand { DeleteUpstreamAtBeginningOfParagraphCommand(this.node); final DocumentNode node; @@ -784,7 +784,7 @@ ExecutionInstruction anyCharacterToInsertInParagraph({ return didInsertCharacter ? ExecutionInstruction.haltExecution : ExecutionInstruction.continueExecution; } -class DeleteParagraphCommand implements EditCommand { +class DeleteParagraphCommand extends EditCommand { DeleteParagraphCommand({ required this.nodeId, }); diff --git a/super_editor/lib/src/default_editor/tasks.dart b/super_editor/lib/src/default_editor/tasks.dart index f121792ee..d72b58095 100644 --- a/super_editor/lib/src/default_editor/tasks.dart +++ b/super_editor/lib/src/default_editor/tasks.dart @@ -386,7 +386,7 @@ class ChangeTaskCompletionRequest implements EditRequest { int get hashCode => nodeId.hashCode ^ isComplete.hashCode; } -class ChangeTaskCompletionCommand implements EditCommand { +class ChangeTaskCompletionCommand extends EditCommand { ChangeTaskCompletionCommand({required this.nodeId, required this.isComplete}); final String nodeId; @@ -433,7 +433,7 @@ class ConvertParagraphToTaskRequest implements EditRequest { int get hashCode => nodeId.hashCode ^ isComplete.hashCode; } -class ConvertParagraphToTaskCommand implements EditCommand { +class ConvertParagraphToTaskCommand extends EditCommand { const ConvertParagraphToTaskCommand({ required this.nodeId, this.isComplete = false, @@ -467,7 +467,7 @@ class ConvertParagraphToTaskCommand implements EditCommand { } } -class ConvertTaskToParagraphCommand implements EditCommand { +class ConvertTaskToParagraphCommand extends EditCommand { const ConvertTaskToParagraphCommand({ required this.nodeId, this.paragraphMetadata, @@ -514,7 +514,7 @@ class SplitExistingTaskRequest implements EditRequest { final String? newNodeId; } -class SplitExistingTaskCommand implements EditCommand { +class SplitExistingTaskCommand extends EditCommand { const SplitExistingTaskCommand({ required this.nodeId, required this.splitOffset, diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 275611f2d..f7de25021 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -1192,7 +1192,7 @@ class AddTextAttributionsRequest implements EditRequest { // TODO: the add/remove/toggle commands are almost identical except for what they // do to ranges of text. Pull out the common range calculation behavior. /// Applies the given `attributions` to the given `documentSelection`. -class AddTextAttributionsCommand implements EditCommand { +class AddTextAttributionsCommand extends EditCommand { AddTextAttributionsCommand({ required this.documentRange, required this.attributions, @@ -1314,7 +1314,7 @@ class RemoveTextAttributionsRequest implements EditRequest { } /// Removes the given `attributions` from the given `documentSelection`. -class RemoveTextAttributionsCommand implements EditCommand { +class RemoveTextAttributionsCommand extends EditCommand { RemoveTextAttributionsCommand({ required this.documentRange, required this.attributions, @@ -1436,7 +1436,7 @@ class ToggleTextAttributionsRequest implements EditRequest { /// if none of the content in the selection contains any of the /// given `attributions`. Otherwise, all the given `attributions` /// are removed from the content within the `documentSelection`. -class ToggleTextAttributionsCommand implements EditCommand { +class ToggleTextAttributionsCommand extends EditCommand { ToggleTextAttributionsCommand({ required this.documentRange, required this.attributions, @@ -1621,7 +1621,7 @@ class ChangeSingleColumnLayoutComponentStylesRequest implements EditRequest { final SingleColumnLayoutComponentStyles styles; } -class ChangeSingleColumnLayoutComponentStylesCommand implements EditCommand { +class ChangeSingleColumnLayoutComponentStylesCommand extends EditCommand { ChangeSingleColumnLayoutComponentStylesCommand({ required this.nodeId, required this.styles, @@ -1660,7 +1660,7 @@ class InsertTextRequest implements EditRequest { final Set attributions; } -class InsertTextCommand implements EditCommand { +class InsertTextCommand extends EditCommand { InsertTextCommand({ required this.documentPosition, required this.textToInsert, @@ -1674,6 +1674,10 @@ class InsertTextCommand implements EditCommand { @override HistoryBehavior get historyBehavior => HistoryBehavior.undoable; + @override + String describe() => + "Insert text - ${documentPosition.nodeId} @ ${(documentPosition.nodePosition as TextNodePosition).offset} - '$textToInsert'"; + @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); @@ -1827,7 +1831,7 @@ class InsertAttributedTextRequest implements EditRequest { final AttributedText textToInsert; } -class InsertAttributedTextCommand implements EditCommand { +class InsertAttributedTextCommand extends EditCommand { InsertAttributedTextCommand({ required this.documentPosition, required this.textToInsert, diff --git a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart index ba7e75645..1da709fc9 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart @@ -112,7 +112,7 @@ class SubmitComposingActionTagRequest implements EditRequest { const SubmitComposingActionTagRequest(); } -class SubmitComposingActionTagCommand implements EditCommand { +class SubmitComposingActionTagCommand extends EditCommand { @override HistoryBehavior get historyBehavior => HistoryBehavior.undoable; @@ -186,7 +186,7 @@ class CancelComposingActionTagRequest implements EditRequest { int get hashCode => tagRule.hashCode; } -class CancelComposingActionTagCommand implements EditCommand { +class CancelComposingActionTagCommand extends EditCommand { const CancelComposingActionTagCommand(this._tagRule); final TagRule _tagRule; 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 36f1b6782..c8a54b35b 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 @@ -162,7 +162,7 @@ class FillInComposingStableTagRequest implements EditRequest { int get hashCode => tag.hashCode ^ tagRule.hashCode; } -class FillInComposingUserTagCommand implements EditCommand { +class FillInComposingUserTagCommand extends EditCommand { const FillInComposingUserTagCommand( this._tag, this._tagRule, @@ -281,7 +281,7 @@ class CancelComposingStableTagRequest implements EditRequest { int get hashCode => tagRule.hashCode; } -class CancelComposingStableTagCommand implements EditCommand { +class CancelComposingStableTagCommand extends EditCommand { const CancelComposingStableTagCommand(this._tagRule); final TagRule _tagRule; diff --git a/super_editor/test/super_editor/infrastructure/editor_test.dart b/super_editor/test/super_editor/infrastructure/editor_test.dart index 4912550ed..baac6224b 100644 --- a/super_editor/test/super_editor/infrastructure/editor_test.dart +++ b/super_editor/test/super_editor/infrastructure/editor_test.dart @@ -690,7 +690,7 @@ class _ExpandingCommandRequest implements EditRequest { final int levelsOfGeneration; } -class _ExpandingCommand implements EditCommand { +class _ExpandingCommand extends EditCommand { const _ExpandingCommand(this.request); final _ExpandingCommandRequest request; diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index e04a33cce..86000e172 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -149,9 +149,11 @@ void main() { print("------- UNDO ------"); print(""); await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); expect(editContext.document.nodes.first, isA()); - expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "--- "); + // TODO: Change this expectation to include a trailing space " ". Undoing the conversion shouldn't remove teh space + expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—-"); }); }); } From 50deab3fbd4ea2db9c381a19ed9c06b398c6feb6 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 25 May 2024 23:01:24 -0700 Subject: [PATCH 11/19] Changed EditReactions to be two-phase: modifyContent and react. Also added react() changes to new transaction. --- super_editor/lib/src/core/editor.dart | 70 ++++++++++++++++--- .../composer/composer_reactions.dart | 2 +- .../default_document_editor_reactions.dart | 10 +-- .../text_tokenizing/action_tags.dart | 2 +- .../text_tokenizing/pattern_tags.dart | 2 +- .../text_tokenizing/stable_tags.dart | 4 +- .../infrastructure/editor_test.dart | 18 ++--- .../super_editor_undo_redo_test.dart | 3 +- .../src/markdown_inline_upstream_plugin.dart | 2 +- .../test/custom_parsers/upsell_block.dart | 5 ++ 10 files changed, 86 insertions(+), 32 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 7f7bc9265..dae503387 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -298,13 +298,31 @@ class Editor implements RequestDispatcher { print(""); _isReacting = true; + + // First, let reactions modify the content of the active transaction. + for (final reaction in reactionPipeline) { + // Note: we pass the active change list because reactions will cause more + // changes to be added to that list. + print( + "${DateTime.now().microsecondsSinceEpoch} Running modify content reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); + reaction.modifyContent(context, this, _activeChangeList!); + } + + // Second, start a new transaction and let reactions add separate changes. + // ignore: prefer_const_constructors + _transaction = CommandTransaction([]); for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. print( - "${DateTime.now().microsecondsSinceEpoch} Running reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); + "${DateTime.now().microsecondsSinceEpoch} Running react reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); reaction.react(context, this, _activeChangeList!); } + + if (_transaction!.commands.isNotEmpty) { + _commandHistory.add(_transaction!); + } + print("${DateTime.now().microsecondsSinceEpoch} DONE _reactToChanges()"); // FIXME: try removing this notify listeners @@ -662,27 +680,57 @@ class DocumentEdit implements EditEvent { int get hashCode => change.hashCode; } -/// An object that's notified with a change list from one or more -/// commands that were just executed. +/// An object that's notified with a change list from one or more commands that were just +/// executed. /// -/// An [EditReaction] can use the given [executor] to spawn additional -/// [EditCommand]s that should run in response to the [changeList]. +/// An [EditReaction] can use the given [reactionExecutor] to spawn additional [EditCommand]s +/// that should run in response to the [changeList]. abstract class EditReaction { - void react(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList); + const EditReaction(); + + /// Executes additional [modifications] within the current editor transaction. + /// + /// If undo is run, the recent changes AND the [modifications] will be undone, together. + /// This is useful, for example, for a reaction such as spell-check, whose reaction is + /// tied directly to the content and shouldn't stand on its own. + /// + /// To execute actions that are undone on their own, use [react]. + void modifyContent(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) {} + + /// Executes additional [actions] in a new standalone transaction. + /// + /// If undo is run, these changes will be undone, but the changes leading up to this + /// call to [react] will not be undone by that undo call. + /// + /// To execute additional actions that are undone at the same time as the preceding + /// changes, use [modifyContent]. + void react(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) {} } /// An [EditReaction] that delegates its reaction to a given callback function. -class FunctionalEditReaction implements EditReaction { - FunctionalEditReaction(this._react); +class FunctionalEditReaction extends EditReaction { + FunctionalEditReaction({ + Reaction? modifyContent, + Reaction? react, + }) : _modifyContent = modifyContent, + _react = react, + assert(modifyContent != null || react != null); - final void Function(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) - _react; + final Reaction? _modifyContent; + final Reaction? _react; + + @override + void modifyContent(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) => + _modifyContent?.call(editorContext, requestDispatcher, changeList); @override void react(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) => - _react(editorContext, requestDispatcher, changeList); + _react?.call(editorContext, requestDispatcher, changeList); } +typedef Reaction = void Function( + EditContext editorContext, RequestDispatcher requestDispatcher, List changeList); + /// An object that's notified with a change list from one or more /// commands that were just executed within a [Editor]. /// diff --git a/super_editor/lib/src/default_editor/composer/composer_reactions.dart b/super_editor/lib/src/default_editor/composer/composer_reactions.dart index a868f03da..f8cc82e8a 100644 --- a/super_editor/lib/src/default_editor/composer/composer_reactions.dart +++ b/super_editor/lib/src/default_editor/composer/composer_reactions.dart @@ -37,7 +37,7 @@ import 'package:super_editor/src/default_editor/text.dart'; /// Conversely, if the caret moves due to the user typing a character, or /// if the selection is expanded, then this reaction doesn't activate any /// styles. -class UpdateComposerTextStylesReaction implements EditReaction { +class UpdateComposerTextStylesReaction extends EditReaction { UpdateComposerTextStylesReaction({ Set? stylesToExtend, }) : _stylesToExtend = stylesToExtend ?? defaultExtendableStyles; diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index c879c3fa4..c05a355a2 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -253,7 +253,7 @@ class BlockquoteConversionReaction extends ParagraphPrefixConversionReaction { /// the node's text is kept. /// /// Applied only to all [TextNode]s. -class HorizontalRuleConversionReaction implements EditReaction { +class HorizontalRuleConversionReaction extends EditReaction { // Matches "---" or "—-" (an em-dash followed by a regular dash) at the beginning of a line, // followed by a space. static final _hrPattern = RegExp(r'^(---|—-)\s'); @@ -330,7 +330,7 @@ class HorizontalRuleConversionReaction implements EditReaction { /// Base class for [EditReaction]s that want to take action when the user types text at /// the beginning of a paragraph, which matches a given [RegExp]. -abstract class ParagraphPrefixConversionReaction implements EditReaction { +abstract class ParagraphPrefixConversionReaction extends EditReaction { const ParagraphPrefixConversionReaction({ bool requireSpaceInsertion = true, }) : _requireSpaceInsertion = requireSpaceInsertion; @@ -391,7 +391,7 @@ abstract class ParagraphPrefixConversionReaction implements EditReaction { /// When the user creates a new node, and the previous node is just a URL /// to an image, the replaces the previous node with the referenced image. -class ImageUrlConversionReaction implements EditReaction { +class ImageUrlConversionReaction extends EditReaction { const ImageUrlConversionReaction(); @override @@ -534,7 +534,7 @@ class ImageUrlConversionReaction implements EditReaction { /// A plain text URL only has a link applied to it when the user enters a space " " /// after a token that looks like a URL. If the user doesn't enter a trailing space, /// or the preceding token doesn't look like a URL, then the link attribution isn't aplied. -class LinkifyReaction implements EditReaction { +class LinkifyReaction extends EditReaction { const LinkifyReaction({ this.updatePolicy = LinkUpdatePolicy.preserve, }); @@ -883,7 +883,7 @@ enum LinkUpdatePolicy { /// dash are removed and an em-dash (—) is inserted. /// /// This reaction applies to all [TextNode]s in the document. -class DashConversionReaction implements EditReaction { +class DashConversionReaction extends EditReaction { const DashConversionReaction(); @override diff --git a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart index 1da709fc9..099a1183c 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/action_tags.dart @@ -264,7 +264,7 @@ class CancelComposingActionTagCommand extends EditCommand { } } -class ActionTagComposingReaction implements EditReaction { +class ActionTagComposingReaction extends EditReaction { ActionTagComposingReaction({ required TagRule tagRule, required OnUpdateComposingActionTag onUpdateComposingActionTag, diff --git a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart index 660964e01..2c8e037dd 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart @@ -199,7 +199,7 @@ class PatternTagIndex with ChangeNotifier implements Editable { /// #. /// ## /// -class PatternTagReaction implements EditReaction { +class PatternTagReaction extends EditReaction { PatternTagReaction({ TagRule tagRule = hashTagRule, }) : _tagRule = tagRule; 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 c8a54b35b..58fbedf73 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 @@ -369,7 +369,7 @@ extension StableTagIndexEditable on EditContext { /// An [EditReaction] that creates, updates, and removes composing stable tags, and commits those /// composing tags, causing them to become uneditable. -class TagUserReaction implements EditReaction { +class TagUserReaction extends EditReaction { const TagUserReaction({ required TagRule tagRule, this.onUpdateComposingStableTag, @@ -1110,7 +1110,7 @@ class ComposingStableTag { } /// An [EditReaction] that prevents partial selection of a stable user tag. -class AdjustSelectionAroundTagReaction implements EditReaction { +class AdjustSelectionAroundTagReaction extends EditReaction { const AdjustSelectionAroundTagReaction(this._tagRule); final TagRule _tagRule; diff --git a/super_editor/test/super_editor/infrastructure/editor_test.dart b/super_editor/test/super_editor/infrastructure/editor_test.dart index baac6224b..f7467a6a4 100644 --- a/super_editor/test/super_editor/infrastructure/editor_test.dart +++ b/super_editor/test/super_editor/infrastructure/editor_test.dart @@ -161,9 +161,11 @@ void main() { }, requestHandlers: List.from(defaultRequestHandlers), reactionPipeline: [ - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { - reactionCount += 1; - }), + FunctionalEditReaction( + react: (editorContext, requestDispatcher, changeList) { + reactionCount += 1; + }, + ), ], ); @@ -201,7 +203,7 @@ void main() { }, requestHandlers: List.from(defaultRequestHandlers), reactionPipeline: [ - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { + FunctionalEditReaction(react: (editorContext, requestDispatcher, changeList) { TextInsertionEvent? insertEEvent; for (final edit in changeList) { if (edit is! DocumentEdit) { @@ -290,7 +292,7 @@ void main() { requestHandlers: List.from(defaultRequestHandlers), reactionPipeline: [ // Reaction 1 causes a change - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { + FunctionalEditReaction(react: (editorContext, requestDispatcher, changeList) { TextInsertionEvent? insertHEvent; for (final edit in changeList) { if (edit is! DocumentEdit) { @@ -321,7 +323,7 @@ void main() { ]); }), // Reaction 2 verifies that it sees the change event from reaction 1. - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { + FunctionalEditReaction(react: (editorContext, requestDispatcher, changeList) { TextInsertionEvent? insertEEvent; for (final edit in changeList) { if (edit is! DocumentEdit) { @@ -375,7 +377,7 @@ void main() { }, requestHandlers: List.from(defaultRequestHandlers), reactionPipeline: [ - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { + FunctionalEditReaction(react: (editorContext, requestDispatcher, changeList) { reactionRunCount += 1; // We expect this reaction to run after we execute a command, but we don't @@ -607,7 +609,7 @@ void main() { final editorPieces = _createStandardEditor( initialDocument: longTextDoc(), additionalReactions: [ - FunctionalEditReaction((editorContext, requestDispatcher, changeList) { + FunctionalEditReaction(react: (editorContext, requestDispatcher, changeList) { expect(changeList.length, 1); final event = changeList.first as DocumentEdit; diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index 86000e172..d42087db5 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -152,8 +152,7 @@ void main() { await widgetTester.pump(); expect(editContext.document.nodes.first, isA()); - // TODO: Change this expectation to include a trailing space " ". Undoing the conversion shouldn't remove teh space - expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—-"); + expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—- "); }); }); } diff --git a/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart b/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart index 41ed40372..7aa572a2b 100644 --- a/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart +++ b/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart @@ -65,7 +65,7 @@ const defaultUpstreamInlineMarkdownParsers = [ /// This reaction only identifies spans of Markdown styles within individual [TextNode]s, which /// immediately precedes the caret. For example, "Hello **bold**|" will apply the bold style, /// but "Hello **bold** wo|" won't apply bold. -class MarkdownInlineUpstreamSyntaxReaction implements EditReaction { +class MarkdownInlineUpstreamSyntaxReaction extends EditReaction { const MarkdownInlineUpstreamSyntaxReaction(this._parsers); final List _parsers; diff --git a/super_editor_markdown/test/custom_parsers/upsell_block.dart b/super_editor_markdown/test/custom_parsers/upsell_block.dart index 129e97be5..1acd11d4d 100644 --- a/super_editor_markdown/test/custom_parsers/upsell_block.dart +++ b/super_editor_markdown/test/custom_parsers/upsell_block.dart @@ -23,6 +23,11 @@ class UpsellNode extends BlockNode with ChangeNotifier { String? copyContent(NodeSelection selection) { return null; } + + @override + DocumentNode copy() { + return UpsellNode(id); + } } /// Markdown block-parser for upsell messages. From a1f70f5cf3a82a5242f92f33734e0b28d624c63f Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sat, 25 May 2024 23:26:56 -0700 Subject: [PATCH 12/19] Fix merge test failure --- super_editor/test/super_editor/text_entry/links_test.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/super_editor/test/super_editor/text_entry/links_test.dart b/super_editor/test/super_editor/text_entry/links_test.dart index 924608f6d..8e2e186eb 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -1767,13 +1767,11 @@ void main() { await tester.doubleTapInParagraph(doc.nodes.first.id, 5); // Replace "google" with "duckduckgo". - // await tester.typeImeText('duckduckgo'); - await tester.typeImeText('du'); + await tester.typeImeText('duckduckgo'); // Ensure the text and the link were updated. final text = SuperEditorInspector.findTextInComponent(doc.nodes.first.id); - // expect(text.text, "www.duckduckgo.com"); - expect(text.text, "www.du.com"); + expect(text.text, "www.duckduckgo.com"); print("Actual text spans: ${text.spans}"); expect( text.hasAttributionsThroughout( From 765ef51e01e5d1e61060108855142efbc27e9692 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Sun, 26 May 2024 17:34:41 -0700 Subject: [PATCH 13/19] Added a number of new test cases for undo/redo. Removed a bunch of print statements. --- .../lib/src/core/document_composer.dart | 6 - super_editor/lib/src/core/editor.dart | 29 +- .../common_editor_operations.dart | 19 +- .../default_document_editor.dart | 1 - .../default_document_editor_reactions.dart | 16 - super_editor/lib/src/default_editor/text.dart | 3 - .../text_tokenizing/pattern_tags.dart | 2 - .../super_editor_undo_redo_test.dart | 372 +++++++++++++----- .../super_editor/text_entry/links_test.dart | 1 - .../text_entry/tagging/pattern_tags_test.dart | 3 - 10 files changed, 279 insertions(+), 173 deletions(-) diff --git a/super_editor/lib/src/core/document_composer.dart b/super_editor/lib/src/core/document_composer.dart index 52344f86e..b567a20b4 100644 --- a/super_editor/lib/src/core/document_composer.dart +++ b/super_editor/lib/src/core/document_composer.dart @@ -118,8 +118,6 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { _didChangeSelectionDuringTransaction = true; } - print("Changing selection in composer to:"); - print("$newSelection"); _latestSelectionChange = DocumentSelectionChange( selection: newSelection, reason: reason, @@ -162,8 +160,6 @@ class MutableDocumentComposer extends DocumentComposer implements Editable { _isInInteractionMode.resumeNotifications(); if (_didReset) { - print("Force notifying composer listeners. Current selection is:"); - print("${_selectionNotifier.value}"); // Our state was reset (possibly for to undo an operation). Anything may have changed. // Force notify all listeners. _didReset = false; @@ -372,8 +368,6 @@ class ChangeSelectionCommand extends EditCommand { final composer = context.find(Editor.composerKey); final initialSelection = composer.selection; - print("ChangeSelectionCommand - Changing selection to:"); - print("$newSelection"); composer.setSelectionWithReason(newSelection, reason); executor.logChanges([ diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index dae503387..ac9341aa3 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -193,12 +193,12 @@ class Editor implements RequestDispatcher { void execute(List requests) { if (requests.isEmpty) { // No changes were requested. Don't waste time starting and ending transactions, etc. - print("No requests were given"); + print("WARNING: Tried to execute requests without providing any requests!"); print(StackTrace.current); return; } - print("Request execution:"); + print("Executing requests:"); for (final request in requests) { print(" - ${request.runtimeType}"); } @@ -270,7 +270,6 @@ class Editor implements RequestDispatcher { } void _onTransactionStart() { - print("_onTransactionStart()"); for (final editable in context._resources.values) { editable.onTransactionStart(); } @@ -278,11 +277,9 @@ class Editor implements RequestDispatcher { void _onTransactionEnd() { for (final editable in context._resources.values) { - print("_onTransactionEnd() - ending transaction on editable: $editable"); editable.onTransactionEnd(_activeChangeList!); } - print("Null'ing out the active change list after ending the transaction"); _activeChangeList = null; } @@ -291,20 +288,12 @@ class Editor implements RequestDispatcher { return; } - print("${DateTime.now().microsecondsSinceEpoch} _reactToChanges()"); - for (final change in _activeChangeList!) { - print(" - ${change.runtimeType}"); - } - print(""); - _isReacting = true; // First, let reactions modify the content of the active transaction. for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. - print( - "${DateTime.now().microsecondsSinceEpoch} Running modify content reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); reaction.modifyContent(context, this, _activeChangeList!); } @@ -314,8 +303,6 @@ class Editor implements RequestDispatcher { for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. - print( - "${DateTime.now().microsecondsSinceEpoch} Running react reaction ${reaction.runtimeType}. Active change list: $_activeChangeList"); reaction.react(context, this, _activeChangeList!); } @@ -323,8 +310,6 @@ class Editor implements RequestDispatcher { _commandHistory.add(_transaction!); } - print("${DateTime.now().microsecondsSinceEpoch} DONE _reactToChanges()"); - // FIXME: try removing this notify listeners // Notify all listeners that care about changes, but won't spawn additional requests. _notifyListeners(); @@ -333,7 +318,7 @@ class Editor implements RequestDispatcher { } void undo() { - print("Running undo()"); + print("RUNNING UNDO"); if (_commandHistory.isEmpty) { return; } @@ -368,7 +353,6 @@ class Editor implements RequestDispatcher { print("Replaying all command history except for the most recent transaction..."); final changeEvents = []; for (final commandTransaction in _commandHistory) { - print("Starting a command transaction."); for (final command in commandTransaction.commands) { print("Executing command: ${command.runtimeType}"); // We re-run the commands without tracking changes and without running reactions @@ -377,10 +361,8 @@ class Editor implements RequestDispatcher { final commandChanges = _executeCommand(command); changeEvents.addAll(commandChanges); } - print("Ending a command transaction."); } - print("Ending the transaction for all Editables..."); for (final editable in context._resources.values) { // Let editables start notifying listeners again. editable.onTransactionEnd(changeEvents); @@ -390,7 +372,7 @@ class Editor implements RequestDispatcher { } void redo() { - print("Running redo()"); + print("RUNNING REDO"); if (_commandFuture.isEmpty) { return; } @@ -412,7 +394,8 @@ class Editor implements RequestDispatcher { edits.addAll(commandEdits); } _commandHistory.add(commandTransaction); - print("Done with redo()"); + + print("DONE WITH REDO"); for (final editable in context._resources.values) { // Let editables start notifying listeners again. diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index 9efdfeea0..383af3fc2 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -2190,6 +2190,10 @@ class CommonEditorOperations { void paste() { DocumentPosition pastePosition = composer.selection!.extent; + // Start a transaction so that we can capture both the initial deletion behavior, + // and the clipboard content insertion, all as one transaction. + editor.startTransaction(); + // Delete all currently selected content. if (!composer.selection!.isCollapsed) { pastePosition = CommonEditorOperations.getDocumentPositionAfterExpandedDeletion( @@ -2216,6 +2220,8 @@ class CommonEditorOperations { composer: composer, pastePosition: pastePosition, ); + + editor.endTransaction(); } Future _paste({ @@ -2230,7 +2236,6 @@ class CommonEditorOperations { PasteEditorRequest( content: content, pastePosition: pastePosition, - composer: composer, ), ]); } @@ -2240,26 +2245,21 @@ class PasteEditorRequest implements EditRequest { PasteEditorRequest({ required this.content, required this.pastePosition, - required this.composer, }); final String content; final DocumentPosition pastePosition; - final DocumentComposer composer; } class PasteEditorCommand extends EditCommand { PasteEditorCommand({ required String content, required DocumentPosition pastePosition, - required DocumentComposer composer, }) : _content = content, - _pastePosition = pastePosition, - _composer = composer; + _pastePosition = pastePosition; final String _content; final DocumentPosition _pastePosition; - final DocumentComposer _composer; @override HistoryBehavior get historyBehavior => HistoryBehavior.undoable; @@ -2267,6 +2267,7 @@ class PasteEditorCommand extends EditCommand { @override void execute(EditContext context, CommandExecutor executor) { final document = context.find(Editor.documentKey); + final composer = context.find(Editor.composerKey); final currentNodeWithSelection = document.getNodeById(_pastePosition.nodeId); if (currentNodeWithSelection is! TextNode) { throw Exception('Can\'t handle pasting text within node of type: $currentNodeWithSelection'); @@ -2345,7 +2346,7 @@ class PasteEditorCommand extends EditCommand { SelectionReason.userInteraction, ), ); - editorOpsLog.fine('New selection after paste operation: ${_composer.selection}'); + editorOpsLog.fine('New selection after paste operation: ${composer.selection}'); editorOpsLog.fine('Done with paste command.'); } @@ -2452,8 +2453,6 @@ class DeleteUpstreamCharacterCommand extends EditCommand { final previousCharacterOffset = getCharacterStartBounds(textNode.text.text, currentTextOffset); // Delete the selected content. - print( - "Deleting character at $previousCharacterOffset. Moving selection to: ${textNode.selectionAt(previousCharacterOffset)}"); executor ..executeCommand( DeleteContentCommand( diff --git a/super_editor/lib/src/default_editor/default_document_editor.dart b/super_editor/lib/src/default_editor/default_document_editor.dart index aa66907a1..e0d106888 100644 --- a/super_editor/lib/src/default_editor/default_document_editor.dart +++ b/super_editor/lib/src/default_editor/default_document_editor.dart @@ -219,7 +219,6 @@ final defaultRequestHandlers = List.unmodifiable([ ? PasteEditorCommand( content: request.content, pastePosition: request.pastePosition, - composer: request.composer, ) : null, ]); diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index c4abbc70c..b306cf285 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -71,7 +71,6 @@ class HeaderConversionReaction extends ParagraphPrefixConversionReaction { ParagraphNode paragraph, String match, ) { - print("FOUND PREFIX MATCH!"); final prefixLength = match.length - 1; // -1 for the space on the end late Attribution headerAttribution = _getHeaderAttributionForLevel(prefixLength); @@ -262,12 +261,10 @@ class HorizontalRuleConversionReaction extends EditReaction { @override void react(EditContext editorContext, RequestDispatcher requestDispatcher, List changeList) { - print("HR Reaction"); if (changeList.length < 2) { // This reaction requires at least an insertion event and a selection change event. // There are less than two events in the the change list, therefore this reaction // shouldn't apply. Fizzle. - print("Less than 2 changes in change list - fizzling."); return; } @@ -275,7 +272,6 @@ class HorizontalRuleConversionReaction extends EditReaction { final didTypeSpace = EditInspector.didTypeSpace(document, changeList); if (!didTypeSpace) { - print("User didn't type a space - fizzling"); return; } @@ -292,11 +288,9 @@ class HorizontalRuleConversionReaction extends EditReaction { final paragraph = document.getNodeById(textInsertionEvent.nodeId) as TextNode; final match = _hrPattern.firstMatch(paragraph.text.text)?.group(0); if (match == null) { - print("No HR pattern match. Fizzling."); return; } - print("User inserted HR pattern. Replacing text with HR."); // The user typed a horizontal rule pattern at the beginning of a paragraph. // - Remove the dashes and the space. // - Insert a horizontal rule before the paragraph. @@ -348,27 +342,22 @@ abstract class ParagraphPrefixConversionReaction extends EditReaction { @override void react(EditContext editContext, RequestDispatcher requestDispatcher, List changeList) { - print("Running paragraph prefix matcher for pattern: ${pattern.pattern}"); final document = editContext.find(Editor.documentKey); final typedText = EditInspector.findLastTextUserTyped(document, changeList); if (typedText == null) { - print("User didn't type any text. Fizzling."); return; } if (_requireSpaceInsertion && !typedText.text.text.endsWith(" ")) { - print("User didn't end with a space. Fizzling."); return; } final paragraph = document.getNodeById(typedText.nodeId); if (paragraph is! ParagraphNode) { - print("Not a ParagraphNode. Fizzling."); return; } final match = pattern.firstMatch(paragraph.text.text)?.group(0); if (match == null) { - print("Didn't find a match for the pattern. Fizzling."); return; } @@ -544,7 +533,6 @@ class LinkifyReaction extends EditReaction { @override void react(EditContext editContext, RequestDispatcher requestDispatcher, List edits) { - print("(MAYBE) Running linkify reaction"); final document = editContext.find(Editor.documentKey); final composer = editContext.find(Editor.composerKey); bool didInsertSpace = false; @@ -982,9 +970,7 @@ class EditInspector { // insertion event with a space " ". DocumentEdit? lastDocumentEditEvent; SelectionChangeEvent? lastSelectionChangeEvent; - print("Inspecting ${edits.length} edits"); for (int i = edits.length - 1; i >= 0; i -= 1) { - print("Index: $i"); if (edits[i] is DocumentEdit) { lastDocumentEditEvent = edits[i] as DocumentEdit; } else if (lastSelectionChangeEvent == null && edits[i] is SelectionChangeEvent) { @@ -996,11 +982,9 @@ class EditInspector { } } if (lastDocumentEditEvent == null) { - print("There was no document edit. Fizzling."); return false; } if (lastSelectionChangeEvent == null) { - print("There was no selection change after inserting space. Fizzling."); return false; } diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 8ac3c9caa..0791a6318 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -1704,8 +1704,6 @@ class InsertTextCommand extends EditCommand { return; } - print("Inserting text: '$textToInsert'"); - final textPosition = documentPosition.nodePosition as TextPosition; final textOffset = textPosition.offset; textNode.text = textNode.text.insertString( @@ -1713,7 +1711,6 @@ class InsertTextCommand extends EditCommand { startOffset: textOffset, applyAttributions: attributions, ); - print("Updated node text: '${textNode.text}'"); executor.logChanges([ DocumentEdit( diff --git a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart index 2c8e037dd..17db46005 100644 --- a/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart +++ b/super_editor/lib/src/default_editor/text_tokenizing/pattern_tags.dart @@ -360,8 +360,6 @@ class PatternTagReaction extends EditReaction { editorPatternTagsLog.fine( "Found a pattern tag around caret: '${tagAroundCaret.indexedTag.tag}' - surrounding it with an attribution: ${tagAroundCaret.indexedTag.startOffset} -> ${tagAroundCaret.indexedTag.endOffset}"); - print( - "FOUND A TAG: ${selectedNode.text.substring(tagAroundCaret.indexedTag.startOffset, tagAroundCaret.indexedTag.endOffset)}"); requestDispatcher.execute([ // Remove the old pattern tag attribution(s). RemoveTextAttributionsRequest( diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index d42087db5..632ec0065 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -9,132 +9,266 @@ import 'supereditor_test_tools.dart'; void main() { group("Super Editor > undo redo >", () { - testWidgets("insert a word", (widgetTester) async { - final document = deserializeMarkdownToDocument("Hello world"); - final composer = MutableDocumentComposer(); - final editor = createDefaultDocumentEditor(document: document, composer: composer); - final paragraphId = document.nodes.first.id; - - editor.execute([ - ChangeSelectionRequest( - DocumentSelection.collapsed( - position: DocumentPosition( + group("text insertion >", () { + testWidgets("insert a word", (widgetTester) async { + final document = deserializeMarkdownToDocument("Hello world"); + final composer = MutableDocumentComposer(); + final editor = createDefaultDocumentEditor(document: document, composer: composer); + final paragraphId = document.nodes.first.id; + + editor.execute([ + ChangeSelectionRequest( + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 6), + ), + ), + SelectionChangeType.placeCaret, + SelectionReason.userInteraction, + ) + ]); + + editor.execute([ + InsertTextRequest( + documentPosition: DocumentPosition( nodeId: paragraphId, nodePosition: const TextNodePosition(offset: 6), ), + textToInsert: "another", + attributions: {}, ), - SelectionChangeType.placeCaret, - SelectionReason.userInteraction, - ) - ]); - - editor.execute([ - InsertTextRequest( - documentPosition: DocumentPosition( - nodeId: paragraphId, - nodePosition: const TextNodePosition(offset: 6), + ]); + + expect(serializeDocumentToMarkdown(document), "Hello another world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 13), + ), ), - textToInsert: "another", - attributions: {}, - ), - ]); - - expect(serializeDocumentToMarkdown(document), "Hello another world"); - expect( - composer.selection, - DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: paragraphId, - nodePosition: const TextNodePosition(offset: 13), + ); + + // Undo the event. + editor.undo(); + + expect(serializeDocumentToMarkdown(document), "Hello world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 6), + ), ), - ), - ); - - // Undo the event. - editor.undo(); - - expect(serializeDocumentToMarkdown(document), "Hello world"); - expect( - composer.selection, - DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: paragraphId, - nodePosition: const TextNodePosition(offset: 6), + ); + + // Redo the event. + editor.redo(); + + expect(serializeDocumentToMarkdown(document), "Hello another world"); + expect( + composer.selection, + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: paragraphId, + nodePosition: const TextNodePosition(offset: 13), + ), ), - ), - ); - - // Redo the event. - editor.redo(); - - expect(serializeDocumentToMarkdown(document), "Hello another world"); - expect( - composer.selection, - DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: paragraphId, - nodePosition: const TextNodePosition(offset: 13), + ); + }); + + testWidgetsOnMac("type by character", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + // Type characters. + await widgetTester.typeImeText("Hello"); + + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); + expect( + SuperEditorInspector.findDocumentSelection(), + const DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: "1", + nodePosition: TextNodePosition(offset: 5), + ), ), - ), - ); + ); + + // --- Undo character insertions --- + await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("Hell", "1", 4); + + await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("Hel", "1", 3); + + await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("He", "1", 2); + + await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("H", "1", 1); + + await widgetTester.pressCmdZ(widgetTester); + _expectDocumentWithCaret("", "1", 0); + + //----- Redo Changes ---- + await widgetTester.pressCmdShiftZ(widgetTester); + _expectDocumentWithCaret("H", "1", 1); + + await widgetTester.pressCmdShiftZ(widgetTester); + _expectDocumentWithCaret("He", "1", 2); + + await widgetTester.pressCmdShiftZ(widgetTester); + _expectDocumentWithCaret("Hel", "1", 3); + + await widgetTester.pressCmdShiftZ(widgetTester); + _expectDocumentWithCaret("Hell", "1", 4); + + await widgetTester.pressCmdShiftZ(widgetTester); + _expectDocumentWithCaret("Hello", "1", 5); + }); }); - testWidgetsOnMac("type by character", (widgetTester) async { - await widgetTester // - .createDocument() - .withSingleEmptyParagraph() - .pump(); + group("content conversions >", () { + testWidgetsOnMac("paragraph to header", (widgetTester) async { + final editContext = await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await widgetTester.placeCaretInParagraph("1", 0); - // Type characters. - await widgetTester.typeImeText("Hello"); + // Type text that causes a conversion to a header node. + await widgetTester.typeImeText("# "); - expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); - expect( - SuperEditorInspector.findDocumentSelection(), - const DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: "1", - nodePosition: TextNodePosition(offset: 5), - ), - ), - ); + // Ensure that the paragraph is now a header. + final document = editContext.document; + var paragraph = document.nodes.first as ParagraphNode; + expect(paragraph.metadata['blockType'], header1Attribution); + expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, ""); - // --- Undo character insertions --- - await widgetTester.pressCmdZ(widgetTester); - _expectDocumentWithCaret("Hell", "1", 4); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); - await widgetTester.pressCmdZ(widgetTester); - _expectDocumentWithCaret("Hel", "1", 3); + // Ensure that the header attribution is gone. + paragraph = document.nodes.first as ParagraphNode; + expect(paragraph.metadata['blockType'], paragraphAttribution); + expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, "# "); + }); - await widgetTester.pressCmdZ(widgetTester); - _expectDocumentWithCaret("He", "1", 2); + testWidgetsOnMac("dashes to em dash", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); - await widgetTester.pressCmdZ(widgetTester); - _expectDocumentWithCaret("H", "1", 1); + await widgetTester.placeCaretInParagraph("1", 0); - await widgetTester.pressCmdZ(widgetTester); - _expectDocumentWithCaret("", "1", 0); + // Type text that causes a conversion to a header node. + await widgetTester.typeImeText("--"); - //----- Redo Changes ---- - await widgetTester.pressCmdShiftZ(widgetTester); - _expectDocumentWithCaret("H", "1", 1); + // Ensure that the paragraph is now a header. + expect(SuperEditorInspector.findTextInComponent("1").text, "—"); - await widgetTester.pressCmdShiftZ(widgetTester); - _expectDocumentWithCaret("He", "1", 2); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); - await widgetTester.pressCmdShiftZ(widgetTester); - _expectDocumentWithCaret("Hel", "1", 3); + // Ensure that the em dash was reverted to the regular dashes. + expect(SuperEditorInspector.findTextInComponent("1").text, "--"); - await widgetTester.pressCmdShiftZ(widgetTester); - _expectDocumentWithCaret("Hell", "1", 4); + // Continue typing. + await widgetTester.typeImeText(" "); - await widgetTester.pressCmdShiftZ(widgetTester); - _expectDocumentWithCaret("Hello", "1", 5); + // Ensure that the dashes weren't reconverted into an em dash. + expect(SuperEditorInspector.findTextInComponent("1").text, "-- "); + }); + + testWidgetsOnMac("paragraph to list item", (widgetTester) async { + final editContext = await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + // Type text that causes a conversion to a list item node. + await widgetTester.typeImeText("1. "); + + // Ensure that the paragraph is now a list item. + final document = editContext.document; + var node = document.nodes.first as TextNode; + expect(node, isA()); + expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, ""); + + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the node is back to a paragraph. + node = document.nodes.first as TextNode; + expect(node, isA()); + expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, "1. "); + }); + + testWidgetsOnMac("url to a link", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + // Type text that causes a conversion to a link. + await widgetTester.typeImeText("google.com "); + + // Ensure that the URL is now linkified. + expect( + SuperEditorInspector.findTextInComponent("1").getAttributionSpansByFilter((a) => a is LinkAttribution), + { + const AttributionSpan( + attribution: LinkAttribution("https://google.com"), + start: 0, + end: 9, + ), + }, + ); + + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the URL is no longer linkified. + expect( + SuperEditorInspector.findTextInComponent("1").getAttributionSpansByFilter((a) => a is LinkAttribution), + const {}, + ); + }); + + testWidgetsOnMac("paragraph to horizontal rule", (widgetTester) async { + final editContext = await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + await widgetTester.typeImeText("--- "); + expect(editContext.document.nodes.first, isA()); + + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + expect(editContext.document.nodes.first, isA()); + expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—- "); + }); }); - testWidgetsOnMac("convert to horizontal rule", (widgetTester) async { + testWidgetsOnMac("pasted content", (widgetTester) async { final editContext = await widgetTester // .createDocument() .withSingleEmptyParagraph() @@ -142,17 +276,39 @@ void main() { await widgetTester.placeCaretInParagraph("1", 0); - await widgetTester.typeImeText("--- "); - expect(editContext.document.nodes.first, isA()); + // Paste multiple nodes of content. + widgetTester.simulateClipboard(); + await widgetTester.setSimulatedClipboardContent(''' +This is paragraph 1 +This is paragraph 2 +This is paragraph 3'''); + await widgetTester.pressCmdV(); + + // Ensure the pasted content was applied as expected. + final document = editContext.document; + expect(document.nodes.length, 3); + expect(SuperEditorInspector.findTextInComponent(document.nodes[0].id).text, "This is paragraph 1"); + expect(SuperEditorInspector.findTextInComponent(document.nodes[1].id).text, "This is paragraph 2"); + expect(SuperEditorInspector.findTextInComponent(document.nodes[2].id).text, "This is paragraph 3"); - print(""); - print("------- UNDO ------"); - print(""); + // Undo the paste. await widgetTester.pressCmdZ(widgetTester); await widgetTester.pump(); - expect(editContext.document.nodes.first, isA()); - expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—- "); + // Ensure we're back to a single empty paragraph. + expect(document.nodes.length, 1); + expect(SuperEditorInspector.findTextInComponent(document.nodes[0].id).text, ""); + + // Redo the paste + // TODO: remove WidgetTester as required argument to this robot method + await widgetTester.pressCmdShiftZ(widgetTester); + await widgetTester.pump(); + + // Ensure the pasted content was applied as expected. + expect(document.nodes.length, 3); + expect(SuperEditorInspector.findTextInComponent(document.nodes[0].id).text, "This is paragraph 1"); + expect(SuperEditorInspector.findTextInComponent(document.nodes[1].id).text, "This is paragraph 2"); + expect(SuperEditorInspector.findTextInComponent(document.nodes[2].id).text, "This is paragraph 3"); }); }); } diff --git a/super_editor/test/super_editor/text_entry/links_test.dart b/super_editor/test/super_editor/text_entry/links_test.dart index 8e2e186eb..51a4aff47 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -1772,7 +1772,6 @@ void main() { // Ensure the text and the link were updated. final text = SuperEditorInspector.findTextInComponent(doc.nodes.first.id); expect(text.text, "www.duckduckgo.com"); - print("Actual text spans: ${text.spans}"); expect( text.hasAttributionsThroughout( attributions: { diff --git a/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart b/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart index ed9c7f0a6..9bc3a8322 100644 --- a/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart +++ b/super_editor/test/super_editor/text_entry/tagging/pattern_tags_test.dart @@ -463,17 +463,14 @@ void main() { await tester.typeImeText("#abcdefghij "); // Delete part of the end. - print("-------- DELETING AT END ---------"); await tester.placeCaretInParagraph("1", 11); await tester.pressBackspace(); // Delete part of the middle. - print("-------- DELETING IN MIDDLE ---------"); await tester.placeCaretInParagraph("1", 6); await tester.pressBackspace(); // Delete part of the beginning. - print("-------- DELETING AT BEGINNING ---------"); await tester.placeCaretInParagraph("1", 2); await tester.pressBackspace(); From 1eddf53c5e6452d193348f7ee6c8c50c6b5f18c9 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 27 May 2024 00:05:57 -0700 Subject: [PATCH 14/19] Added a basic concept of transaction merging so that undo doesn't apply per character. --- super_editor/lib/src/core/editor.dart | 164 +++++++++++++++++- .../default_document_editor.dart | 2 + super_editor/pubspec.yaml | 1 + .../super_editor_undo_redo_test.dart | 66 +++++++ .../super_editor/supereditor_test_tools.dart | 13 +- 5 files changed, 240 insertions(+), 6 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index ac9341aa3..89edd9c65 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -1,8 +1,10 @@ import 'dart:math'; import 'package:attributed_text/attributed_text.dart'; +import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; import 'package:super_editor/src/default_editor/paragraph.dart'; +import 'package:super_editor/src/default_editor/text.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; import 'package:uuid/uuid.dart'; @@ -53,6 +55,7 @@ class Editor implements RequestDispatcher { Editor({ required Map editables, List? requestHandlers, + this.historyGroupingPolicy = neverMergePolicy, List? reactionPipeline, List? listeners, }) : requestHandlers = requestHandlers ?? [], @@ -73,6 +76,8 @@ class Editor implements RequestDispatcher { /// Service Locator that provides all resources that are relevant for document editing. late final EditContext context; + final HistoryGroupingPolicy historyGroupingPolicy; + /// Executes [EditCommand]s and collects a list of changes. late final _DocumentEditorCommandExecutor _commandExecutor; @@ -150,8 +155,7 @@ class Editor implements RequestDispatcher { print("STARTING TRANSACTION"); _isInTransaction = true; _activeChangeList = []; - // ignore: prefer_const_constructors - _transaction = CommandTransaction([]); + _transaction = CommandTransaction([], clock.now()); _onTransactionStart(); print("TRANSACTION WAS STARTED"); @@ -166,7 +170,18 @@ class Editor implements RequestDispatcher { } if (_transaction!.commands.isNotEmpty) { - _commandHistory.add(_transaction!); + if (_commandHistory.isNotEmpty && + historyGroupingPolicy.shouldMergeLatestTransaction(_transaction!, _commandHistory.last)) { + // Merge this transaction with the transaction just before it. This is used, for example, + // to group repeated text input into a single undoable transaction. + _commandHistory.last + ..commands.addAll(_transaction!.commands) + ..changes.addAll(_transaction!.changes) + ..lastChangeTime = clock.now(); + } else { + // Add this transaction onto the history stack. + _commandHistory.add(_transaction!); + } } // Now that an atomic set of changes have completed, let the reactions followup @@ -221,9 +236,13 @@ class Editor implements RequestDispatcher { if (command.historyBehavior == HistoryBehavior.undoable) { undoableCommands.add(command); + _transaction!.changes.addAll(List.from(commandChanges)); } } + // Log the time at the end of the actions in this transaction. + _transaction!.lastChangeTime = clock.now(); + if (undoableCommands.isNotEmpty) { _transaction!.commands.addAll(undoableCommands); } @@ -299,7 +318,7 @@ class Editor implements RequestDispatcher { // Second, start a new transaction and let reactions add separate changes. // ignore: prefer_const_constructors - _transaction = CommandTransaction([]); + _transaction = CommandTransaction([], clock.now()); for (final reaction in reactionPipeline) { // Note: we pass the active change list because reactions will cause more // changes to be added to that list. @@ -413,10 +432,134 @@ class Editor implements RequestDispatcher { } } +abstract interface class HistoryGroupingPolicy { + bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction); +} + +/// A [HistoryGroupingPolicy] that defers to a list of other individual policies. +/// +/// For most applications, an [Editor]'s transaction grouping policy should probably be +/// a [HistoryGroupingPolicyList] because most applications will want a number of different +/// heuristics that decide when to merge transactions. +/// +/// You can change the way the list of policies make a decision by way of the [choice] +/// property. You can either merge transactions when *any* of the policies want to merge +/// ([HistoryGroupingPolicyListChoice.anyPass]), or you can merge transactions when *all* +/// of the policies want to merge ([HistoryGroupingPolicyListChoice.allPass]). +class HistoryGroupingPolicyList implements HistoryGroupingPolicy { + const HistoryGroupingPolicyList( + this.policies, [ + this.choice = HistoryGroupingPolicyListChoice.anyPass, + ]); + + final List policies; + final HistoryGroupingPolicyListChoice choice; + + @override + bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) { + for (final policy in policies) { + switch (choice) { + case HistoryGroupingPolicyListChoice.anyPass: + if (policy.shouldMergeLatestTransaction(newTransaction, previousTransaction)) { + return true; + } + case HistoryGroupingPolicyListChoice.allPass: + if (!policy.shouldMergeLatestTransaction(newTransaction, previousTransaction)) { + return false; + } + } + } + + switch (choice) { + case HistoryGroupingPolicyListChoice.anyPass: + // None of the policies wanted to merge. Don't merge transactions. + return false; + case HistoryGroupingPolicyListChoice.allPass: + // All of the policies wanted to merge. Merge the transactions. + return true; + } + } +} + +enum HistoryGroupingPolicyListChoice { + /// Transactions will be combined if a single policy in the list wants to + /// combine them. + anyPass, + + /// Transactions will be combined only if every policy in the list wants to + /// combine them. + allPass; +} + +const neverMergePolicy = _NeverMergePolicy(); + +class _NeverMergePolicy implements HistoryGroupingPolicy { + const _NeverMergePolicy(); + + @override + bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) => false; +} + +/// A sane default configuration of a [MergeRapidTextInputPolicy]. +/// +/// To customize the merge time, create a [MergeRapidTextInputPolicy] with the desired merge time. +const mergeRapidTextInputPolicy = MergeRapidTextInputPolicy(); + +class MergeRapidTextInputPolicy implements HistoryGroupingPolicy { + const MergeRapidTextInputPolicy([this._maxMergeTime = const Duration(milliseconds: 100)]); + + final Duration _maxMergeTime; + + @override + bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) { + final newContentEvents = newTransaction.changes + .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) + .toList(); + if (newContentEvents.isEmpty) { + return false; + } + final newTextInsertionEvents = + newContentEvents.where((change) => change is DocumentEdit && change.change is TextInsertionEvent).toList(); + if (newTextInsertionEvents.length != newContentEvents.length) { + // There were 1+ new content changes that weren't text input. Don't merge transactions. + return false; + } + + // At this point we know that all new content changes were text input. + + // Check that the previous transaction was also all text input. + final previousContentEvents = previousTransaction.changes + .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) + .toList(); + if (previousContentEvents.isEmpty) { + return false; + } + final previousTextInsertionEvents = + previousContentEvents.where((change) => change is DocumentEdit && change.change is TextInsertionEvent).toList(); + if (previousTextInsertionEvents.length != previousContentEvents.length) { + // There were 1+ new content changes that weren't text input. Don't merge transactions. + return false; + } + + if (newTransaction.firstChangeTime.difference(previousTransaction.lastChangeTime!) > _maxMergeTime) { + // The text insertions were far enough apart in time that we don't want to merge them. + return false; + } + + // The new and previous transactions were entirely text input. They happend quickly. + // Merge them together. + return true; + } +} + class CommandTransaction { - const CommandTransaction(this.commands); + CommandTransaction(this.commands, this.firstChangeTime) : changes = []; final List commands; + final List changes; + + DateTime firstChangeTime; + DateTime? lastChangeTime; } /// An implementation of [CommandExecutor], designed for [Editor]. @@ -475,6 +618,17 @@ abstract mixin class Editable { /// [Editable] should notify interested parties of changes. void onTransactionEnd(List edits) {} + // /// Creates and returns a snapshot of this [Editable]'s current state, or `null` if + // /// this [Editable] is in its initial state. + // /// + // /// The returned snapshot must be a deep copy of any relevant information. It must not + // /// hold any references to data outside the snapshot. + // Object? createSnapshot(); + // + // /// Updates the state of this [Editable] to match the given [snapshot]. + // void restoreSnapshot(Object snapshot); + + /// Resets this [Editable] to its initial state. void reset() {} } diff --git a/super_editor/lib/src/default_editor/default_document_editor.dart b/super_editor/lib/src/default_editor/default_document_editor.dart index e0d106888..f3a11314e 100644 --- a/super_editor/lib/src/default_editor/default_document_editor.dart +++ b/super_editor/lib/src/default_editor/default_document_editor.dart @@ -14,6 +14,7 @@ import 'default_document_editor_reactions.dart'; Editor createDefaultDocumentEditor({ required MutableDocument document, required MutableDocumentComposer composer, + HistoryGroupingPolicy historyGroupingPolicy = mergeRapidTextInputPolicy, }) { final editor = Editor( editables: { @@ -21,6 +22,7 @@ Editor createDefaultDocumentEditor({ Editor.composerKey: composer, }, requestHandlers: List.from(defaultRequestHandlers), + historyGroupingPolicy: historyGroupingPolicy, reactionPipeline: List.from(defaultEditorReactions), ); diff --git a/super_editor/pubspec.yaml b/super_editor/pubspec.yaml index 1b4e04d03..4aafa9bfb 100644 --- a/super_editor/pubspec.yaml +++ b/super_editor/pubspec.yaml @@ -37,6 +37,7 @@ dependencies: flutter_test: sdk: flutter flutter_test_robots: 0.0.23 + clock: ^1.1.1 dependency_overrides: # # Override to local mono-repo path so devs can test this repo diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index 632ec0065..aa6a48a09 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -1,3 +1,4 @@ +import 'package:clock/clock.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_robots/flutter_test_robots.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; @@ -310,6 +311,71 @@ This is paragraph 3'''); expect(SuperEditorInspector.findTextInComponent(document.nodes[1].id).text, "This is paragraph 2"); expect(SuperEditorInspector.findTextInComponent(document.nodes[2].id).text, "This is paragraph 3"); }); + + group("transaction grouping >", () { + testWidgetsOnMac("merges rapidly inserted text", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .withHistoryGroupingPolicy(MergeRapidTextInputPolicy()) + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + // Type characters quickly. + await widgetTester.typeImeText("Hello"); + + // Ensure our typed text exists. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); + + // Undo the typing. + print("------------ UNDO -------------"); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the whole word was undone. + expect(SuperEditorInspector.findTextInComponent("1").text, ""); + }); + + testWidgetsOnMac("separates text typed later", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .withHistoryGroupingPolicy(const MergeRapidTextInputPolicy()) + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 0)), () async { + // Type characters quickly. + await widgetTester.typeImeText("Hel"); + }); + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 150)), () async { + // Type characters quickly. + await widgetTester.typeImeText("lo "); + }); + + // Wait a bit. + await widgetTester.pump(const Duration(seconds: 3)); + + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 3, 0)), () async { + // Type characters quickly. + await widgetTester.typeImeText("World!"); + }); + + // Ensure our typed text exists. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello World!"); + + // Undo the typing. + print("------------ UNDO -------------"); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the text typed later was removed, but the text typed earlier + // remains. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello "); + }); + }); }); } diff --git a/super_editor/test/super_editor/supereditor_test_tools.dart b/super_editor/test/super_editor/supereditor_test_tools.dart index 123cd24e0..0b00b4693 100644 --- a/super_editor/test/super_editor/supereditor_test_tools.dart +++ b/super_editor/test/super_editor/supereditor_test_tools.dart @@ -4,6 +4,7 @@ import 'dart:ui' as ui; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; +import 'package:super_editor/src/core/editor.dart'; import 'package:super_editor/super_editor.dart'; import 'package:super_editor/super_editor_test.dart'; import 'package:super_editor_markdown/super_editor_markdown.dart'; @@ -244,6 +245,11 @@ class TestSuperEditorConfigurator { return this; } + TestSuperEditorConfigurator withHistoryGroupingPolicy(HistoryGroupingPolicy policy) { + _config.historyGroupPolicy = policy; + return this; + } + /// Configures the [SuperEditor] to constrain its maxHeight and maxWidth using the given [size]. TestSuperEditorConfigurator withEditorSize(ui.Size? size) { _config.editorSize = size; @@ -409,7 +415,11 @@ class TestSuperEditorConfigurator { final layoutKey = _config.layoutKey!; final focusNode = _config.focusNode ?? FocusNode(); final composer = MutableDocumentComposer(initialSelection: _config.selection); - final editor = createDefaultDocumentEditor(document: _config.document, composer: composer) + final editor = createDefaultDocumentEditor( + document: _config.document, + composer: composer, + historyGroupingPolicy: _config.historyGroupPolicy ?? neverMergePolicy, + ) ..requestHandlers.insertAll(0, _config.addedRequestHandlers) ..reactionPipeline.insertAll(0, _config.addedReactions); @@ -645,6 +655,7 @@ class SuperEditorTestConfiguration { ScrollController? scrollController; bool insideCustomScrollView = false; DocumentGestureMode? gestureMode; + HistoryGroupingPolicy? historyGroupPolicy; TextInputSource? inputSource; SuperEditorSelectionPolicies? selectionPolicies; SelectionStyles? selectionStyles; From 7043540df5d2683dc00e75740b175b4b1cd14ca2 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 27 May 2024 17:30:42 -0700 Subject: [PATCH 15/19] WIP: Updating Super Editor demo to show transaction history visualization --- .../example/lib/main_super_editor.dart | 88 ++++++++++++++++--- 1 file changed, 74 insertions(+), 14 deletions(-) diff --git a/super_editor/example/lib/main_super_editor.dart b/super_editor/example/lib/main_super_editor.dart index 1f027746f..1336761fb 100644 --- a/super_editor/example/lib/main_super_editor.dart +++ b/super_editor/example/lib/main_super_editor.dart @@ -1,5 +1,4 @@ import 'package:example/demos/example_editor/_example_document.dart'; -import 'package:example/demos/example_editor/example_editor.dart'; import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:flutter_localizations/flutter_localizations.dart'; @@ -31,7 +30,7 @@ void main() { runApp( MaterialApp( home: Scaffold( - body: ExampleEditor(), + body: _Demo(), ), supportedLocales: const [ Locale('en', ''), @@ -48,8 +47,68 @@ void main() { ); } +class _Demo extends StatefulWidget { + const _Demo(); + + @override + State<_Demo> createState() => _DemoState(); +} + +class _DemoState extends State<_Demo> { + late MutableDocument _document; + late MutableDocumentComposer _composer; + late Editor _docEditor; + + @override + void initState() { + super.initState(); + _document = createInitialDocument(); + _composer = MutableDocumentComposer(); + _docEditor = createDefaultDocumentEditor(document: _document, composer: _composer); + } + + @override + void dispose() { + _composer.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return Row( + children: [ + Expanded( + child: _StandardEditor( + document: _document, + composer: _composer, + editor: _docEditor, + ), + ), + _buildToolbar(), + ], + ); + } + + Widget _buildToolbar() { + return Container( + width: 24, + height: double.infinity, + color: const Color(0xFF2F2F2F), + child: Column(), + ); + } +} + class _StandardEditor extends StatefulWidget { - const _StandardEditor(); + const _StandardEditor({ + required this.document, + required this.composer, + required this.editor, + }); + + final MutableDocument document; + final MutableDocumentComposer composer; + final Editor editor; @override State<_StandardEditor> createState() => _StandardEditorState(); @@ -58,10 +117,6 @@ class _StandardEditor extends StatefulWidget { class _StandardEditorState extends State<_StandardEditor> { final GlobalKey _docLayoutKey = GlobalKey(); - late MutableDocument _doc; - late MutableDocumentComposer _composer; - late Editor _docEditor; - late FocusNode _editorFocusNode; late ScrollController _scrollController; @@ -69,9 +124,6 @@ class _StandardEditorState extends State<_StandardEditor> { @override void initState() { super.initState(); - _doc = createInitialDocument(); - _composer = MutableDocumentComposer(); - _docEditor = createDefaultDocumentEditor(document: _doc, composer: _composer); _editorFocusNode = FocusNode(); _scrollController = ScrollController(); } @@ -80,19 +132,27 @@ class _StandardEditorState extends State<_StandardEditor> { void dispose() { _scrollController.dispose(); _editorFocusNode.dispose(); - _composer.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return SuperEditor( - editor: _docEditor, - document: _doc, - composer: _composer, + editor: widget.editor, + document: widget.document, + composer: widget.composer, focusNode: _editorFocusNode, scrollController: _scrollController, documentLayoutKey: _docLayoutKey, + stylesheet: defaultStylesheet.copyWith( + addRulesAfter: [ + taskStyles, + ], + ), + componentBuilders: [ + TaskComponentBuilder(widget.editor), + ...defaultComponentBuilders, + ], ); } } From 094cdb21dd826e96a8362c8079e75589d249aa0b Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 30 May 2024 13:46:55 -0700 Subject: [PATCH 16/19] Fixed merge issue --- .../supereditor_components_test.dart | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/super_editor/test/super_editor/supereditor_components_test.dart b/super_editor/test/super_editor/supereditor_components_test.dart index 2aa34d70f..95425f3f4 100644 --- a/super_editor/test/super_editor/supereditor_components_test.dart +++ b/super_editor/test/super_editor/supereditor_components_test.dart @@ -78,6 +78,21 @@ void main() { expect(RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1), SystemMouseCursors.basic); }); }); + + testWidgetsOnArbitraryDesktop('does not crash when if finds an unkown node type', (tester) async { + // Pump an editor with a node that has no corresponding component builder. + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [_UnkownNode(id: '1')], + ), + ) + .pump(); + + // Reaching this point means the editor did not crash because of the + // unkown node. + }); }); } @@ -185,3 +200,22 @@ class _FakeImageComponentBuilder implements ComponentBuilder { ); } } + +/// A [DocumentNode] without any content. +/// +/// Used to simulate an app-level node type that the editor +/// doesn't know about. +class _UnkownNode extends BlockNode with ChangeNotifier { + _UnkownNode({required this.id}); + + @override + final String id; + + @override + String? copyContent(NodeSelection selection) => ''; + + @override + _UnkownNode copy() { + return _UnkownNode(id: id); + } +} From cb5d71475db6376d93de67ac1865d238cbead63b Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 30 May 2024 17:14:26 -0700 Subject: [PATCH 17/19] Merge transactions that only contain selection and composing region changes - also show history in demo --- .../example/lib/main_super_editor.dart | 106 +++++++- super_editor/lib/src/core/document.dart | 29 ++- .../lib/src/core/document_composer.dart | 10 +- super_editor/lib/src/core/editor.dart | 229 ++++++++++++------ .../default_document_editor.dart | 2 +- .../lib/src/default_editor/paragraph.dart | 2 +- super_editor/lib/src/default_editor/text.dart | 10 + .../super_editor_undo_redo_test.dart | 146 +++++++---- 8 files changed, 402 insertions(+), 132 deletions(-) diff --git a/super_editor/example/lib/main_super_editor.dart b/super_editor/example/lib/main_super_editor.dart index 1336761fb..dcc7d6a08 100644 --- a/super_editor/example/lib/main_super_editor.dart +++ b/super_editor/example/lib/main_super_editor.dart @@ -90,11 +90,107 @@ class _DemoState extends State<_Demo> { } Widget _buildToolbar() { - return Container( - width: 24, - height: double.infinity, - color: const Color(0xFF2F2F2F), - child: Column(), + return Row( + mainAxisSize: MainAxisSize.min, + children: [ + _EditorHistoryPanel(editor: _docEditor), + Container( + width: 24, + height: double.infinity, + color: const Color(0xFF2F2F2F), + child: Column(), + ), + ], + ); + } +} + +class _EditorHistoryPanel extends StatefulWidget { + const _EditorHistoryPanel({ + required this.editor, + }); + + final Editor editor; + + @override + State<_EditorHistoryPanel> createState() => _EditorHistoryPanelState(); +} + +class _EditorHistoryPanelState extends State<_EditorHistoryPanel> { + final _scrollController = ScrollController(); + late EditListener _editListener; + + @override + void initState() { + super.initState(); + + _editListener = FunctionalEditListener(_onEditorChange); + widget.editor.addListener(_editListener); + } + + @override + void didUpdateWidget(_EditorHistoryPanel oldWidget) { + super.didUpdateWidget(oldWidget); + + if (widget.editor != oldWidget.editor) { + oldWidget.editor.removeListener(_editListener); + widget.editor.addListener(_editListener); + } + } + + @override + void dispose() { + _scrollController.dispose(); + widget.editor.removeListener(_editListener); + super.dispose(); + } + + void _onEditorChange(changes) { + setState(() { + // Build the latest list of changes. + }); + + // Always scroll to bottom of transaction list. + WidgetsBinding.instance.addPostFrameCallback((_) { + _scrollController.position.jumpTo(_scrollController.position.maxScrollExtent); + }); + } + + @override + Widget build(BuildContext context) { + return Theme( + data: ThemeData( + brightness: Brightness.dark, + ), + child: Container( + width: 300, + height: double.infinity, + color: const Color(0xFF333333), + child: SingleChildScrollView( + controller: _scrollController, + child: Padding( + padding: const EdgeInsets.symmetric(vertical: 24.0), + child: Column( + children: [ + for (final history in widget.editor.history) + ListTile( + title: Text("${history.changes.length} changes"), + titleTextStyle: TextStyle( + fontSize: 16, + ), + subtitle: Text("${history.changes.map((event) => event.describe()).join("\n")}"), + subtitleTextStyle: TextStyle( + color: Colors.white.withOpacity(0.5), + fontSize: 10, + height: 1.4, + ), + visualDensity: VisualDensity.compact, + ), + ], + ), + ), + ), + ), ); } } diff --git a/super_editor/lib/src/core/document.dart b/super_editor/lib/src/core/document.dart index deb206ab3..7dd29c122 100644 --- a/super_editor/lib/src/core/document.dart +++ b/super_editor/lib/src/core/document.dart @@ -99,16 +99,21 @@ class DocumentChangeLog { /// Marker interface for all document changes. abstract class DocumentChange { - // Marker interface + const DocumentChange(); + + /// Describes this change in a human-readable way. + String describe() => toString(); } /// A [DocumentChange] that impacts a single, specified [DocumentNode] with [nodeId]. -abstract class NodeDocumentChange implements DocumentChange { +abstract class NodeDocumentChange extends DocumentChange { + const NodeDocumentChange(); + String get nodeId; } /// A new [DocumentNode] was inserted in the [Document]. -class NodeInsertedEvent implements NodeDocumentChange { +class NodeInsertedEvent extends NodeDocumentChange { const NodeInsertedEvent(this.nodeId, this.insertionIndex); @override @@ -116,6 +121,9 @@ class NodeInsertedEvent implements NodeDocumentChange { final int insertionIndex; + @override + String describe() => "Inserted node: $nodeId"; + @override String toString() => "NodeInsertedEvent ($nodeId)"; @@ -132,7 +140,7 @@ class NodeInsertedEvent implements NodeDocumentChange { } /// A [DocumentNode] was moved to a new index. -class NodeMovedEvent implements NodeDocumentChange { +class NodeMovedEvent extends NodeDocumentChange { const NodeMovedEvent({ required this.nodeId, required this.from, @@ -144,6 +152,9 @@ class NodeMovedEvent implements NodeDocumentChange { final int from; final int to; + @override + String describe() => "Moved node ($nodeId): $from -> $to"; + @override String toString() => "NodeMovedEvent ($nodeId: $from -> $to)"; @@ -161,7 +172,7 @@ class NodeMovedEvent implements NodeDocumentChange { } /// A [DocumentNode] was removed from the [Document]. -class NodeRemovedEvent implements NodeDocumentChange { +class NodeRemovedEvent extends NodeDocumentChange { const NodeRemovedEvent(this.nodeId, this.removedNode); @override @@ -169,6 +180,9 @@ class NodeRemovedEvent implements NodeDocumentChange { final DocumentNode removedNode; + @override + String describe() => "Removed node: $nodeId"; + @override String toString() => "NodeRemovedEvent ($nodeId)"; @@ -185,12 +199,15 @@ class NodeRemovedEvent implements NodeDocumentChange { /// A node change might signify a content change, such as text changing in a paragraph, or /// it might signify a node changing its type of content, such as converting a paragraph /// to an image. -class NodeChangeEvent implements NodeDocumentChange { +class NodeChangeEvent extends NodeDocumentChange { const NodeChangeEvent(this.nodeId); @override final String nodeId; + @override + String describe() => "Changed node: $nodeId"; + @override String toString() => "NodeChangeEvent ($nodeId)"; diff --git a/super_editor/lib/src/core/document_composer.dart b/super_editor/lib/src/core/document_composer.dart index b567a20b4..5aed36b75 100644 --- a/super_editor/lib/src/core/document_composer.dart +++ b/super_editor/lib/src/core/document_composer.dart @@ -382,7 +382,7 @@ class ChangeSelectionCommand extends EditCommand { } /// A [EditEvent] that represents a change to the user's selection within a document. -class SelectionChangeEvent implements EditEvent { +class SelectionChangeEvent extends EditEvent { const SelectionChangeEvent({ required this.oldSelection, required this.newSelection, @@ -396,12 +396,15 @@ class SelectionChangeEvent implements EditEvent { // TODO: can we replace the concept of a `reason` with `changeType` final String reason; + @override + String describe() => "Selection - ${changeType.name}, $reason"; + @override String toString() => "[SelectionChangeEvent] - New selection: $newSelection, change type: $changeType"; } /// A [EditEvent] that represents a change to the user's composing region within a document. -class ComposingRegionChangeEvent implements EditEvent { +class ComposingRegionChangeEvent extends EditEvent { const ComposingRegionChangeEvent({ required this.oldComposingRegion, required this.newComposingRegion, @@ -410,6 +413,9 @@ class ComposingRegionChangeEvent implements EditEvent { final DocumentRange? oldComposingRegion; final DocumentRange? newComposingRegion; + @override + String describe() => "Composing - ${newComposingRegion ?? "empty"}"; + @override String toString() => "[ComposingRegionChangeEvent] - New composing region: $newComposingRegion"; } diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 89edd9c65..314ada496 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -76,13 +76,15 @@ class Editor implements RequestDispatcher { /// Service Locator that provides all resources that are relevant for document editing. late final EditContext context; + /// Policies that determine when a new transaction of changes should be combined with the + /// previous transaction, impacting what is undone by undo. final HistoryGroupingPolicy historyGroupingPolicy; /// Executes [EditCommand]s and collects a list of changes. late final _DocumentEditorCommandExecutor _commandExecutor; - final _commandHistory = []; - final _commandFuture = []; + final history = []; + final future = []; /// A pipeline of objects that receive change-lists from command execution /// and get the first opportunity to spawn additional commands before the @@ -170,17 +172,31 @@ class Editor implements RequestDispatcher { } if (_transaction!.commands.isNotEmpty) { - if (_commandHistory.isNotEmpty && - historyGroupingPolicy.shouldMergeLatestTransaction(_transaction!, _commandHistory.last)) { - // Merge this transaction with the transaction just before it. This is used, for example, - // to group repeated text input into a single undoable transaction. - _commandHistory.last - ..commands.addAll(_transaction!.commands) - ..changes.addAll(_transaction!.changes) - ..lastChangeTime = clock.now(); - } else { + if (history.isEmpty) { // Add this transaction onto the history stack. - _commandHistory.add(_transaction!); + history.add(_transaction!); + } else { + final mergeChoice = historyGroupingPolicy.shouldMergeLatestTransaction(_transaction!, history.last); + switch (mergeChoice) { + case TransactionMerge.noOpinion: + case TransactionMerge.doNotMerge: + // Don't alter the transaction history, just add the new transaction to the history. + history.add(_transaction!); + case TransactionMerge.mergeOnTop: + // Merge this transaction with the transaction just before it. This is used, for example, + // to group repeated text input into a single undoable transaction. + history.last + ..commands.addAll(_transaction!.commands) + ..changes.addAll(_transaction!.changes) + ..lastChangeTime = clock.now(); + case TransactionMerge.replacePrevious: + // Replaces the most recent transaction with the new transaction. This is used, for example, + // to throw away unnecessary history about selection and composing region changes, for which + // only the most recent value is relevant. + history + ..removeLast() + ..add(_transaction!); + } } } @@ -326,24 +342,24 @@ class Editor implements RequestDispatcher { } if (_transaction!.commands.isNotEmpty) { - _commandHistory.add(_transaction!); + history.add(_transaction!); } // FIXME: try removing this notify listeners // Notify all listeners that care about changes, but won't spawn additional requests. - _notifyListeners(); + _notifyListeners(List.from(_activeChangeList!, growable: false)); _isReacting = false; } void undo() { print("RUNNING UNDO"); - if (_commandHistory.isEmpty) { + if (history.isEmpty) { return; } print("History before undo:"); - for (final transaction in _commandHistory) { + for (final transaction in history) { print(" - transaction"); for (final command in transaction.commands) { print(" - ${command.runtimeType}: ${command.describe()}"); @@ -352,8 +368,8 @@ class Editor implements RequestDispatcher { print("---"); // Move the latest command from the history to the future. - final transactionToUndo = _commandHistory.removeLast(); - _commandFuture.add(transactionToUndo); + final transactionToUndo = history.removeLast(); + future.add(transactionToUndo); print("The commands being undone are:"); for (final command in transactionToUndo.commands) { print(" - ${command.runtimeType}: ${command.describe()}"); @@ -371,7 +387,7 @@ class Editor implements RequestDispatcher { // Replay all history except for the most recent command transaction. print("Replaying all command history except for the most recent transaction..."); final changeEvents = []; - for (final commandTransaction in _commandHistory) { + for (final commandTransaction in history) { for (final command in commandTransaction.commands) { print("Executing command: ${command.runtimeType}"); // We re-run the commands without tracking changes and without running reactions @@ -387,17 +403,18 @@ class Editor implements RequestDispatcher { editable.onTransactionEnd(changeEvents); } + _notifyListeners([]); print("DONE WITH UNDO"); } void redo() { print("RUNNING REDO"); - if (_commandFuture.isEmpty) { + if (future.isEmpty) { return; } print("Future transaction:"); - for (final command in _commandFuture.last.commands) { + for (final command in future.last.commands) { print(" - ${command.runtimeType}"); } @@ -406,13 +423,13 @@ class Editor implements RequestDispatcher { editable.onTransactionStart(); } - final commandTransaction = _commandFuture.removeLast(); + final commandTransaction = future.removeLast(); final edits = []; for (final command in commandTransaction.commands) { final commandEdits = _executeCommand(command); edits.addAll(commandEdits); } - _commandHistory.add(commandTransaction); + history.add(commandTransaction); print("DONE WITH REDO"); @@ -422,8 +439,7 @@ class Editor implements RequestDispatcher { } } - void _notifyListeners() { - final changeList = List.from(_activeChangeList!, growable: false); + void _notifyListeners(List changeList) { for (final listener in _changeListeners) { // Note: we pass a given copy of the change list, because listeners should // never cause additional editor changes. @@ -432,8 +448,56 @@ class Editor implements RequestDispatcher { } } +/// The merge policies that are used in the standard [Editor] construction. +const defaultMergePolicy = HistoryGroupingPolicyList( + [ + mergeRepeatSelectionChangesPolicy, + mergeRapidTextInputPolicy, + ], +); + abstract interface class HistoryGroupingPolicy { - bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction); + TransactionMerge shouldMergeLatestTransaction( + CommandTransaction newTransaction, + CommandTransaction previousTransaction, + ); +} + +enum TransactionMerge { + noOpinion, + doNotMerge, + mergeOnTop, + replacePrevious; + + static TransactionMerge chooseMoreConservative(TransactionMerge a, TransactionMerge b) { + if (a == b) { + // They're the same. It doesn't matter. + return a; + } + + switch (a) { + case TransactionMerge.noOpinion: + // No opinion has no particular conservative vs liberal metric. Return the other one. + return b; + case TransactionMerge.doNotMerge: + // Explicitly not merging is the most conservative. Return this one. + return a; + case TransactionMerge.mergeOnTop: + if (b == TransactionMerge.doNotMerge) { + // doNotMerge is the only more conservative choice than merging on top. + return b; + } + + return a; + case TransactionMerge.replacePrevious: + if (b == TransactionMerge.noOpinion) { + return a; + } + + // replacePrevious is the lease conservative option. The other one always wins. + return b; + } + } } /// A [HistoryGroupingPolicy] that defers to a list of other individual policies. @@ -447,48 +511,29 @@ abstract interface class HistoryGroupingPolicy { /// ([HistoryGroupingPolicyListChoice.anyPass]), or you can merge transactions when *all* /// of the policies want to merge ([HistoryGroupingPolicyListChoice.allPass]). class HistoryGroupingPolicyList implements HistoryGroupingPolicy { - const HistoryGroupingPolicyList( - this.policies, [ - this.choice = HistoryGroupingPolicyListChoice.anyPass, - ]); + const HistoryGroupingPolicyList(this.policies); final List policies; - final HistoryGroupingPolicyListChoice choice; @override - bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) { + TransactionMerge shouldMergeLatestTransaction( + CommandTransaction newTransaction, + CommandTransaction previousTransaction, + ) { + TransactionMerge mostConservativeChoice = TransactionMerge.noOpinion; + for (final policy in policies) { - switch (choice) { - case HistoryGroupingPolicyListChoice.anyPass: - if (policy.shouldMergeLatestTransaction(newTransaction, previousTransaction)) { - return true; - } - case HistoryGroupingPolicyListChoice.allPass: - if (!policy.shouldMergeLatestTransaction(newTransaction, previousTransaction)) { - return false; - } + final newMergeChoice = policy.shouldMergeLatestTransaction(newTransaction, previousTransaction); + if (newMergeChoice == TransactionMerge.doNotMerge) { + // A policy has explicitly requested not to merge. Don't merge. + return TransactionMerge.doNotMerge; } - } - switch (choice) { - case HistoryGroupingPolicyListChoice.anyPass: - // None of the policies wanted to merge. Don't merge transactions. - return false; - case HistoryGroupingPolicyListChoice.allPass: - // All of the policies wanted to merge. Merge the transactions. - return true; + mostConservativeChoice = TransactionMerge.chooseMoreConservative(mostConservativeChoice, newMergeChoice); } - } -} - -enum HistoryGroupingPolicyListChoice { - /// Transactions will be combined if a single policy in the list wants to - /// combine them. - anyPass, - /// Transactions will be combined only if every policy in the list wants to - /// combine them. - allPass; + return mostConservativeChoice; + } } const neverMergePolicy = _NeverMergePolicy(); @@ -497,7 +542,42 @@ class _NeverMergePolicy implements HistoryGroupingPolicy { const _NeverMergePolicy(); @override - bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) => false; + TransactionMerge shouldMergeLatestTransaction( + CommandTransaction newTransaction, CommandTransaction previousTransaction) => + TransactionMerge.doNotMerge; +} + +const mergeRepeatSelectionChangesPolicy = MergeRepeatSelectionChangesPolicy(); + +class MergeRepeatSelectionChangesPolicy implements HistoryGroupingPolicy { + const MergeRepeatSelectionChangesPolicy(); + + @override + TransactionMerge shouldMergeLatestTransaction( + CommandTransaction newTransaction, CommandTransaction previousTransaction) { + final isNewTransactionAllSelectionAndComposing = newTransaction.changes + .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) + .isEmpty; + + if (!isNewTransactionAllSelectionAndComposing) { + // The new transaction contains meaningful changes. Don't merge anything. + return TransactionMerge.noOpinion; + } + + final isPreviousTransactionAllSelectionAndComposing = previousTransaction.changes + .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) + .isEmpty; + + if (!isPreviousTransactionAllSelectionAndComposing) { + // The previous transaction contains meaningful changes. Add the new selection/composing + // changes on top so that they're undone with the previous content change. + return TransactionMerge.noOpinion; + } + + // The previous and new transactions are all selection and composing changes. We don't + // care about this history. Replaces the previous transaction with the new transaction. + return TransactionMerge.replacePrevious; + } } /// A sane default configuration of a [MergeRapidTextInputPolicy]. @@ -511,18 +591,19 @@ class MergeRapidTextInputPolicy implements HistoryGroupingPolicy { final Duration _maxMergeTime; @override - bool shouldMergeLatestTransaction(CommandTransaction newTransaction, CommandTransaction previousTransaction) { + TransactionMerge shouldMergeLatestTransaction( + CommandTransaction newTransaction, CommandTransaction previousTransaction) { final newContentEvents = newTransaction.changes .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) .toList(); if (newContentEvents.isEmpty) { - return false; + return TransactionMerge.noOpinion; } final newTextInsertionEvents = newContentEvents.where((change) => change is DocumentEdit && change.change is TextInsertionEvent).toList(); if (newTextInsertionEvents.length != newContentEvents.length) { // There were 1+ new content changes that weren't text input. Don't merge transactions. - return false; + return TransactionMerge.noOpinion; } // At this point we know that all new content changes were text input. @@ -532,23 +613,23 @@ class MergeRapidTextInputPolicy implements HistoryGroupingPolicy { .where((change) => change is! SelectionChangeEvent && change is! ComposingRegionChangeEvent) .toList(); if (previousContentEvents.isEmpty) { - return false; + return TransactionMerge.noOpinion; } final previousTextInsertionEvents = previousContentEvents.where((change) => change is DocumentEdit && change.change is TextInsertionEvent).toList(); if (previousTextInsertionEvents.length != previousContentEvents.length) { // There were 1+ new content changes that weren't text input. Don't merge transactions. - return false; + return TransactionMerge.noOpinion; } if (newTransaction.firstChangeTime.difference(previousTransaction.lastChangeTime!) > _maxMergeTime) { // The text insertions were far enough apart in time that we don't want to merge them. - return false; + return TransactionMerge.noOpinion; } - // The new and previous transactions were entirely text input. They happend quickly. + // The new and previous transactions were entirely text input. They happened quickly. // Merge them together. - return true; + return TransactionMerge.mergeOnTop; } } @@ -558,7 +639,7 @@ class CommandTransaction { final List commands; final List changes; - DateTime firstChangeTime; + final DateTime firstChangeTime; DateTime? lastChangeTime; } @@ -795,17 +876,23 @@ abstract class EditRequest { /// A change that took place within a [Editor]. abstract class EditEvent { - // Marker interface for all editor change events. + const EditEvent(); + + /// Describes this change in a human-readable way. + String describe() => toString(); } /// An [EditEvent] that altered a [Document]. /// /// The specific [Document] change is available in [change]. -class DocumentEdit implements EditEvent { +class DocumentEdit extends EditEvent { DocumentEdit(this.change); final DocumentChange change; + @override + String describe() => change.describe(); + @override String toString() => "DocumentEdit -> $change"; diff --git a/super_editor/lib/src/default_editor/default_document_editor.dart b/super_editor/lib/src/default_editor/default_document_editor.dart index f3a11314e..95b3144c2 100644 --- a/super_editor/lib/src/default_editor/default_document_editor.dart +++ b/super_editor/lib/src/default_editor/default_document_editor.dart @@ -14,7 +14,7 @@ import 'default_document_editor_reactions.dart'; Editor createDefaultDocumentEditor({ required MutableDocument document, required MutableDocumentComposer composer, - HistoryGroupingPolicy historyGroupingPolicy = mergeRapidTextInputPolicy, + HistoryGroupingPolicy historyGroupingPolicy = defaultMergePolicy, }) { final editor = Editor( editables: { diff --git a/super_editor/lib/src/default_editor/paragraph.dart b/super_editor/lib/src/default_editor/paragraph.dart index 986e3ee03..e1e701790 100644 --- a/super_editor/lib/src/default_editor/paragraph.dart +++ b/super_editor/lib/src/default_editor/paragraph.dart @@ -720,7 +720,7 @@ class DeleteUpstreamAtBeginningOfParagraphCommand extends EditCommand { } } -class Intention implements EditEvent { +class Intention extends EditEvent { Intention.start() : _isStart = true; Intention.end() : _isStart = false; diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 0791a6318..921b23ab3 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -1604,6 +1604,10 @@ class AttributionChangeEvent extends NodeChangeEvent { final SpanRange range; final Set attributions; + @override + String describe() => + "${change == AttributionChange.added ? "Added" : "Removed"} attributions ($nodeId) - ${range.start} -> ${range.end}: $attributions"; + @override String toString() => "AttributionChangeEvent ('$nodeId' - ${range.start} -> ${range.end} ($change): '$attributions')"; @@ -1751,6 +1755,9 @@ class TextInsertionEvent extends NodeChangeEvent { final int offset; final AttributedText text; + @override + String describe() => "Inserted text ($nodeId) @ $offset: '${text.text}'"; + @override String toString() => "TextInsertionEvent ('$nodeId' - $offset -> '${text.text}')"; @@ -1777,6 +1784,9 @@ class TextDeletedEvent extends NodeChangeEvent { final int offset; final AttributedText deletedText; + @override + String describe() => "Deleted text ($nodeId) @ $offset: ${deletedText.text}"; + @override String toString() => "TextDeletedEvent ('$nodeId' - $offset -> '${deletedText.text}')"; diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index aa6a48a09..d1f22d7ee 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -313,67 +313,121 @@ This is paragraph 3'''); }); group("transaction grouping >", () { - testWidgetsOnMac("merges rapidly inserted text", (widgetTester) async { - await widgetTester // - .createDocument() - .withSingleEmptyParagraph() - .withHistoryGroupingPolicy(MergeRapidTextInputPolicy()) - .pump(); + group("text merging >", () { + testWidgetsOnMac("merges rapidly inserted text", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .withHistoryGroupingPolicy(MergeRapidTextInputPolicy()) + .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await widgetTester.placeCaretInParagraph("1", 0); - // Type characters quickly. - await widgetTester.typeImeText("Hello"); + // Type characters quickly. + await widgetTester.typeImeText("Hello"); - // Ensure our typed text exists. - expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); + // Ensure our typed text exists. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); - // Undo the typing. - print("------------ UNDO -------------"); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + // Undo the typing. + print("------------ UNDO -------------"); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the whole word was undone. + expect(SuperEditorInspector.findTextInComponent("1").text, ""); + }); - // Ensure that the whole word was undone. - expect(SuperEditorInspector.findTextInComponent("1").text, ""); + testWidgetsOnMac("separates text typed later", (widgetTester) async { + await widgetTester // + .createDocument() + .withSingleEmptyParagraph() + .withHistoryGroupingPolicy(const MergeRapidTextInputPolicy()) + .pump(); + + await widgetTester.placeCaretInParagraph("1", 0); + + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 0)), () async { + // Type characters quickly. + await widgetTester.typeImeText("Hel"); + }); + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 150)), () async { + // Type characters quickly. + await widgetTester.typeImeText("lo "); + }); + + // Wait a bit. + await widgetTester.pump(const Duration(seconds: 3)); + + await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 3, 0)), () async { + // Type characters quickly. + await widgetTester.typeImeText("World!"); + }); + + // Ensure our typed text exists. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello World!"); + + // Undo the typing. + print("------------ UNDO -------------"); + await widgetTester.pressCmdZ(widgetTester); + await widgetTester.pump(); + + // Ensure that the text typed later was removed, but the text typed earlier + // remains. + expect(SuperEditorInspector.findTextInComponent("1").text, "Hello "); + }); }); - testWidgetsOnMac("separates text typed later", (widgetTester) async { - await widgetTester // - .createDocument() - .withSingleEmptyParagraph() - .withHistoryGroupingPolicy(const MergeRapidTextInputPolicy()) - .pump(); + group("selection and composing >", () { + testWidgetsOnMac("merges transactions with only selection and composing changes", (widgetTester) async { + final testContext = await widgetTester // + .createDocument() + .withLongDoc() + .withHistoryGroupingPolicy(defaultMergePolicy) + .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await widgetTester.placeCaretInParagraph("1", 0); - await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 0)), () async { - // Type characters quickly. - await widgetTester.typeImeText("Hel"); - }); - await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 150)), () async { - // Type characters quickly. - await widgetTester.typeImeText("lo "); - }); + // Ensure we start with one history transaction for placing the caret. + final editor = testContext.editor; + expect(editor.history.length, 1); - // Wait a bit. - await widgetTester.pump(const Duration(seconds: 3)); + // Move the selection around a few times. + await widgetTester.placeCaretInParagraph("2", 5); - await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 3, 0)), () async { - // Type characters quickly. - await widgetTester.typeImeText("World!"); + await widgetTester.placeCaretInParagraph("3", 3); + + await widgetTester.placeCaretInParagraph("4", 0); + + // Ensure that all selection changes were merged into the initial transaction. + expect(editor.history.length, 1); }); - // Ensure our typed text exists. - expect(SuperEditorInspector.findTextInComponent("1").text, "Hello World!"); + testWidgetsOnMac("does not merge transactions when non-selection changes are present", (widgetTester) async { + final testContext = await widgetTester // + .createDocument() + .withLongDoc() + .withHistoryGroupingPolicy(defaultMergePolicy) + .pump(); - // Undo the typing. - print("------------ UNDO -------------"); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await widgetTester.placeCaretInParagraph("1", 0); + + // Ensure we start with one history transaction for placing the caret. + final editor = testContext.editor; + expect(editor.history.length, 1); - // Ensure that the text typed later was removed, but the text typed earlier - // remains. - expect(SuperEditorInspector.findTextInComponent("1").text, "Hello "); + // Type a few characters. + await widgetTester.typeImeText("Hello "); + + // Move caret to start of paragraph. + await widgetTester.placeCaretInParagraph("1", 0); + + // Type a few more characters + await widgetTester.typeImeText("World "); + + // Ensure we have 4 transactions: selection, typing, selection, typing. + expect(editor.history.length, 4); + }); }); }); }); From 9e448bf722e83dbd4deeace7295097e8e96ffeeb Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 8 Jul 2024 15:33:36 -0700 Subject: [PATCH 18/19] Removed all print statements, fixed a couple failing tests in Markdown, resolved a number of lint warnings --- .../clones/quill/lib/editor/toolbar.dart | 8 +-- super_editor/example_docs/lib/toolbar.dart | 1 - super_editor/lib/src/core/editor.dart | 56 +++++++++---------- .../lib/src/default_editor/tasks.dart | 19 ++++++- .../lib/src/infrastructure/_logging.dart | 2 + .../platforms/ios/ios_document_controls.dart | 2 +- .../super_editor_undo_redo_test.dart | 4 +- .../supereditor_scrolling_test.dart | 4 +- .../super_editor/supereditor_test_tools.dart | 1 - .../text_entry/tagging/pattern_tags_test.dart | 2 +- ...tributed_text_editing_controller_test.dart | 12 ++-- .../lib/src/image_syntax.dart | 8 +-- .../src/markdown_inline_upstream_plugin.dart | 15 +++-- .../super_editor_markdown_pasting_test.dart | 2 +- 14 files changed, 78 insertions(+), 58 deletions(-) diff --git a/super_editor/clones/quill/lib/editor/toolbar.dart b/super_editor/clones/quill/lib/editor/toolbar.dart index d46df9fbe..3099430a2 100644 --- a/super_editor/clones/quill/lib/editor/toolbar.dart +++ b/super_editor/clones/quill/lib/editor/toolbar.dart @@ -185,7 +185,7 @@ class _FormattingToolbarState extends State { ]); // Clear the field and hide the URL bar - _urlController!.clear(); + _urlController!.clearTextAndSelection(); _urlFocusNode.unfocus(disposition: UnfocusDisposition.previouslyFocusedChild); _linkPopoverController.close(); setState(() {}); @@ -220,7 +220,7 @@ class _FormattingToolbarState extends State { } // Clear the field and hide the URL bar - _imageController!.clear(); + _imageController!.clearTextAndSelection(); _imageFocusNode.unfocus(disposition: UnfocusDisposition.previouslyFocusedChild); _imagePopoverController.close(); setState(() {}); @@ -535,7 +535,7 @@ class _FormattingToolbarState extends State { onPressed: () { setState(() { _urlFocusNode.unfocus(); - _urlController!.clear(); + _urlController!.clearTextAndSelection(); }); }, ), @@ -605,7 +605,7 @@ class _FormattingToolbarState extends State { onPressed: () { setState(() { _imageFocusNode.unfocus(); - _imageController!.clear(); + _imageController!.clearTextAndSelection(); }); }, ), diff --git a/super_editor/example_docs/lib/toolbar.dart b/super_editor/example_docs/lib/toolbar.dart index 9698c148d..d7faed25f 100644 --- a/super_editor/example_docs/lib/toolbar.dart +++ b/super_editor/example_docs/lib/toolbar.dart @@ -1,6 +1,5 @@ import 'dart:math'; -import 'package:example_docs/editor.dart'; import 'package:example_docs/infrastructure/icon_selector.dart'; import 'package:example_docs/infrastructure/color_selector.dart'; import 'package:example_docs/infrastructure/text_item_selector.dart'; diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 314ada496..227057a68 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -154,13 +154,12 @@ class Editor implements RequestDispatcher { return; } - print("STARTING TRANSACTION"); + editorEditsLog.info("Starting transaction"); _isInTransaction = true; _activeChangeList = []; _transaction = CommandTransaction([], clock.now()); _onTransactionStart(); - print("TRANSACTION WAS STARTED"); } /// Ends a transaction that was started with a call to [startTransaction]. @@ -213,7 +212,7 @@ class Editor implements RequestDispatcher { // transaction. _onTransactionEnd(); - print("TRANSACTION WAS ENDED"); + editorEditsLog.info("Finished transaction"); } /// Executes the given [requests]. @@ -224,14 +223,13 @@ class Editor implements RequestDispatcher { void execute(List requests) { if (requests.isEmpty) { // No changes were requested. Don't waste time starting and ending transactions, etc. - print("WARNING: Tried to execute requests without providing any requests!"); - print(StackTrace.current); + editorEditsLog.warning("Tried to execute requests without providing any requests"); return; } - print("Executing requests:"); + editorEditsLog.finer("Executing requests:"); for (final request in requests) { - print(" - ${request.runtimeType}"); + editorEditsLog.finer(" - ${request.runtimeType}"); } if (_activeCommandCount == 0 && !_isInTransaction) { @@ -353,29 +351,29 @@ class Editor implements RequestDispatcher { } void undo() { - print("RUNNING UNDO"); + editorEditsLog.info("Running undo"); if (history.isEmpty) { return; } - print("History before undo:"); + editorEditsLog.finer("History before undo:"); for (final transaction in history) { - print(" - transaction"); + editorEditsLog.finer(" - transaction"); for (final command in transaction.commands) { - print(" - ${command.runtimeType}: ${command.describe()}"); + editorEditsLog.finer(" - ${command.runtimeType}: ${command.describe()}"); } } - print("---"); + editorEditsLog.finer("---"); // Move the latest command from the history to the future. final transactionToUndo = history.removeLast(); future.add(transactionToUndo); - print("The commands being undone are:"); + editorEditsLog.finer("The commands being undone are:"); for (final command in transactionToUndo.commands) { - print(" - ${command.runtimeType}: ${command.describe()}"); + editorEditsLog.finer(" - ${command.runtimeType}: ${command.describe()}"); } - print("Resetting all editables to their last checkpoint..."); + editorEditsLog.finer("Resetting all editables to their last checkpoint..."); for (final editable in context._resources.values) { // Don't let editables notify listeners during undo. editable.onTransactionStart(); @@ -385,11 +383,11 @@ class Editor implements RequestDispatcher { } // Replay all history except for the most recent command transaction. - print("Replaying all command history except for the most recent transaction..."); + editorEditsLog.finer("Replaying all command history except for the most recent transaction..."); final changeEvents = []; for (final commandTransaction in history) { for (final command in commandTransaction.commands) { - print("Executing command: ${command.runtimeType}"); + editorEditsLog.finer("Executing command: ${command.runtimeType}"); // We re-run the commands without tracking changes and without running reactions // because any relevant reactions should have run the first time around, and already // submitted their commands. @@ -398,24 +396,27 @@ class Editor implements RequestDispatcher { } } + editorEditsLog.info("Finished undo"); + + editorEditsLog.finer("Ending transaction on all editables"); for (final editable in context._resources.values) { // Let editables start notifying listeners again. editable.onTransactionEnd(changeEvents); } + // TODO: find out why this is needed. If it's not, remove it. _notifyListeners([]); - print("DONE WITH UNDO"); } void redo() { - print("RUNNING REDO"); + editorEditsLog.info("Running redo"); if (future.isEmpty) { return; } - print("Future transaction:"); + editorEditsLog.finer("Future transaction:"); for (final command in future.last.commands) { - print(" - ${command.runtimeType}"); + editorEditsLog.finer(" - ${command.runtimeType}"); } for (final editable in context._resources.values) { @@ -431,12 +432,16 @@ class Editor implements RequestDispatcher { } history.add(commandTransaction); - print("DONE WITH REDO"); + editorEditsLog.info("Finished redo"); + editorEditsLog.finer("Ending transaction on all editables"); for (final editable in context._resources.values) { // Let editables start notifying listeners again. editable.onTransactionEnd(edits); } + + // TODO: find out why this is needed. If it's not, remove it. + _notifyListeners([]); } void _notifyListeners(List changeList) { @@ -1240,19 +1245,12 @@ class MutableDocument implements Document, Editable { @override void reset() { - print("Resetting the MutableDocument"); - _nodes ..clear() ..addAll(_latestNodesSnapshot.map((node) => node.copy()).toList()); _refreshNodeIdCaches(); _didReset = true; - - for (final node in _nodes) { - print("$node"); - } - print(""); } /// Updates all the maps which use the node id as the key. diff --git a/super_editor/lib/src/default_editor/tasks.dart b/super_editor/lib/src/default_editor/tasks.dart index 6f0d4c749..e1744f80a 100644 --- a/super_editor/lib/src/default_editor/tasks.dart +++ b/super_editor/lib/src/default_editor/tasks.dart @@ -1,7 +1,24 @@ +import 'package:attributed_text/attributed_text.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'package:super_editor/src/core/document.dart'; +import 'package:super_editor/src/core/document_composer.dart'; +import 'package:super_editor/src/core/document_layout.dart'; +import 'package:super_editor/src/core/document_selection.dart'; +import 'package:super_editor/src/core/edit_context.dart'; +import 'package:super_editor/src/core/editor.dart'; +import 'package:super_editor/src/core/styles.dart'; import 'package:super_editor/src/default_editor/blocks/indentation.dart'; -import 'package:super_editor/super_editor.dart'; +import 'package:super_editor/src/default_editor/multi_node_editing.dart'; +import 'package:super_editor/src/default_editor/paragraph.dart'; +import 'package:super_editor/src/default_editor/text.dart'; +import 'package:super_editor/src/infrastructure/_logging.dart'; +import 'package:super_editor/src/infrastructure/attributed_text_styles.dart'; +import 'package:super_editor/src/infrastructure/composable_text.dart'; +import 'package:super_editor/src/infrastructure/keyboard.dart'; + +import 'attributions.dart'; +import 'layout_single_column/layout_single_column.dart'; /// This file includes everything needed to add the concept of a task /// to Super Editor. This includes: diff --git a/super_editor/lib/src/infrastructure/_logging.dart b/super_editor/lib/src/infrastructure/_logging.dart index 31a82776c..bc1092222 100644 --- a/super_editor/lib/src/infrastructure/_logging.dart +++ b/super_editor/lib/src/infrastructure/_logging.dart @@ -4,6 +4,7 @@ import 'package:logging/logging.dart' as logging; class LogNames { static const editor = 'editor'; + static const editorEdits = 'editor.edits'; static const editorPolicies = 'editor.policies'; static const editorScrolling = 'editor.scrolling'; static const editorGestures = 'editor.gestures'; @@ -48,6 +49,7 @@ class LogNames { } final editorLog = logging.Logger(LogNames.editor); +final editorEditsLog = logging.Logger(LogNames.editorEdits); final editorPoliciesLog = logging.Logger(LogNames.editorPolicies); final editorScrollingLog = logging.Logger(LogNames.editorScrolling); final editorGesturesLog = logging.Logger(LogNames.editorGestures); diff --git a/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart b/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart index 494a22174..afc2894b5 100644 --- a/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart +++ b/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart @@ -672,7 +672,7 @@ class IosControlsDocumentLayerState extends DocumentLayoutLayerState", () { testWidgetsOnAllPlatforms("user can delete pieces of tags", (tester) async { - final context = await _pumpTestEditor( + await _pumpTestEditor( tester, singleParagraphEmptyDoc(), ); diff --git a/super_editor/test/super_textfield/attributed_text_editing_controller_test.dart b/super_editor/test/super_textfield/attributed_text_editing_controller_test.dart index 89359c98f..f9b220988 100644 --- a/super_editor/test/super_textfield/attributed_text_editing_controller_test.dart +++ b/super_editor/test/super_textfield/attributed_text_editing_controller_test.dart @@ -808,8 +808,8 @@ void main() { int listenerNotifyCount = 0; final controller = AttributedTextEditingController( text: AttributedText('my text'), - selection: TextSelection.collapsed(offset: 7), - composingRegion: TextRange(start: 3, end: 7), + selection: const TextSelection.collapsed(offset: 7), + composingRegion: const TextRange(start: 3, end: 7), ) ..composingAttributions = { boldAttribution, @@ -837,8 +837,8 @@ void main() { // throw a compile error, at which time it will be safe to remove it. controller ..text = AttributedText('my text') - ..selection = TextSelection.collapsed(offset: 7) - ..composingRegion = TextRange(start: 3, end: 7) + ..selection = const TextSelection.collapsed(offset: 7) + ..composingRegion = const TextRange(start: 3, end: 7) ..composingAttributions = {boldAttribution}; listenerNotifyCount = 0; @@ -859,8 +859,8 @@ void main() { int listenerNotifyCount = 0; final controller = AttributedTextEditingController( text: AttributedText('my text'), - selection: TextSelection.collapsed(offset: 7), - composingRegion: TextRange(start: 3, end: 7), + selection: const TextSelection.collapsed(offset: 7), + composingRegion: const TextRange(start: 3, end: 7), ) ..composingAttributions = { boldAttribution, diff --git a/super_editor_markdown/lib/src/image_syntax.dart b/super_editor_markdown/lib/src/image_syntax.dart index acf9b6a24..3e4f4e1d6 100644 --- a/super_editor_markdown/lib/src/image_syntax.dart +++ b/super_editor_markdown/lib/src/image_syntax.dart @@ -31,11 +31,11 @@ class SuperEditorImageSyntax extends md.LinkSyntax { String? tag, required List Function() getChildren, }) { - var text = parser.source!.substring(opener.endPos, parser.pos); + var text = parser.source.substring(opener.endPos, parser.pos); // The current character is the `]` that closed the link text. Examine the // next character, to determine what type of link we might have (a '(' // means a possible inline link; otherwise a possible reference link). - if (parser.pos + 1 >= parser.source!.length) { + if (parser.pos + 1 >= parser.source.length) { // The `]` is at the end of the document, but this may still be a valid // shortcut reference link. return _tryCreateReferenceLink(parser, text, getChildren: getChildren); @@ -67,7 +67,7 @@ class SuperEditorImageSyntax extends md.LinkSyntax { parser.advanceBy(1); // At this point, we've matched `[...][`. Maybe a *full* reference link, // like `[foo][bar]` or a *collapsed* reference link, like `[foo][]`. - if (parser.pos + 1 < parser.source!.length && parser.charAt(parser.pos + 1) == AsciiTable.rightBracket) { + if (parser.pos + 1 < parser.source.length && parser.charAt(parser.pos + 1) == AsciiTable.rightBracket) { // That opening `[` is not actually part of the link. Maybe a // *shortcut* reference link (followed by a `[`). parser.advanceBy(1); @@ -100,7 +100,7 @@ class SuperEditorImageSyntax extends md.LinkSyntax { // Parse an optional width. final width = _tryParseNumber(parser); - final downstreamCharacter = parser.source!.substring(parser.pos, parser.pos + 1); + final downstreamCharacter = parser.source.substring(parser.pos, parser.pos + 1); if (downstreamCharacter.toLowerCase() != 'x') { // The image size must have a "x" between the width and height, but the input doesn't. Fizzle. return null; diff --git a/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart b/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart index b75871d10..bf95fb9df 100644 --- a/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart +++ b/super_editor_markdown/lib/src/markdown_inline_upstream_plugin.dart @@ -153,6 +153,10 @@ class MarkdownInlineUpstreamSyntaxReaction extends EditReaction { return const []; } + final newCaretPosition = DocumentPosition( + nodeId: editedNode.id, + nodePosition: TextNodePosition(offset: markdownRun.start + markdownRun.replacementText.length), + ); return [ // Delete the whole run of Markdown text, e.g., "**my bold**". DeleteContentRequest( @@ -179,14 +183,17 @@ class MarkdownInlineUpstreamSyntaxReaction extends EditReaction { // were removed. ChangeSelectionRequest( DocumentSelection.collapsed( - position: DocumentPosition( - nodeId: editedNode.id, - nodePosition: TextNodePosition(offset: markdownRun.start + markdownRun.replacementText.length), - ), + position: newCaretPosition, ), SelectionChangeType.alteredContent, SelectionReason.contentChange, ), + ChangeComposingRegionRequest( + DocumentRange( + start: newCaretPosition, + end: newCaretPosition, + ), + ), ]; } } diff --git a/super_editor_markdown/test/super_editor_markdown_pasting_test.dart b/super_editor_markdown/test/super_editor_markdown_pasting_test.dart index 8d1fbab97..ad1f4266e 100644 --- a/super_editor_markdown/test/super_editor_markdown_pasting_test.dart +++ b/super_editor_markdown/test/super_editor_markdown_pasting_test.dart @@ -353,7 +353,7 @@ Aenean mattis ante justo, quis sollicitudin metus interdum id.''', }); testWidgetsOnMac("can paste a link", (tester) async { - final (_, document, composer) = await _pumpSuperEditor( + final (_, document, _) = await _pumpSuperEditor( tester, deserializeMarkdownToDocument(""), ); From e6c51775351a42e9dad72f41c65544080c2def0a Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Mon, 8 Jul 2024 15:57:41 -0700 Subject: [PATCH 19/19] PR cleanup --- super_editor/lib/src/core/editor.dart | 50 ++--- .../document_ime/document_delta_editing.dart | 14 +- .../super_editor_undo_redo_test.dart | 176 +++++++++--------- 3 files changed, 119 insertions(+), 121 deletions(-) diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 227057a68..d117ad6ac 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -83,8 +83,11 @@ class Editor implements RequestDispatcher { /// Executes [EditCommand]s and collects a list of changes. late final _DocumentEditorCommandExecutor _commandExecutor; - final history = []; - final future = []; + List get history => List.from(_history); + final _history = []; + + List get future => List.from(_future); + final _future = []; /// A pipeline of objects that receive change-lists from command execution /// and get the first opportunity to spawn additional commands before the @@ -171,20 +174,20 @@ class Editor implements RequestDispatcher { } if (_transaction!.commands.isNotEmpty) { - if (history.isEmpty) { + if (_history.isEmpty) { // Add this transaction onto the history stack. - history.add(_transaction!); + _history.add(_transaction!); } else { - final mergeChoice = historyGroupingPolicy.shouldMergeLatestTransaction(_transaction!, history.last); + final mergeChoice = historyGroupingPolicy.shouldMergeLatestTransaction(_transaction!, _history.last); switch (mergeChoice) { case TransactionMerge.noOpinion: case TransactionMerge.doNotMerge: // Don't alter the transaction history, just add the new transaction to the history. - history.add(_transaction!); + _history.add(_transaction!); case TransactionMerge.mergeOnTop: // Merge this transaction with the transaction just before it. This is used, for example, // to group repeated text input into a single undoable transaction. - history.last + _history.last ..commands.addAll(_transaction!.commands) ..changes.addAll(_transaction!.changes) ..lastChangeTime = clock.now(); @@ -192,7 +195,7 @@ class Editor implements RequestDispatcher { // Replaces the most recent transaction with the new transaction. This is used, for example, // to throw away unnecessary history about selection and composing region changes, for which // only the most recent value is relevant. - history + _history ..removeLast() ..add(_transaction!); } @@ -340,7 +343,7 @@ class Editor implements RequestDispatcher { } if (_transaction!.commands.isNotEmpty) { - history.add(_transaction!); + _history.add(_transaction!); } // FIXME: try removing this notify listeners @@ -352,12 +355,12 @@ class Editor implements RequestDispatcher { void undo() { editorEditsLog.info("Running undo"); - if (history.isEmpty) { + if (_history.isEmpty) { return; } editorEditsLog.finer("History before undo:"); - for (final transaction in history) { + for (final transaction in _history) { editorEditsLog.finer(" - transaction"); for (final command in transaction.commands) { editorEditsLog.finer(" - ${command.runtimeType}: ${command.describe()}"); @@ -366,8 +369,8 @@ class Editor implements RequestDispatcher { editorEditsLog.finer("---"); // Move the latest command from the history to the future. - final transactionToUndo = history.removeLast(); - future.add(transactionToUndo); + final transactionToUndo = _history.removeLast(); + _future.add(transactionToUndo); editorEditsLog.finer("The commands being undone are:"); for (final command in transactionToUndo.commands) { editorEditsLog.finer(" - ${command.runtimeType}: ${command.describe()}"); @@ -385,7 +388,7 @@ class Editor implements RequestDispatcher { // Replay all history except for the most recent command transaction. editorEditsLog.finer("Replaying all command history except for the most recent transaction..."); final changeEvents = []; - for (final commandTransaction in history) { + for (final commandTransaction in _history) { for (final command in commandTransaction.commands) { editorEditsLog.finer("Executing command: ${command.runtimeType}"); // We re-run the commands without tracking changes and without running reactions @@ -410,12 +413,12 @@ class Editor implements RequestDispatcher { void redo() { editorEditsLog.info("Running redo"); - if (future.isEmpty) { + if (_future.isEmpty) { return; } editorEditsLog.finer("Future transaction:"); - for (final command in future.last.commands) { + for (final command in _future.last.commands) { editorEditsLog.finer(" - ${command.runtimeType}"); } @@ -424,13 +427,13 @@ class Editor implements RequestDispatcher { editable.onTransactionStart(); } - final commandTransaction = future.removeLast(); + final commandTransaction = _future.removeLast(); final edits = []; for (final command in commandTransaction.commands) { final commandEdits = _executeCommand(command); edits.addAll(commandEdits); } - history.add(commandTransaction); + _history.add(commandTransaction); editorEditsLog.info("Finished redo"); @@ -565,7 +568,8 @@ class MergeRepeatSelectionChangesPolicy implements HistoryGroupingPolicy { .isEmpty; if (!isNewTransactionAllSelectionAndComposing) { - // The new transaction contains meaningful changes. Don't merge anything. + // The new transaction contains meaningful changes. Let other policies decide + // what to do. return TransactionMerge.noOpinion; } @@ -576,7 +580,7 @@ class MergeRepeatSelectionChangesPolicy implements HistoryGroupingPolicy { if (!isPreviousTransactionAllSelectionAndComposing) { // The previous transaction contains meaningful changes. Add the new selection/composing // changes on top so that they're undone with the previous content change. - return TransactionMerge.noOpinion; + return TransactionMerge.mergeOnTop; } // The previous and new transactions are all selection and composing changes. We don't @@ -639,13 +643,15 @@ class MergeRapidTextInputPolicy implements HistoryGroupingPolicy { } class CommandTransaction { - CommandTransaction(this.commands, this.firstChangeTime) : changes = []; + CommandTransaction(this.commands, this.firstChangeTime) + : changes = [], + lastChangeTime = firstChangeTime; final List commands; final List changes; final DateTime firstChangeTime; - DateTime? lastChangeTime; + DateTime lastChangeTime; } /// An implementation of [CommandExecutor], designed for [Editor]. diff --git a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart index ff8792275..a37a6ea1d 100644 --- a/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart +++ b/super_editor/lib/src/default_editor/document_ime/document_delta_editing.dart @@ -282,17 +282,10 @@ class TextDeltasDocumentEditor { editorImeLog .fine("Updating the Document Composer's selection to place caret at insertion offset:\n$insertionSelection"); final selectionBeforeInsertion = selection.value; - // editor.execute([ - // ChangeSelectionRequest( - // insertionSelection, - // SelectionChangeType.placeCaret, - // SelectionReason.userInteraction, - // ), - // ]); editorImeLog.fine("Inserting the text at the Document Composer's selection"); final didInsert = _insertPlainText( - insertionSelection, + insertionSelection.extent, textInserted, ); editorImeLog.fine("Insertion successful? $didInsert"); @@ -310,10 +303,9 @@ class TextDeltasDocumentEditor { } bool _insertPlainText( - DocumentSelection insertionSelection, + DocumentPosition insertionPosition, String text, ) { - DocumentPosition insertionPosition = insertionSelection.extent; editorOpsLog.fine('Attempting to insert "$text" at position: $insertionPosition'); DocumentNode? insertionNode = document.getNodeById(insertionPosition.nodeId); @@ -342,7 +334,7 @@ class TextDeltasDocumentEditor { editorOpsLog.finer("Text before insertion: '${insertionNode.text.text}'"); editor.execute([ ChangeSelectionRequest( - insertionSelection, + DocumentSelection.collapsed(position: insertionPosition), SelectionChangeType.placeCaret, SelectionReason.userInteraction, ), diff --git a/super_editor/test/super_editor/super_editor_undo_redo_test.dart b/super_editor/test/super_editor/super_editor_undo_redo_test.dart index 600e73fbe..e7b55e9da 100644 --- a/super_editor/test/super_editor/super_editor_undo_redo_test.dart +++ b/super_editor/test/super_editor/super_editor_undo_redo_test.dart @@ -11,7 +11,7 @@ import 'supereditor_test_tools.dart'; void main() { group("Super Editor > undo redo >", () { group("text insertion >", () { - testWidgets("insert a word", (widgetTester) async { + testWidgets("insert a word", (tester) async { final document = deserializeMarkdownToDocument("Hello world"); final composer = MutableDocumentComposer(); final editor = createDefaultDocumentEditor(document: document, composer: composer); @@ -81,16 +81,16 @@ void main() { ); }); - testWidgetsOnMac("type by character", (widgetTester) async { - await widgetTester // + testWidgetsOnMac("type by character", (tester) async { + await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Type characters. - await widgetTester.typeImeText("Hello"); + await tester.typeImeText("Hello"); expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); expect( @@ -104,50 +104,50 @@ void main() { ); // --- Undo character insertions --- - await widgetTester.pressCmdZ(widgetTester); + await tester.pressCmdZ(tester); _expectDocumentWithCaret("Hell", "1", 4); - await widgetTester.pressCmdZ(widgetTester); + await tester.pressCmdZ(tester); _expectDocumentWithCaret("Hel", "1", 3); - await widgetTester.pressCmdZ(widgetTester); + await tester.pressCmdZ(tester); _expectDocumentWithCaret("He", "1", 2); - await widgetTester.pressCmdZ(widgetTester); + await tester.pressCmdZ(tester); _expectDocumentWithCaret("H", "1", 1); - await widgetTester.pressCmdZ(widgetTester); + await tester.pressCmdZ(tester); _expectDocumentWithCaret("", "1", 0); //----- Redo Changes ---- - await widgetTester.pressCmdShiftZ(widgetTester); + await tester.pressCmdShiftZ(tester); _expectDocumentWithCaret("H", "1", 1); - await widgetTester.pressCmdShiftZ(widgetTester); + await tester.pressCmdShiftZ(tester); _expectDocumentWithCaret("He", "1", 2); - await widgetTester.pressCmdShiftZ(widgetTester); + await tester.pressCmdShiftZ(tester); _expectDocumentWithCaret("Hel", "1", 3); - await widgetTester.pressCmdShiftZ(widgetTester); + await tester.pressCmdShiftZ(tester); _expectDocumentWithCaret("Hell", "1", 4); - await widgetTester.pressCmdShiftZ(widgetTester); + await tester.pressCmdShiftZ(tester); _expectDocumentWithCaret("Hello", "1", 5); }); }); group("content conversions >", () { - testWidgetsOnMac("paragraph to header", (widgetTester) async { - final editContext = await widgetTester // + testWidgetsOnMac("paragraph to header", (tester) async { + final editContext = await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Type text that causes a conversion to a header node. - await widgetTester.typeImeText("# "); + await tester.typeImeText("# "); // Ensure that the paragraph is now a header. final document = editContext.document; @@ -155,8 +155,8 @@ void main() { expect(paragraph.metadata['blockType'], header1Attribution); expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, ""); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the header attribution is gone. paragraph = document.nodes.first as ParagraphNode; @@ -164,43 +164,43 @@ void main() { expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, "# "); }); - testWidgetsOnMac("dashes to em dash", (widgetTester) async { - await widgetTester // + testWidgetsOnMac("dashes to em dash", (tester) async { + await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); - // Type text that causes a conversion to a header node. - await widgetTester.typeImeText("--"); + // Type text that causes a conversion to an "em" dash. + await tester.typeImeText("--"); - // Ensure that the paragraph is now a header. + // Ensure that the double dashes are now an "em" dash. expect(SuperEditorInspector.findTextInComponent("1").text, "—"); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the em dash was reverted to the regular dashes. expect(SuperEditorInspector.findTextInComponent("1").text, "--"); // Continue typing. - await widgetTester.typeImeText(" "); + await tester.typeImeText(" "); // Ensure that the dashes weren't reconverted into an em dash. expect(SuperEditorInspector.findTextInComponent("1").text, "-- "); }); - testWidgetsOnMac("paragraph to list item", (widgetTester) async { - final editContext = await widgetTester // + testWidgetsOnMac("paragraph to list item", (tester) async { + final editContext = await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Type text that causes a conversion to a list item node. - await widgetTester.typeImeText("1. "); + await tester.typeImeText("1. "); // Ensure that the paragraph is now a list item. final document = editContext.document; @@ -208,8 +208,8 @@ void main() { expect(node, isA()); expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, ""); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the node is back to a paragraph. node = document.nodes.first as TextNode; @@ -217,16 +217,16 @@ void main() { expect(SuperEditorInspector.findTextInComponent(document.nodes.first.id).text, "1. "); }); - testWidgetsOnMac("url to a link", (widgetTester) async { - await widgetTester // + testWidgetsOnMac("url to a link", (tester) async { + await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Type text that causes a conversion to a link. - await widgetTester.typeImeText("google.com "); + await tester.typeImeText("google.com "); // Ensure that the URL is now linkified. expect( @@ -240,8 +240,8 @@ void main() { }, ); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the URL is no longer linkified. expect( @@ -250,40 +250,40 @@ void main() { ); }); - testWidgetsOnMac("paragraph to horizontal rule", (widgetTester) async { - final editContext = await widgetTester // + testWidgetsOnMac("paragraph to horizontal rule", (tester) async { + final editContext = await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); - await widgetTester.typeImeText("--- "); + await tester.typeImeText("--- "); expect(editContext.document.nodes.first, isA()); - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); expect(editContext.document.nodes.first, isA()); expect(SuperEditorInspector.findTextInComponent(editContext.document.nodes.first.id).text, "—- "); }); }); - testWidgetsOnMac("pasted content", (widgetTester) async { - final editContext = await widgetTester // + testWidgetsOnMac("pasted content", (tester) async { + final editContext = await tester // .createDocument() .withSingleEmptyParagraph() .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Paste multiple nodes of content. - widgetTester.simulateClipboard(); - await widgetTester.setSimulatedClipboardContent(''' + tester.simulateClipboard(); + await tester.setSimulatedClipboardContent(''' This is paragraph 1 This is paragraph 2 This is paragraph 3'''); - await widgetTester.pressCmdV(); + await tester.pressCmdV(); // Ensure the pasted content was applied as expected. final document = editContext.document; @@ -293,8 +293,8 @@ This is paragraph 3'''); expect(SuperEditorInspector.findTextInComponent(document.nodes[2].id).text, "This is paragraph 3"); // Undo the paste. - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure we're back to a single empty paragraph. expect(document.nodes.length, 1); @@ -302,8 +302,8 @@ This is paragraph 3'''); // Redo the paste // TODO: remove WidgetTester as required argument to this robot method - await widgetTester.pressCmdShiftZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdShiftZ(tester); + await tester.pump(); // Ensure the pasted content was applied as expected. expect(document.nodes.length, 3); @@ -314,61 +314,61 @@ This is paragraph 3'''); group("transaction grouping >", () { group("text merging >", () { - testWidgetsOnMac("merges rapidly inserted text", (widgetTester) async { - await widgetTester // + testWidgetsOnMac("merges rapidly inserted text", (tester) async { + await tester // .createDocument() .withSingleEmptyParagraph() .withHistoryGroupingPolicy(const MergeRapidTextInputPolicy()) .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Type characters quickly. - await widgetTester.typeImeText("Hello"); + await tester.typeImeText("Hello"); // Ensure our typed text exists. expect(SuperEditorInspector.findTextInComponent("1").text, "Hello"); // Undo the typing. - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the whole word was undone. expect(SuperEditorInspector.findTextInComponent("1").text, ""); }); - testWidgetsOnMac("separates text typed later", (widgetTester) async { - await widgetTester // + testWidgetsOnMac("separates text typed later", (tester) async { + await tester // .createDocument() .withSingleEmptyParagraph() .withHistoryGroupingPolicy(const MergeRapidTextInputPolicy()) .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 0)), () async { // Type characters quickly. - await widgetTester.typeImeText("Hel"); + await tester.typeImeText("Hel"); }); await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 0, 150)), () async { // Type characters quickly. - await widgetTester.typeImeText("lo "); + await tester.typeImeText("lo "); }); // Wait a bit. - await widgetTester.pump(const Duration(seconds: 3)); + await tester.pump(const Duration(seconds: 3)); await withClock(Clock(() => DateTime(2024, 05, 26, 12, 0, 3, 0)), () async { // Type characters quickly. - await widgetTester.typeImeText("World!"); + await tester.typeImeText("World!"); }); // Ensure our typed text exists. expect(SuperEditorInspector.findTextInComponent("1").text, "Hello World!"); // Undo the typing. - await widgetTester.pressCmdZ(widgetTester); - await widgetTester.pump(); + await tester.pressCmdZ(tester); + await tester.pump(); // Ensure that the text typed later was removed, but the text typed earlier // remains. @@ -377,54 +377,54 @@ This is paragraph 3'''); }); group("selection and composing >", () { - testWidgetsOnMac("merges transactions with only selection and composing changes", (widgetTester) async { - final testContext = await widgetTester // + testWidgetsOnMac("merges transactions with only selection and composing changes", (tester) async { + final testContext = await tester // .createDocument() .withLongDoc() .withHistoryGroupingPolicy(defaultMergePolicy) .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Ensure we start with one history transaction for placing the caret. final editor = testContext.editor; expect(editor.history.length, 1); // Move the selection around a few times. - await widgetTester.placeCaretInParagraph("2", 5); + await tester.placeCaretInParagraph("2", 5); - await widgetTester.placeCaretInParagraph("3", 3); + await tester.placeCaretInParagraph("3", 3); - await widgetTester.placeCaretInParagraph("4", 0); + await tester.placeCaretInParagraph("4", 0); // Ensure that all selection changes were merged into the initial transaction. expect(editor.history.length, 1); }); - testWidgetsOnMac("does not merge transactions when non-selection changes are present", (widgetTester) async { - final testContext = await widgetTester // + testWidgetsOnMac("does not merge transactions when non-selection changes are present", (tester) async { + final testContext = await tester // .createDocument() .withLongDoc() .withHistoryGroupingPolicy(defaultMergePolicy) .pump(); - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); // Ensure we start with one history transaction for placing the caret. final editor = testContext.editor; expect(editor.history.length, 1); // Type a few characters. - await widgetTester.typeImeText("Hello "); + await tester.typeImeText("Hello "); // Move caret to start of paragraph. - await widgetTester.placeCaretInParagraph("1", 0); + await tester.placeCaretInParagraph("1", 0); - // Type a few more characters - await widgetTester.typeImeText("World "); + // Type a few more characters. + await tester.typeImeText("World "); - // Ensure we have 4 transactions: selection, typing, selection, typing. - expect(editor.history.length, 4); + // Ensure we have 4 transactions: selection, typing+selection, typing. + expect(editor.history.length, 3); }); }); });