From 79e8a5038f014b54ed96af8020a08e0e13b7d719 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 16:48:34 +0000 Subject: [PATCH 1/2] fix: record character formatting and hyperlink removal for undo Bold/italic/underline/strikethrough/sub/superscript/clear-formatting commands, Ctrl+B/I/U, the font family/size dropdowns and the text/highlight colour pickers called FormattingEngine/FontFormattingEngine directly, so none of them could be undone. ApplyStyleAction existed but only took a fixed transform (the toggles decide on/off from the current runs), restored the paragraph style on undo, and paired snapshots with block indices by position, restoring the wrong paragraph when a table sat inside the range. ApplyStyleAction now also wraps any formatting operation, snapshots each paragraph with its block index, restores only the inlines on undo, and merges split runs after applying. EditorContext.ApplyCharacterFormatting pushes one ApplyStyleAction for a non-empty selection (keeping the selection on the same text) and leaves a collapsed selection to the engines' pending style, which is not recorded. The commands, keyboard shortcuts and MAUI dropdowns/pickers all go through it. HyperlinkEngine.RemoveHyperlink now keeps the link's runs and their styles instead of one plain run, and RemoveHyperlinkCommand pushes a new RemoveHyperlinkAction that snapshots the paragraph so undo restores the link. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0197tHRWhTiagB1MTsDuPkoa --- CHANGELOG.md | 4 + docs/architecture.md | 2 +- docs/formatting.md | 10 +- docs/images-hyperlinks.md | 5 +- .../Commands/EditorContext.cs | 28 ++ .../Commands/FontCommands.cs | 28 +- .../Commands/FormattingCommands.cs | 14 +- .../Commands/HyperlinkCommands.cs | 13 +- .../Editing/UndoableActions.cs | 130 ++++++- .../Formatting/HyperlinkEngine.cs | 26 +- .../Input/EditorInputController.cs | 19 +- src/MintedTextEditor.Maui/MintedEditorView.cs | 29 +- .../CharacterFormatUndoTests.cs | 331 ++++++++++++++++++ 13 files changed, 586 insertions(+), 53 deletions(-) create mode 100644 tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index bfb4481..85f9554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed - The package version is set once in `Directory.Build.props` instead of `1.0.0` in each project, so local and CI pack runs produce the current version rather than 1.0.0. +### Fixed +- **Undo for character formatting**: bold, italic, underline, strikethrough, subscript, superscript and clear formatting (toolbar, Ctrl+B/I/U and commands), the font family and size dropdowns, and the text and highlight colour pickers now record one undo step each. Undo restores the previous runs with their exact styles and redo reapplies the change. Toggling with a collapsed caret still only sets the pending style and is not recorded. `ApplyStyleAction` gains a constructor that wraps any formatting operation, and its undo no longer resets the paragraph style or restores the wrong paragraph when a table lies inside the range. +- **Removing a hyperlink**: `HyperlinkEngine.RemoveHyperlink` keeps the link text's formatting instead of turning it into one plain run, and `RemoveHyperlinkCommand` (toolbar and context menu) is now undoable through the new `RemoveHyperlinkAction`. + --- ## [0.3.1] — 2026-10-21 diff --git a/docs/architecture.md b/docs/architecture.md index 94adea2..0b75493 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -80,7 +80,7 @@ MintedTextEditor is structured in three layers, each with a clear responsibility The core layer is **fully platform-independent** — it targets `net10.0` only and has no dependency on MAUI, SkiaSharp, or any platform SDK. This makes it testable with a mock `IDrawingContext` in a plain xUnit project (`tests/MintedTextEditor.Core.Tests`). The toolbar icon SVGs (Lucide, Heroicons, Material Symbols) are embedded resources in this assembly. -**Document Model**: `TextStyle` is an immutable class whose `With…` methods return new instances; `ParagraphStyle` is a mutable class. Undo works by recording actions that know how to reverse themselves, not by snapshotting styles. +**Document Model**: `TextStyle` is an immutable class whose `With…` methods return new instances; `ParagraphStyle` is a mutable class. Undo works by recording actions that know how to reverse themselves; actions that restyle or delete text (`ApplyStyleAction`, `DeleteRangeAction`, …) snapshot the runs of the paragraphs they touch and restore them on undo. **TextLayoutEngine** converts a `Document` into a `DocumentLayout` of `LayoutBlock`s (or `TableLayoutBlock`s), each with `LayoutLine`s made of positioned `LayoutRun`s. Wrapping never splits a grapheme cluster (emoji ZWJ sequences, flags, combining marks), and very long unbroken text wraps in linear time. `LayoutCache` keeps the layout per block and re-lays out only the blocks a change touches. `DocumentHitTester` maps a point to a `DocumentPosition`. diff --git a/docs/formatting.md b/docs/formatting.md index 904ee75..08b35e5 100644 --- a/docs/formatting.md +++ b/docs/formatting.md @@ -53,7 +53,7 @@ font.RemoveHighlightColor(doc, range); ### Programmatic (DocumentEditor + UndoManager) -The engines above change the document directly and are not recorded for undo. For undo-aware programmatic formatting, push an `ApplyStyleAction`; `UndoManager.Push` executes it and records it: +The engines above change the document directly and are not recorded for undo. The toolbar, the keyboard shortcuts and the character-formatting commands (`ToggleBoldCommand` … `ClearFormattingCommand`, `ApplyFontFamilyCommand`, `ApplyFontSizeCommand`, `ApplyTextColorCommand`, `ApplyHighlightColorCommand`) are: each records one undo step. From code, run one of those commands with an `EditorContext` (see [Commands & Events](commands-events.md)), or push an `ApplyStyleAction`; `UndoManager.Push` executes it and records it: ```csharp using MintedTextEditor.Core.Editing; @@ -63,8 +63,14 @@ var action = new ApplyStyleAction(doc, range, s => s .WithFontSize(14f)); editor.UndoManager.Push(action); // executes and records undo + +// Or wrap an engine call, e.g. a toggle that decides on/off from the current text +editor.UndoManager.Push(new ApplyStyleAction(doc, range, + r => editor.Formatting.ToggleBold(doc, r), "Bold")); ``` +Undo restores the runs exactly as they were, styles included. Push an `ApplyStyleAction` only for a non-empty range; with a collapsed caret, call the engine directly to set the pending style. + ### Querying Current Formatting ```csharp @@ -138,7 +144,7 @@ DocumentEditor.ApplyParagraphStyle(doc, range, s => s.Direction = TextDirection. ## Undo / Redo Integration -Typing, deleting, pasting, and the insert commands (hyperlink, image, table, rows and columns) are recorded by the `UndoManager`. The formatting engines are not; use `ApplyStyleAction` (above) when formatting must be undoable. +Typing, deleting, pasting, the insert commands (hyperlink, image, table, rows and columns), removing a hyperlink, and character formatting from the toolbar (buttons, font family and size dropdowns, text and highlight colour pickers), keyboard shortcuts and commands are recorded by the `UndoManager`, one step per action. Toggling a format with a collapsed caret only sets the pending style and is not recorded. Calling the formatting engines directly is not recorded either; use the commands or `ApplyStyleAction` (above) when formatting from code must be undoable. ```csharp editor.Undo(); // or editor.UndoManager.Undo() diff --git a/docs/images-hyperlinks.md b/docs/images-hyperlinks.md index 8445d05..abb0c8f 100644 --- a/docs/images-hyperlinks.md +++ b/docs/images-hyperlinks.md @@ -125,8 +125,11 @@ if (link is not null) ### Removing a Hyperlink +The link's text stays in place with its character formatting (bold, colours, …). From code, use the undoable `RemoveHyperlinkCommand`, which acts on the link at the caret, or call the engine directly, without undo: + ```csharp -HyperlinkEngine.RemoveHyperlink(editor.Document, editor.Selection.Active); // keeps the text +new RemoveHyperlinkCommand().Execute(ctx); // undoable +HyperlinkEngine.RemoveHyperlink(editor.Document, editor.Selection.Active); // not recorded for undo ``` The toolbar's Object menu also has Open Link and Remove Link. diff --git a/src/MintedTextEditor.Core/Commands/EditorContext.cs b/src/MintedTextEditor.Core/Commands/EditorContext.cs index 29e8352..ae55bef 100644 --- a/src/MintedTextEditor.Core/Commands/EditorContext.cs +++ b/src/MintedTextEditor.Core/Commands/EditorContext.cs @@ -1,3 +1,4 @@ +using MintedTextEditor.Core.Document; using MintedTextEditor.Core.Editing; using MintedTextEditor.Core.Formatting; @@ -43,4 +44,31 @@ public EditorContext( FontFormattingEngine = fontFormattingEngine; Clipboard = clipboard; } + + /// + /// Applies a character-formatting operation to the current selection as one undo step, + /// keeping the selection on the same text afterwards. + /// + /// + /// When the selection is empty, runs directly and nothing is + /// recorded: the formatting engines only update their pending style in that case. + /// Otherwise an is pushed to . + /// + /// Description of the undo step, e.g. "Bold". + /// + /// Formats the range it is given, e.g. r => ctx.FormattingEngine.ToggleBold(ctx.Document, r). + /// + public void ApplyCharacterFormatting(string description, Action apply) + { + var (anchor, active) = DocumentEditor.ApplyCharacterFormatting( + Document, Selection.Anchor, Selection.Active, + range => + { + if (range.IsEmpty) + apply(range); + else + UndoManager.Push(new ApplyStyleAction(Document, range, apply, description)); + }); + Selection.Set(anchor, active); + } } diff --git a/src/MintedTextEditor.Core/Commands/FontCommands.cs b/src/MintedTextEditor.Core/Commands/FontCommands.cs index 4326beb..53f7f95 100644 --- a/src/MintedTextEditor.Core/Commands/FontCommands.cs +++ b/src/MintedTextEditor.Core/Commands/FontCommands.cs @@ -7,7 +7,7 @@ namespace MintedTextEditor.Core.Commands; // ── Apply Font Family ───────────────────────────────────────────────────────── /// Applies to the selected text. -public sealed class ApplyFontFamilyCommand : IEditorCommand +public sealed class ApplyFontFamilyCommand : ICharacterFormattingCommand { /// public string Name => "ApplyFontFamily"; @@ -21,7 +21,8 @@ public sealed class ApplyFontFamilyCommand : IEditorCommand public void Execute(EditorContext ctx) { if (string.IsNullOrEmpty(FontFamily)) return; - ctx.FontFormattingEngine.ApplyFontFamily(ctx.Document, ctx.Selection.Range, FontFamily); + var family = FontFamily; + ctx.ApplyCharacterFormatting("Font", r => ctx.FontFormattingEngine.ApplyFontFamily(ctx.Document, r, family)); } /// @@ -31,7 +32,7 @@ public void Execute(EditorContext ctx) // ── Apply Font Size ─────────────────────────────────────────────────────────── /// Applies to the selected text. -public sealed class ApplyFontSizeCommand : IEditorCommand +public sealed class ApplyFontSizeCommand : ICharacterFormattingCommand { /// public string Name => "ApplyFontSize"; @@ -45,7 +46,8 @@ public sealed class ApplyFontSizeCommand : IEditorCommand public void Execute(EditorContext ctx) { if (FontSize <= 0) return; - ctx.FontFormattingEngine.ApplyFontSize(ctx.Document, ctx.Selection.Range, FontSize); + var size = FontSize; + ctx.ApplyCharacterFormatting("Font Size", r => ctx.FontFormattingEngine.ApplyFontSize(ctx.Document, r, size)); } /// @@ -55,7 +57,7 @@ public void Execute(EditorContext ctx) // ── Apply Text Color ────────────────────────────────────────────────────────── /// Applies as the text colour of the selected text. -public sealed class ApplyTextColorCommand : IEditorCommand +public sealed class ApplyTextColorCommand : ICharacterFormattingCommand { /// public string Name => "ApplyTextColor"; @@ -66,8 +68,11 @@ public sealed class ApplyTextColorCommand : IEditorCommand public EditorColor Color { get; set; } = EditorColor.Black; /// - public void Execute(EditorContext ctx) => - ctx.FontFormattingEngine.ApplyTextColor(ctx.Document, ctx.Selection.Range, Color); + public void Execute(EditorContext ctx) + { + var color = Color; + ctx.ApplyCharacterFormatting("Text Color", r => ctx.FontFormattingEngine.ApplyTextColor(ctx.Document, r, color)); + } /// public bool CanExecute(EditorContext ctx) => true; @@ -76,7 +81,7 @@ public void Execute(EditorContext ctx) => // ── Apply Highlight Color ───────────────────────────────────────────────────── /// Applies as the highlight colour of the selected text. -public sealed class ApplyHighlightColorCommand : IEditorCommand +public sealed class ApplyHighlightColorCommand : ICharacterFormattingCommand { /// public string Name => "ApplyHighlightColor"; @@ -87,8 +92,11 @@ public sealed class ApplyHighlightColorCommand : IEditorCommand public EditorColor Color { get; set; } = EditorColor.Yellow; /// - public void Execute(EditorContext ctx) => - ctx.FontFormattingEngine.ApplyHighlightColor(ctx.Document, ctx.Selection.Range, Color); + public void Execute(EditorContext ctx) + { + var color = Color; + ctx.ApplyCharacterFormatting("Highlight Color", r => ctx.FontFormattingEngine.ApplyHighlightColor(ctx.Document, r, color)); + } /// public bool CanExecute(EditorContext ctx) => true; diff --git a/src/MintedTextEditor.Core/Commands/FormattingCommands.cs b/src/MintedTextEditor.Core/Commands/FormattingCommands.cs index 2d8fe1e..5aeb2ac 100644 --- a/src/MintedTextEditor.Core/Commands/FormattingCommands.cs +++ b/src/MintedTextEditor.Core/Commands/FormattingCommands.cs @@ -15,7 +15,7 @@ public sealed class ToggleBoldCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleBold(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Bold", r => ctx.FormattingEngine.ToggleBold(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -33,7 +33,7 @@ public sealed class ToggleItalicCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleItalic(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Italic", r => ctx.FormattingEngine.ToggleItalic(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -51,7 +51,7 @@ public sealed class ToggleUnderlineCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleUnderline(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Underline", r => ctx.FormattingEngine.ToggleUnderline(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -69,7 +69,7 @@ public sealed class ToggleStrikethroughCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleStrikethrough(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Strikethrough", r => ctx.FormattingEngine.ToggleStrikethrough(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -87,7 +87,7 @@ public sealed class ToggleSubscriptCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleSubscript(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Subscript", r => ctx.FormattingEngine.ToggleSubscript(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -105,7 +105,7 @@ public sealed class ToggleSuperscriptCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ToggleSuperscript(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Superscript", r => ctx.FormattingEngine.ToggleSuperscript(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; @@ -123,7 +123,7 @@ public sealed class ClearFormattingCommand : ICharacterFormattingCommand /// public void Execute(EditorContext ctx) => - ctx.FormattingEngine.ClearFormatting(ctx.Document, ctx.Selection.Range); + ctx.ApplyCharacterFormatting("Clear Formatting", r => ctx.FormattingEngine.ClearFormatting(ctx.Document, r)); /// public bool CanExecute(EditorContext ctx) => true; diff --git a/src/MintedTextEditor.Core/Commands/HyperlinkCommands.cs b/src/MintedTextEditor.Core/Commands/HyperlinkCommands.cs index 1340992..c8fab1e 100644 --- a/src/MintedTextEditor.Core/Commands/HyperlinkCommands.cs +++ b/src/MintedTextEditor.Core/Commands/HyperlinkCommands.cs @@ -40,7 +40,10 @@ public void Execute(EditorContext ctx) // ── Remove Hyperlink ────────────────────────────────────────────────────────── -/// Removes the hyperlink at the caret position. +/// +/// Removes the hyperlink at the caret position, keeping its text and formatting. +/// Recorded as one undo step. +/// public sealed class RemoveHyperlinkCommand : IEditorCommand { /// @@ -49,8 +52,12 @@ public sealed class RemoveHyperlinkCommand : IEditorCommand public string Description => "Removes the hyperlink at the current caret position."; /// - public void Execute(EditorContext ctx) => - HyperlinkEngine.RemoveHyperlink(ctx.Document, ctx.Selection.Active); + public void Execute(EditorContext ctx) + { + if (HyperlinkEngine.GetHyperlinkAtPosition(ctx.Document, ctx.Selection.Active) is null) return; + var pos = ctx.UndoManager.Push(new RemoveHyperlinkAction(ctx.Document, ctx.Selection.Active)); + ctx.Selection.CollapseTo(pos); + } /// public bool CanExecute(EditorContext ctx) => diff --git a/src/MintedTextEditor.Core/Editing/UndoableActions.cs b/src/MintedTextEditor.Core/Editing/UndoableActions.cs index a516d3b..04e2ecf 100644 --- a/src/MintedTextEditor.Core/Editing/UndoableActions.cs +++ b/src/MintedTextEditor.Core/Editing/UndoableActions.cs @@ -67,6 +67,18 @@ internal static void Restore(Paragraph p, ParagraphSnapshot snapshot) AppendInline(p, snap); } + /// + /// Replaces only the inlines of with those in , + /// leaving its paragraph style as it is. + /// + internal static void RestoreInlines(Paragraph p, ParagraphSnapshot snapshot) + { + for (int i = p.Inlines.Count - 1; i >= 0; i--) + p.RemoveInline(i); + foreach (var snap in snapshot.Inlines) + AppendInline(p, snap); + } + /// Creates a new fully populated from a snapshot. internal static Paragraph CreateParagraph(ParagraphSnapshot snapshot) { @@ -345,30 +357,55 @@ public DocumentPosition Undo() // ── ApplyStyleAction ──────────────────────────────────────────────────────── /// -/// Records a style change. Snapshots all affected paragraph inlines before applying -/// so the originals can be restored on undo. +/// Records a character-formatting change over a range. Snapshots the inlines of every +/// affected paragraph before applying, so undo restores the original runs with their exact +/// styles. Redo applies the change again to the restored runs. /// +/// +/// Undo restores only the paragraphs' inlines and leaves their +/// alone, since character formatting never changes it. Runs split by the change are merged +/// back with same-styled neighbours after it is applied. +/// public sealed class ApplyStyleAction : IUndoableAction { private readonly Document.Document _document; private TextRange _range; private readonly ParagraphAnchor _startAnchor; private readonly ParagraphAnchor _endAnchor; - private readonly Func _transform; - private List _snapshots = []; + private readonly Action _apply; + private readonly List<(int BlockIndex, ParagraphSnapshot Snapshot)> _snapshots = []; /// Creates an action that applies to the style of the text in . public ApplyStyleAction(Document.Document document, TextRange range, Func transform) + : this(document, range, r => DocumentEditor.ApplyTextStyle(document, r, transform), "Apply Style") + { + } + + /// + /// Creates an action that runs over . Use it for + /// formatting operations that choose their own transform, such as the + /// toggles, which add a format unless the whole range already has it. + /// + /// The document to format. + /// The range to format. + /// + /// Formats the range it is given. It may split and restyle the text runs of the paragraphs the + /// range covers, but must not add, remove or move text or blocks. It runs again on redo, against + /// the same runs it saw the first time. + /// + /// Description of the undo step, e.g. "Bold". + public ApplyStyleAction(Document.Document document, TextRange range, Action apply, string description) { _document = document; _range = range; _startAnchor = new ParagraphAnchor(document, range.Start); _endAnchor = new ParagraphAnchor(document, range.End); - _transform = transform; + _apply = apply; + Description = description; } /// - public string Description => "Apply Style"; + public string Description { get; } /// public DocumentPosition Execute() @@ -378,23 +415,29 @@ public DocumentPosition Execute() for (int i = _range.Start.BlockIndex; i <= _range.End.BlockIndex && i < _document.Blocks.Count; i++) { if (_document.Blocks[i] is Paragraph p) - _snapshots.Add(SnapshotHelper.Capture(p)); + _snapshots.Add((i, SnapshotHelper.Capture(p))); } - DocumentEditor.ApplyTextStyle(_document, _range, _transform); - return _range.Start; + + _apply(_range); + + foreach (var (blockIndex, _) in _snapshots) + { + if (blockIndex < _document.Blocks.Count && _document.Blocks[blockIndex] is Paragraph p) + DocumentEditor.NormalizeInlines(p); + } + return _endAnchor.Resolve(_document); } /// public DocumentPosition Undo() { - for (int i = 0; i < _snapshots.Count; i++) + foreach (var (blockIndex, snapshot) in _snapshots) { - int blockIdx = _range.Start.BlockIndex + i; - if (blockIdx < _document.Blocks.Count && _document.Blocks[blockIdx] is Paragraph p) - SnapshotHelper.Restore(p, _snapshots[i]); + if (blockIndex < _document.Blocks.Count && _document.Blocks[blockIndex] is Paragraph p) + SnapshotHelper.RestoreInlines(p, snapshot); } _document.NotifyChanged(new DocumentChangedEventArgs(DocumentChangeType.StyleChanged, _range)); - return _range.Start; + return _endAnchor.Resolve(_document); } /// @@ -585,6 +628,65 @@ public DocumentPosition Undo() public bool MergeWith(IUndoableAction next) => false; } +// ── RemoveHyperlinkAction ─────────────────────────────────────────────────── + +/// +/// Records the removal of a hyperlink, whose text stays in place with its formatting. +/// Snapshots the paragraph's inlines so undo restores the link exactly. +/// +public sealed class RemoveHyperlinkAction : IUndoableAction +{ + private readonly Document.Document _document; + private readonly DocumentPosition _position; + private readonly ParagraphAnchor _anchor; + private ParagraphSnapshot? _snapshot; + + /// Creates an action that removes the hyperlink at . + public RemoveHyperlinkAction(Document.Document document, DocumentPosition position) + { + _document = document; + _position = position; + _anchor = new ParagraphAnchor(document, position); + } + + /// + public string Description => "Remove Hyperlink"; + + /// + public DocumentPosition Execute() + { + _snapshot = null; + if (HyperlinkEngine.GetHyperlinkAtPosition(_document, _position) is not null && + _document.Blocks[_position.BlockIndex] is Paragraph para) + { + _snapshot = SnapshotHelper.Capture(para); + HyperlinkEngine.RemoveHyperlink(_document, _position); + } + return _anchor.Resolve(_document); + } + + /// + public DocumentPosition Undo() + { + if (_snapshot is not null && + _position.BlockIndex < _document.Blocks.Count && + _document.Blocks[_position.BlockIndex] is Paragraph para) + { + SnapshotHelper.RestoreInlines(para, _snapshot); + _document.NotifyChanged(new DocumentChangedEventArgs( + DocumentChangeType.TextInserted, + new TextRange(_position, _position))); + } + return _anchor.Resolve(_document); + } + + /// + public DocumentPosition Redo() => Execute(); + + /// + public bool MergeWith(IUndoableAction next) => false; +} + // ── InsertTableAction ───────────────────────────────────────────────────── /// diff --git a/src/MintedTextEditor.Core/Formatting/HyperlinkEngine.cs b/src/MintedTextEditor.Core/Formatting/HyperlinkEngine.cs index 02b4ea0..e4dc2f7 100644 --- a/src/MintedTextEditor.Core/Formatting/HyperlinkEngine.cs +++ b/src/MintedTextEditor.Core/Formatting/HyperlinkEngine.cs @@ -68,9 +68,9 @@ public static void EditHyperlink( } /// - /// Removes the first that starts at - /// , preserving its plain text as a - /// in its place. + /// Removes the at , leaving + /// its text in place as s that keep their character formatting. + /// Not recorded for undo; use for that. /// public static void RemoveHyperlink(Document.Document doc, DocumentPosition position) { @@ -81,9 +81,23 @@ public static void RemoveHyperlink(Document.Document doc, DocumentPosition posit int idx = para.Inlines.IndexOf(hyperlink); if (idx < 0) return; - // Replace hyperlink with a plain TextRun containing the same text - para.Inlines.RemoveAt(idx); - para.Inlines.Insert(idx, new TextRun(hyperlink.GetText()) { Parent = para }); + // Replace the hyperlink with copies of its text runs, keeping each run's style + para.RemoveInline(idx); + int insertAt = idx; + foreach (var child in hyperlink.Children) + { + if (child is TextRun run) + { + if (run.Length > 0) + para.InsertInline(insertAt++, new TextRun(run.Text, run.Style)); + } + else + { + para.InsertInline(insertAt++, child); + } + } + if (para.Inlines.Count == 0) + para.AppendRun(string.Empty); doc.NotifyChanged(new DocumentChangedEventArgs( DocumentChangeType.TextDeleted, diff --git a/src/MintedTextEditor.Core/Input/EditorInputController.cs b/src/MintedTextEditor.Core/Input/EditorInputController.cs index 89e929e..7316f36 100644 --- a/src/MintedTextEditor.Core/Input/EditorInputController.cs +++ b/src/MintedTextEditor.Core/Input/EditorInputController.cs @@ -178,7 +178,7 @@ public bool HandleKeyDown(EditorKeyEventArgs e, Document.Document document, Docu case EditorKey.B when ctrl: if (FormattingEngine != null) { - ApplyFormattingWithSelectionRestore(document, r => FormattingEngine.ToggleBold(document, r)); + ApplyFormattingWithSelectionRestore(document, "Bold", r => FormattingEngine.ToggleBold(document, r)); return true; } return false; @@ -186,7 +186,7 @@ public bool HandleKeyDown(EditorKeyEventArgs e, Document.Document document, Docu case EditorKey.I when ctrl: if (FormattingEngine != null) { - ApplyFormattingWithSelectionRestore(document, r => FormattingEngine.ToggleItalic(document, r)); + ApplyFormattingWithSelectionRestore(document, "Italic", r => FormattingEngine.ToggleItalic(document, r)); return true; } return false; @@ -194,7 +194,7 @@ public bool HandleKeyDown(EditorKeyEventArgs e, Document.Document document, Docu case EditorKey.U when ctrl: if (FormattingEngine != null) { - ApplyFormattingWithSelectionRestore(document, r => FormattingEngine.ToggleUnderline(document, r)); + ApplyFormattingWithSelectionRestore(document, "Underline", r => FormattingEngine.ToggleUnderline(document, r)); return true; } return false; @@ -1076,11 +1076,20 @@ public void MovePageDown(Document.Document document, DocumentLayout layout, IDra /// run splits and merges. Uses /// so the selection is rebuilt from absolute character offsets after /// is called on all affected paragraphs. + /// A non-empty selection is recorded as one undo step; an empty one only changes the + /// pending style and is not recorded. /// - private void ApplyFormattingWithSelectionRestore(Document.Document document, Action applyOp) + private void ApplyFormattingWithSelectionRestore(Document.Document document, string description, Action applyOp) { var (newAnchor, newActive) = DocumentEditor.ApplyCharacterFormatting( - document, _selection.Anchor, _selection.Active, applyOp); + document, _selection.Anchor, _selection.Active, + range => + { + if (range.IsEmpty || UndoManager is null) + applyOp(range); + else + UndoManager.Push(new ApplyStyleAction(document, range, applyOp, description)); + }); _selection.Set(newAnchor, newActive); _caret.MoveTo(newActive); } diff --git a/src/MintedTextEditor.Maui/MintedEditorView.cs b/src/MintedTextEditor.Maui/MintedEditorView.cs index 71beeb4..0a17b7a 100644 --- a/src/MintedTextEditor.Maui/MintedEditorView.cs +++ b/src/MintedTextEditor.Maui/MintedEditorView.cs @@ -500,7 +500,11 @@ private void WireToolbarCallbacks(ToolbarDefinition def) dd.OnSelectionChanged = idx => { if ((uint)idx < (uint)dd.Items.Count) - _fontFormattingEngine.ApplyFontFamily(_document, _inputController.Selection.Range, dd.Items[idx] == "Default" ? "" : dd.Items[idx]); + { + var family = dd.Items[idx] == "Default" ? "" : dd.Items[idx]; + ApplyToolbarCharacterFormatting("Font", + r => _fontFormattingEngine.ApplyFontFamily(_document, r, family)); + } InvalidateCanvas(); }; break; @@ -512,7 +516,10 @@ private void WireToolbarCallbacks(ToolbarDefinition def) dd.OnSelectionChanged = idx => { if ((uint)idx < (uint)dd.Items.Count && float.TryParse(dd.Items[idx], out float sz)) - _fontFormattingEngine.ApplyFontSize(_document, _inputController.Selection.Range, sz); + { + ApplyToolbarCharacterFormatting("Font Size", + r => _fontFormattingEngine.ApplyFontSize(_document, r, sz)); + } InvalidateCanvas(); }; break; @@ -643,7 +650,8 @@ private void WireToolbarCallbacks(ToolbarDefinition def) case "text-color": cp.OnColorSelected = color => { - _fontFormattingEngine.ApplyTextColor(_document, _inputController.Selection.Range, color); + ApplyToolbarCharacterFormatting("Text Color", + r => _fontFormattingEngine.ApplyTextColor(_document, r, color)); InvalidateCanvas(); }; break; @@ -651,7 +659,8 @@ private void WireToolbarCallbacks(ToolbarDefinition def) case "highlight-color": cp.OnColorSelected = color => { - _fontFormattingEngine.ApplyHighlightColor(_document, _inputController.Selection.Range, color); + ApplyToolbarCharacterFormatting("Highlight Color", + r => _fontFormattingEngine.ApplyHighlightColor(_document, r, color)); InvalidateCanvas(); }; break; @@ -660,6 +669,18 @@ private void WireToolbarCallbacks(ToolbarDefinition def) } } + /// + /// Applies a toolbar dropdown or colour-picker format to the selection as one undo step + /// (or to the pending style when the selection is empty), keeping the selection on the same text. + /// + private void ApplyToolbarCharacterFormatting(string description, Action apply) + { + GetEditorContext().ApplyCharacterFormatting(description, apply); + _caret.MoveTo(_inputController.Selection.Active); + RaiseSelectionChanged(); + NotifyImeSelectionChanged(); + } + private void ExecuteToolbarCommand(IEditorCommand command) { var ctx = GetEditorContext(); diff --git a/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs b/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs new file mode 100644 index 0000000..7e6e03f --- /dev/null +++ b/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs @@ -0,0 +1,331 @@ +using MintedTextEditor.Core.Commands; +using MintedTextEditor.Core.Document; +using MintedTextEditor.Core.Editing; +using MintedTextEditor.Core.Formatting; +using MintedTextEditor.Core.Input; +using MintedTextEditor.Core.Layout; +using MintedTextEditor.Core.Rendering; + +namespace MintedTextEditor.Core.Tests; + +/// +/// Character-formatting commands, keyboard shortcuts and hyperlink removal must each record +/// exactly one undo step that restores the previous runs with their exact styles. +/// +public class CharacterFormatUndoTests +{ + private static readonly TextStyle Bold = TextStyle.Default.WithBold(true); + private static readonly TextStyle Italic = TextStyle.Default.WithItalic(true).WithFontSize(18); + + // ── Helpers ─────────────────────────────────────────────────────────────── + + private static Paragraph Runs(params (string Text, TextStyle Style)[] runs) + { + var p = new Paragraph(); + foreach (var (text, style) in runs) + p.AppendRun(text, style); + return p; + } + + /// "Hello " (bold), "big " (plain), "world" (italic, 18pt). + private static EditorDocument MixedDoc() => + new([Runs(("Hello ", Bold), ("big ", TextStyle.Default), ("world", Italic))]); + + /// Flattens the document into comparable (text, style) entries per block. + private static List> Dump(EditorDocument doc) => + doc.Blocks.Select(b => b switch + { + Paragraph p => p.Inlines.Select(i => i switch + { + TextRun r => (object)(r.Text, r.Style), + HyperlinkInline h => (h.Url, h.Title, + string.Join("|", h.Children.OfType().Select(c => $"{c.Text}:{c.Style.IsBold}:{c.Style.IsItalic}"))), + _ => i.GetType().Name, + }).ToList(), + _ => new List { b.GetType().Name }, + }).ToList(); + + private static EditorContext Ctx(EditorDocument doc, DocumentPosition anchor, DocumentPosition active, UndoManager undo) + { + var sel = new Selection(); + sel.Set(anchor, active); + return new EditorContext(doc, sel, undo, new FormattingEngine(), new FontFormattingEngine()); + } + + private static int UndoDepth(UndoManager undo) + { + int n = 0; + while (undo.CanUndo) { undo.Undo(); n++; } + for (int i = 0; i < n; i++) undo.Redo(); + return n; + } + + public static TheoryData CommandNames => new() + { + "ToggleBold", "ToggleItalic", "ToggleUnderline", "ToggleStrikethrough", + "ToggleSubscript", "ToggleSuperscript", "ClearFormatting", + "ApplyFontFamily", "ApplyFontSize", "ApplyTextColor", "ApplyHighlightColor", + }; + + private static IEditorCommand Create(string name) => name switch + { + "ToggleBold" => new ToggleBoldCommand(), + "ToggleItalic" => new ToggleItalicCommand(), + "ToggleUnderline" => new ToggleUnderlineCommand(), + "ToggleStrikethrough" => new ToggleStrikethroughCommand(), + "ToggleSubscript" => new ToggleSubscriptCommand(), + "ToggleSuperscript" => new ToggleSuperscriptCommand(), + "ClearFormatting" => new ClearFormattingCommand(), + "ApplyFontFamily" => new ApplyFontFamilyCommand { FontFamily = "Georgia" }, + "ApplyFontSize" => new ApplyFontSizeCommand { FontSize = 30 }, + "ApplyTextColor" => new ApplyTextColorCommand { Color = EditorColor.Red }, + "ApplyHighlightColor" => new ApplyHighlightColorCommand { Color = EditorColor.Yellow }, + _ => throw new ArgumentOutOfRangeException(nameof(name)), + }; + + // ── Commands: one step, exact undo, redo ───────────────────────────────── + + [Theory] + [MemberData(nameof(CommandNames))] + public void Command_OverMixedRuns_RecordsOneStep_UndoRestoresExactRuns_RedoReapplies(string name) + { + var doc = MixedDoc(); + var undo = new UndoManager(); + // "llo big wor": from inside the bold run to inside the italic run + var ctx = Ctx(doc, new DocumentPosition(0, 0, 2), new DocumentPosition(0, 2, 3), undo); + var before = Dump(doc); + + Create(name).Execute(ctx); + var after = Dump(doc); + + Assert.NotEqual(before, after); + Assert.Equal(1, UndoDepth(undo)); + Assert.Equal("llo big wor", DocumentEditor.GetSelectedText(doc, ctx.Selection.Range)); + + var undoPos = undo.Undo(); + Assert.NotNull(undoPos); + Assert.Equal(before, Dump(doc)); + Assert.False(undo.CanUndo); + + undo.Redo(); + Assert.Equal(after, Dump(doc)); + } + + [Fact] + public void ToggleBold_OffOnFullyBoldText_UndoRestoresBold() + { + var doc = new EditorDocument([Runs(("Hello", Bold))]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 0), new DocumentPosition(0, 0, 5), undo); + + new ToggleBoldCommand().Execute(ctx); + Assert.False(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsBold); + + undo.Undo(); + var runs = ((Paragraph)doc.Blocks[0]).Inlines.Cast().ToList(); + Assert.Single(runs); + Assert.Equal(Bold, runs[0].Style); + + undo.Redo(); + Assert.All(((Paragraph)doc.Blocks[0]).Inlines.Cast(), r => Assert.False(r.Style.IsBold)); + } + + [Fact] + public void RepeatedToggleAndUndo_DoesNotFragmentRuns() + { + var doc = new EditorDocument([new Paragraph("Hello world")]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 0), new DocumentPosition(0, 0, 5), undo); + + new ToggleBoldCommand().Execute(ctx); + new ToggleBoldCommand().Execute(ctx); // selection still covers "Hello" + Assert.Single(((Paragraph)doc.Blocks[0]).Inlines); + Assert.Equal(2, UndoDepth(undo)); + + undo.Undo(); + Assert.Equal(2, ((Paragraph)doc.Blocks[0]).Inlines.Count); + Assert.True(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsBold); + undo.Undo(); + Assert.Single(((Paragraph)doc.Blocks[0]).Inlines); + Assert.False(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsBold); + } + + // ── Multi-paragraph selection ──────────────────────────────────────────── + + [Fact] + public void Command_AcrossParagraphsAndTable_UndoRestoresEveryParagraph() + { + var doc = new EditorDocument( + [ + Runs(("Alpha ", Bold), ("one", TextStyle.Default)), + new TableBlock(1, 1), + Runs(("Gamma", Italic)), + new Paragraph("Delta"), + ]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 2), new DocumentPosition(3, 0, 2), undo); + var before = Dump(doc); + + new ApplyTextColorCommand { Color = EditorColor.Red }.Execute(ctx); + var after = Dump(doc); + Assert.NotEqual(before, after); + Assert.Equal(EditorColor.Red, ((TextRun)((Paragraph)doc.Blocks[2]).Inlines[0]).Style.TextColor); + Assert.Equal(1, UndoDepth(undo)); + + undo.Undo(); + Assert.Equal(before, Dump(doc)); + + undo.Redo(); + Assert.Equal(after, Dump(doc)); + } + + [Fact] + public void Undo_LeavesParagraphStyleUntouched() + { + var doc = new EditorDocument([new Paragraph("Hello")]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 0), new DocumentPosition(0, 0, 5), undo); + + new ToggleItalicCommand().Execute(ctx); + // An unrelated paragraph change made after the formatting step + ((Paragraph)doc.Blocks[0]).Style.Alignment = TextAlignment.Center; + + undo.Undo(); + Assert.Equal(TextAlignment.Center, ((Paragraph)doc.Blocks[0]).Style.Alignment); + Assert.False(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsItalic); + } + + // ── Pending style (collapsed selection) ─────────────────────────────────── + + [Theory] + [MemberData(nameof(CommandNames))] + public void Command_WithCollapsedSelection_SetsPendingStyle_WithoutUndoStep(string name) + { + var doc = MixedDoc(); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, 1, 2); + var ctx = Ctx(doc, caret, caret, undo); + var before = Dump(doc); + + Create(name).Execute(ctx); + + Assert.False(undo.CanUndo); + Assert.Equal(before, Dump(doc)); + Assert.NotNull(ctx.FormattingEngine.PendingStyle ?? ctx.FontFormattingEngine.PendingStyle); + Assert.Equal(caret, ctx.Selection.Active); + } + + // ── Keyboard shortcuts ──────────────────────────────────────────────────── + + [Fact] + public void CtrlB_WithSelection_RecordsOneUndoStep() + { + var doc = new EditorDocument([new Paragraph("Hello")]); + var context = new MockDrawingContext(); + var layout = new TextLayoutEngine().Layout(doc, 800f, context); + var undo = new UndoManager(); + var ctrl = new EditorInputController(new Caret(), new CaretRenderer()) + { + FormattingEngine = new FormattingEngine(), + UndoManager = undo, + }; + + ctrl.HandleKeyDown(new EditorKeyEventArgs(EditorKey.A, modifiers: InputModifiers.Control), doc, layout, context); + ctrl.HandleKeyDown(new EditorKeyEventArgs(EditorKey.B, modifiers: InputModifiers.Control), doc, layout, context); + Assert.True(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsBold); + Assert.Equal(1, UndoDepth(undo)); + + ctrl.HandleKeyDown(new EditorKeyEventArgs(EditorKey.Z, modifiers: InputModifiers.Control), doc, layout, context); + Assert.False(((TextRun)((Paragraph)doc.Blocks[0]).Inlines[0]).Style.IsBold); + } + + [Fact] + public void CtrlB_WithCollapsedSelection_DoesNotRecordUndoStep() + { + var doc = new EditorDocument([new Paragraph("Hello")]); + var context = new MockDrawingContext(); + var layout = new TextLayoutEngine().Layout(doc, 800f, context); + var undo = new UndoManager(); + var fmt = new FormattingEngine(); + var ctrl = new EditorInputController(new Caret(), new CaretRenderer()) + { + FormattingEngine = fmt, + UndoManager = undo, + }; + + ctrl.HandleKeyDown(new EditorKeyEventArgs(EditorKey.B, modifiers: InputModifiers.Control), doc, layout, context); + + Assert.False(undo.CanUndo); + Assert.True(fmt.PendingStyle?.IsBold); + } + + // ── RemoveHyperlink ─────────────────────────────────────────────────────── + + private static (EditorDocument doc, int linkIndex) LinkDoc() + { + var para = Runs(("Click ", TextStyle.Default)); + var doc = new EditorDocument([para]); + var link = new HyperlinkInline("https://example.com", "Example"); + para.AddInline(link); + link.AddChild(new TextRun("bold", Bold)); + link.AddChild(new TextRun(" plain")); + para.AppendRun(" after"); + return (doc, 1); + } + + [Fact] + public void RemoveHyperlink_KeepsLinkTextFormatting() + { + var (doc, idx) = LinkDoc(); + + HyperlinkEngine.RemoveHyperlink(doc, new DocumentPosition(0, idx, 1)); + + var inlines = ((Paragraph)doc.Blocks[0]).Inlines; + Assert.DoesNotContain(inlines, i => i is HyperlinkInline); + Assert.Equal("Click bold plain after", doc.GetText()); + var boldRun = Assert.Single(inlines.OfType(), r => r.Text == "bold"); + Assert.Equal(Bold, boldRun.Style); + Assert.All(inlines, i => Assert.Same(doc.Blocks[0], i.Parent)); + } + + [Fact] + public void RemoveHyperlinkCommand_RecordsOneStep_UndoRestoresLink_RedoRemovesAgain() + { + var (doc, idx) = LinkDoc(); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, idx, 2); + var ctx = Ctx(doc, caret, caret, undo); + var before = Dump(doc); + + new RemoveHyperlinkCommand().Execute(ctx); + var after = Dump(doc); + + Assert.DoesNotContain(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + Assert.Equal(1, UndoDepth(undo)); + // Caret stays on the same character ("Click bo|ld") + Assert.Equal(8, DocumentEditor.GetAbsoluteCharOffset(doc, ctx.Selection.Active)); + + var pos = undo.Undo(); + Assert.Equal(before, Dump(doc)); + var link = Assert.IsType(((Paragraph)doc.Blocks[0]).Inlines[idx]); + Assert.Equal("https://example.com", link.Url); + Assert.Equal("Example", link.Title); + Assert.Equal(Bold, ((TextRun)link.Children[0]).Style); + Assert.Equal(8, DocumentEditor.GetAbsoluteCharOffset(doc, pos!.Value)); + + undo.Redo(); + Assert.Equal(after, Dump(doc)); + } + + [Fact] + public void RemoveHyperlinkCommand_WithoutLinkAtCaret_DoesNotRecordUndoStep() + { + var doc = new EditorDocument([new Paragraph("No link")]); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, 0, 2); + + new RemoveHyperlinkCommand().Execute(Ctx(doc, caret, caret, undo)); + + Assert.False(undo.CanUndo); + } +} From b10af8ab9c0ea1c98aa4ec843061de96ee93e668 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 16:58:27 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20keep?= =?UTF-8?q?=20inline=20identity=20and=20lose=20nothing=20in=20undo=20snaps?= =?UTF-8?q?hots?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SnapshotHelper rebuilt every inline from copied data on restore, so undo steps recorded earlier that hold the original objects (InsertHyperlinkAction, InsertImageAction) could no longer find them: insert a link, remove or format it, undo twice, and the link stayed. Snapshots also turned LineBreak into a "\n" text run and kept only TextRun children of hyperlinks. Adopt the identity-preserving SnapshotHelper from PR #28 (housekeeping/hyperlink-display-text) verbatim so the two PRs merge cleanly: snapshots keep each original inline (and paragraph) with its state, restore puts the same objects back, line breaks and other inline types are kept as themselves, and hyperlink children are captured recursively. The inline-only restore used by ApplyStyleAction and RemoveHyperlinkAction is now a small ApplyStyleAction.RestoreInlines wrapper outside SnapshotHelper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0197tHRWhTiagB1MTsDuPkoa --- CHANGELOG.md | 1 + .../Editing/UndoableActions.cs | 111 +++++++---- .../CharacterFormatUndoTests.cs | 184 +++++++++++++++++- 3 files changed, 256 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6434f32..744bf92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed - **Undo for character formatting**: bold, italic, underline, strikethrough, subscript, superscript and clear formatting (toolbar, Ctrl+B/I/U and commands), the font family and size dropdowns, and the text and highlight colour pickers now record one undo step each. Undo restores the previous runs with their exact styles and redo reapplies the change. Toggling with a collapsed caret still only sets the pending style and is not recorded. `ApplyStyleAction` gains a constructor that wraps any formatting operation, and its undo no longer resets the paragraph style or restores the wrong paragraph when a table lies inside the range. - **Removing a hyperlink**: `HyperlinkEngine.RemoveHyperlink` keeps the link text's formatting instead of turning it into one plain run, and `RemoveHyperlinkCommand` (toolbar and context menu) is now undoable through the new `RemoveHyperlinkAction`. +- **Undo restoring copies**: undoing a delete, paragraph merge, formatting change or link removal puts back the original runs, images, links and line breaks instead of copies, so undoing an earlier image or link insertion afterwards still removes it. Line breaks no longer come back as `"\n"` text, and images and line breaks inside links are no longer dropped. --- diff --git a/src/MintedTextEditor.Core/Editing/UndoableActions.cs b/src/MintedTextEditor.Core/Editing/UndoableActions.cs index 04e2ecf..de8ec9a 100644 --- a/src/MintedTextEditor.Core/Editing/UndoableActions.cs +++ b/src/MintedTextEditor.Core/Editing/UndoableActions.cs @@ -5,12 +5,17 @@ namespace MintedTextEditor.Core.Editing; // ── Shared snapshot helpers ───────────────────────────────────────────────── -internal abstract record InlineSnapshot; -internal record TextRunSnapshot(string Text, TextStyle Style) : InlineSnapshot; -internal record ImageInlineSnapshot(string Source, string AltText, float Width, float Height) : InlineSnapshot; -internal record HyperChildSnapshot(string Text, TextStyle Style); -internal record HyperlinkInlineSnapshot(string Url, string? Title, IReadOnlyList Children) : InlineSnapshot; -internal record ParagraphSnapshot(ParagraphStyle Style, IReadOnlyList Inlines); +// Snapshots keep a reference to each original object together with its mutable state at capture +// time. Restoring puts those same objects back (with that state) instead of creating copies, so +// other undo actions that hold an inline or paragraph reference (e.g. InsertImageAction) still +// find it after an undo. +internal abstract record InlineSnapshot(Inline Inline); +internal record TextRunSnapshot(TextRun Run, string Text, TextStyle Style) : InlineSnapshot(Run); +internal record ImageInlineSnapshot(ImageInline Image, string Source, string AltText, float Width, float Height) : InlineSnapshot(Image); +internal record HyperlinkInlineSnapshot(HyperlinkInline Link, string Url, string? Title, IReadOnlyList Children) : InlineSnapshot(Link); +// Inlines without mutable state of their own (LineBreak, and any other inline type). +internal record OpaqueInlineSnapshot(Inline Inline) : InlineSnapshot(Inline); +internal record ParagraphSnapshot(Paragraph Paragraph, ParagraphStyle Style, IReadOnlyList Inlines); /// /// A position stored as a character offset within its paragraph (or table-cell paragraph) @@ -42,69 +47,90 @@ public bool SameAs(ParagraphAnchor other) internal static class SnapshotHelper { internal static ParagraphSnapshot Capture(Paragraph p) => - new(p.Style.Clone(), p.Inlines.Select(CaptureInline).ToList()); + new(p, p.Style.Clone(), p.Inlines.Select(CaptureInline).ToList()); private static InlineSnapshot CaptureInline(Inline inline) => inline switch { - TextRun t => new TextRunSnapshot(t.Text, t.Style), - ImageInline i => new ImageInlineSnapshot(i.Source, i.AltText, i.Width, i.Height), - HyperlinkInline h => new HyperlinkInlineSnapshot(h.Url, h.Title, - h.Children.OfType().Select(c => new HyperChildSnapshot(c.Text, c.Style)).ToList()), - _ => new TextRunSnapshot(inline.GetText(), TextStyle.Default) + TextRun t => new TextRunSnapshot(t, t.Text, t.Style), + ImageInline i => new ImageInlineSnapshot(i, i.Source, i.AltText, i.Width, i.Height), + HyperlinkInline h => new HyperlinkInlineSnapshot(h, h.Url, h.Title, h.Children.Select(CaptureInline).ToList()), + _ => new OpaqueInlineSnapshot(inline) }; /// - /// Replaces the inlines (and style) of with the data from - /// , without changing 's identity or parent. + /// Puts the inlines (and style) recorded in back into + /// , without changing 's identity or parent. + /// The recorded inline objects themselves are reinserted with their recorded state. /// internal static void Restore(Paragraph p, ParagraphSnapshot snapshot) { - p.Style = snapshot.Style; + p.Style = snapshot.Style.Clone(); // Remove existing inlines from back to front to keep indices stable for (int i = p.Inlines.Count - 1; i >= 0; i--) p.RemoveInline(i); foreach (var snap in snapshot.Inlines) - AppendInline(p, snap); + p.AddInline(RestoreInline(p, snap)); } /// - /// Replaces only the inlines of with those in , - /// leaving its paragraph style as it is. + /// Returns the paragraph recorded in , restored to its recorded + /// content, for reinsertion into the document. /// - internal static void RestoreInlines(Paragraph p, ParagraphSnapshot snapshot) + internal static Paragraph CreateParagraph(ParagraphSnapshot snapshot) { - for (int i = p.Inlines.Count - 1; i >= 0; i--) - p.RemoveInline(i); - foreach (var snap in snapshot.Inlines) - AppendInline(p, snap); + Restore(snapshot.Paragraph, snapshot); + return snapshot.Paragraph; } - /// Creates a new fully populated from a snapshot. - internal static Paragraph CreateParagraph(ParagraphSnapshot snapshot) + /// + /// Restores the paragraphs recorded from consecutive blocks starting at . + /// A recorded paragraph still in the document is restored in place; one that was removed (merged + /// away) is reinserted at its original index. Returns whether any paragraph was reinserted, in + /// which case later blocks have moved and the caller must report a structural change. + /// + internal static bool RestoreBlocks(Document.Document document, int startBlock, IReadOnlyList snapshots) { - var p = new Paragraph { Style = snapshot.Style }; - foreach (var snap in snapshot.Inlines) - AppendInline(p, snap); - return p; + bool reinserted = false; + for (int i = 0; i < snapshots.Count; i++) + { + var p = snapshots[i].Paragraph; + if (document.Blocks.Contains(p)) + { + Restore(p, snapshots[i]); + } + else + { + document.InsertBlock(Math.Min(startBlock + i, document.Blocks.Count), CreateParagraph(snapshots[i])); + reinserted = true; + } + } + return reinserted; } - private static void AppendInline(Paragraph p, InlineSnapshot snap) + private static Inline RestoreInline(Paragraph p, InlineSnapshot snap) { switch (snap) { case TextRunSnapshot t: - p.AppendRun(t.Text, t.Style); + t.Run.Text = t.Text; + t.Run.Style = t.Style; break; case ImageInlineSnapshot i: - p.AddInline(new ImageInline(i.Source, i.AltText, i.Width, i.Height)); + i.Image.Source = i.Source; + i.Image.AltText = i.AltText; + i.Image.Width = i.Width; + i.Image.Height = i.Height; break; case HyperlinkInlineSnapshot h: - var link = new HyperlinkInline(h.Url, h.Title); - p.AddInline(link); // set link.Parent = p before adding children + h.Link.Url = h.Url; + h.Link.Title = h.Title; + h.Link.Children.Clear(); foreach (var c in h.Children) - link.AddChild(new TextRun(c.Text, c.Style)); + h.Link.Children.Add(RestoreInline(p, c)); break; } + snap.Inline.Parent = p; + return snap.Inline; } } @@ -434,12 +460,23 @@ public DocumentPosition Undo() foreach (var (blockIndex, snapshot) in _snapshots) { if (blockIndex < _document.Blocks.Count && _document.Blocks[blockIndex] is Paragraph p) - SnapshotHelper.RestoreInlines(p, snapshot); + RestoreInlines(p, snapshot); } _document.NotifyChanged(new DocumentChangedEventArgs(DocumentChangeType.StyleChanged, _range)); return _endAnchor.Resolve(_document); } + /// + /// Restores the inlines recorded in but keeps 's + /// current paragraph style, which inline-only changes never alter. + /// + internal static void RestoreInlines(Paragraph p, ParagraphSnapshot snapshot) + { + var style = p.Style; + SnapshotHelper.Restore(p, snapshot); + p.Style = style; + } + /// public DocumentPosition Redo() => Execute(); @@ -672,7 +709,7 @@ public DocumentPosition Undo() _position.BlockIndex < _document.Blocks.Count && _document.Blocks[_position.BlockIndex] is Paragraph para) { - SnapshotHelper.RestoreInlines(para, _snapshot); + ApplyStyleAction.RestoreInlines(para, _snapshot); _document.NotifyChanged(new DocumentChangedEventArgs( DocumentChangeType.TextInserted, new TextRange(_position, _position))); diff --git a/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs b/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs index 7e6e03f..f180dee 100644 --- a/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs +++ b/tests/MintedTextEditor.Core.Tests/CharacterFormatUndoTests.cs @@ -38,13 +38,21 @@ private static List> Dump(EditorDocument doc) => Paragraph p => p.Inlines.Select(i => i switch { TextRun r => (object)(r.Text, r.Style), - HyperlinkInline h => (h.Url, h.Title, - string.Join("|", h.Children.OfType().Select(c => $"{c.Text}:{c.Style.IsBold}:{c.Style.IsItalic}"))), - _ => i.GetType().Name, + _ => Describe(i), }).ToList(), _ => new List { b.GetType().Name }, }).ToList(); + /// Lossless text description of an inline, recursing into hyperlink children. + private static string Describe(Inline inline) => inline switch + { + TextRun r => $"T[{r.Text}|{r.Style.GetHashCode()}|{r.Style.IsBold}]", + ImageInline img => $"I[{img.Source}|{img.AltText}|{img.Width}x{img.Height}]", + LineBreak => "BR", + HyperlinkInline h => $"L[{h.Url}|{h.Title}|{string.Join(",", h.Children.Select(Describe))}]", + _ => inline.GetType().Name, + }; + private static EditorContext Ctx(EditorDocument doc, DocumentPosition anchor, DocumentPosition active, UndoManager undo) { var sel = new Selection(); @@ -328,4 +336,174 @@ public void RemoveHyperlinkCommand_WithoutLinkAtCaret_DoesNotRecordUndoStep() Assert.False(undo.CanUndo); } + + // ── Undo of earlier insertions after a snapshot restore ─────────────────── + // Snapshot restores must put the original inline objects back: earlier insert + // actions find what they inserted by reference when they are undone. + + private static void SelectRange(EditorContext ctx, int fromAbs, int toAbs) + { + var context = new DocumentPosition(0, 0, 0); + ctx.Selection.Set( + DocumentEditor.AbsoluteCharOffsetToPosition(ctx.Document, context, fromAbs), + DocumentEditor.AbsoluteCharOffsetToPosition(ctx.Document, context, toAbs)); + } + + [Fact] + public void InsertLink_RemoveLink_UndoTwice_RemovesTheLink() + { + var doc = new EditorDocument([new Paragraph("Click here now")]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 6), new DocumentPosition(0, 0, 10), undo); + + new InsertHyperlinkCommand { Url = "https://example.com" }.Execute(ctx); + var linkIndex = ((Paragraph)doc.Blocks[0]).Inlines.FindIndex(i => i is HyperlinkInline); + ctx.Selection.CollapseTo(new DocumentPosition(0, linkIndex, 1)); + new RemoveHyperlinkCommand().Execute(ctx); + + undo.Undo(); // link is back + Assert.Contains(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + undo.Undo(); // insertion undone + Assert.DoesNotContain(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + Assert.Equal("Click here now", doc.GetText()); + Assert.False(undo.CanUndo); + } + + [Fact] + public void InsertLink_FormatOverIt_UndoTwice_RemovesTheLink() + { + var doc = new EditorDocument([new Paragraph("Click here now")]); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 6), new DocumentPosition(0, 0, 10), undo); + + new InsertHyperlinkCommand { Url = "https://example.com" }.Execute(ctx); + SelectRange(ctx, 0, 14); + new ToggleBoldCommand().Execute(ctx); + + undo.Undo(); // bold undone; link still there + Assert.Contains(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + undo.Undo(); // insertion undone + Assert.DoesNotContain(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + Assert.Equal("Click here now", doc.GetText()); + } + + [Fact] + public void InsertImage_FormatOverIt_UndoTwice_RemovesTheImage() + { + var doc = new EditorDocument([new Paragraph("Before after")]); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, 0, 6); + var ctx = Ctx(doc, caret, caret, undo); + + new InsertImageCommand { Source = "pic.png", Width = 10, Height = 20 }.Execute(ctx); + SelectRange(ctx, 0, 13); + new ApplyTextColorCommand { Color = EditorColor.Red }.Execute(ctx); + + undo.Undo(); // colour undone; image still there + Assert.Single(((Paragraph)doc.Blocks[0]).Inlines.OfType()); + undo.Undo(); // insertion undone + Assert.Empty(((Paragraph)doc.Blocks[0]).Inlines.OfType()); + Assert.Equal("Before after", doc.GetText()); + } + + [Fact] + public void InsertImage_RemoveLinkInSameParagraph_UndoTwice_RemovesTheImage() + { + var (doc, _) = LinkDoc(); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, 0, 2); + var ctx = Ctx(doc, caret, caret, undo); + + new InsertImageCommand { Source = "pic.png" }.Execute(ctx); + var linkIndex = ((Paragraph)doc.Blocks[0]).Inlines.FindIndex(i => i is HyperlinkInline); + ctx.Selection.CollapseTo(new DocumentPosition(0, linkIndex, 1)); + new RemoveHyperlinkCommand().Execute(ctx); + + undo.Undo(); // link back + Assert.Contains(((Paragraph)doc.Blocks[0]).Inlines, i => i is HyperlinkInline); + undo.Undo(); // image insertion undone + Assert.Empty(((Paragraph)doc.Blocks[0]).Inlines.OfType()); + Assert.Equal("Click bold plain after", doc.GetText()); + } + + // ── Lossless snapshots ──────────────────────────────────────────────────── + + [Fact] + public void Format_AcrossLineBreak_UndoRestoresLineBreak_RedoKeepsIt() + { + var para = new Paragraph("first"); + var doc = new EditorDocument([para]); + var lineBreak = new LineBreak(); + para.AddInline(lineBreak); + para.AppendRun("second"); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 2), new DocumentPosition(0, 2, 3), undo); + var before = Dump(doc); + + new ToggleBoldCommand().Execute(ctx); + var after = Dump(doc); + Assert.Contains(para.Inlines, i => i is LineBreak); + + undo.Undo(); + Assert.Equal(before, Dump(doc)); + Assert.Same(lineBreak, para.Inlines[1]); + + undo.Redo(); + Assert.Equal(after, Dump(doc)); + Assert.Contains(para.Inlines, i => i is LineBreak); + } + + [Fact] + public void RemoveLink_WithNonTextChildren_UndoRestoresAllChildren_RedoKeepsThem() + { + var para = new Paragraph("See "); + var doc = new EditorDocument([para]); + var link = new HyperlinkInline("https://example.com", "Example"); + para.AddInline(link); + link.AddChild(new TextRun("logo", Bold)); + link.AddChild(new ImageInline("logo.png", "Logo", 16, 16)); + link.AddChild(new LineBreak()); + link.AddChild(new TextRun("caption", Italic)); + para.AppendRun(" end"); + var undo = new UndoManager(); + var caret = new DocumentPosition(0, 1, 1); + var ctx = Ctx(doc, caret, caret, undo); + var before = Dump(doc); + + new RemoveHyperlinkCommand().Execute(ctx); + var after = Dump(doc); + Assert.Contains(para.Inlines, i => i is ImageInline); + Assert.Contains(para.Inlines, i => i is LineBreak); + Assert.Equal("See logo\ncaption end", doc.GetText()); + + undo.Undo(); + Assert.Equal(before, Dump(doc)); + Assert.Same(link, para.Inlines[1]); + Assert.All(link.Children, c => Assert.Same(para, c.Parent)); + + undo.Redo(); + Assert.Equal(after, Dump(doc)); + } + + [Fact] + public void Format_OverLinkWithNonTextChildren_UndoKeepsChildren() + { + var para = new Paragraph("A "); + var doc = new EditorDocument([para]); + var link = new HyperlinkInline("https://example.com"); + para.AddInline(link); + link.AddChild(new TextRun("x", Bold)); + link.AddChild(new LineBreak()); + link.AddChild(new ImageInline("i.png")); + para.AppendRun(" B"); + var undo = new UndoManager(); + var ctx = Ctx(doc, new DocumentPosition(0, 0, 0), new DocumentPosition(0, 2, 2), undo); + var before = Dump(doc); + + new ToggleItalicCommand().Execute(ctx); + undo.Undo(); + + Assert.Equal(before, Dump(doc)); + Assert.Same(link, para.Inlines[1]); + } }