From 45a02950cb57ec33e19586cdce606beb40e257f3 Mon Sep 17 00:00:00 2001 From: Roland Bengtsson Date: Wed, 16 Sep 2026 21:31:35 +0300 Subject: [PATCH] fix: make Bold and Italic toggle off on a second press Wrapping leaves the selection on the word itself, inside the markers it just added. The toggle then asked whether the selected text starts and ends with the marker, which is false in exactly the state the first press produces, so a second press wrapped the wrapped text: Unidentified -> **Unidentified** -> ****Unidentified**** The toggle now also looks immediately outside the selection, and removes the markers it finds there. Markers that continue into more of the same character are left alone, because they belong to a longer run and so to a different construct. Pressing Italic on a word inside '**bold**' therefore adds its own markers, giving '***word***', rather than taking half the bold away. The fix is in the shared editor model, so both the VCL and FMX editors get it. Three tests, written first and confirmed failing with the reported strings. --- Source/Layout/Markdown4D.Editor.Model.pas | 44 ++++++++++++++++++++++ Tests/Markdown4D.Editor.Model.Tests.pas | 46 +++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/Source/Layout/Markdown4D.Editor.Model.pas b/Source/Layout/Markdown4D.Editor.Model.pas index f300c21..d02dada 100644 --- a/Source/Layout/Markdown4D.Editor.Model.pas +++ b/Source/Layout/Markdown4D.Editor.Model.pas @@ -88,6 +88,7 @@ TMarkdownEditorModel = class function CollapsedIndexOf(const HeaderOffset: Integer): Integer; function TryRegionAtHeader(const HeaderLine: Integer; out Region: TFoldRegion): Boolean; procedure WrapOrToggle(const Marker: string); + function MarkersSurroundSelection(const Start, Len: Integer; const Marker: string): Boolean; procedure InsertLink; procedure WrapCodeBlock; procedure ToggleHeading(const Level: Integer); @@ -1313,12 +1314,55 @@ procedure TMarkdownEditorModel.WrapOrToggle(const Marker: string); Exit; end; + // Wrapping leaves the selection on the word itself, inside the markers it just + // added, so pressing the same command again has to recognise them from there. + // Without this a second press wraps the wrapped text and doubles the markers. + if MarkersSurroundSelection(Start, Len, Marker) then + begin + ApplyReplace(Start - MarkerLen, Len + 2 * MarkerLen, Selected, False); + FAnchor := Start - MarkerLen; + FCaret := FAnchor + System.Length(Selected); + Exit; + end; + const Wrapped = Marker + Selected + Marker; ApplyReplace(Start, Len, Wrapped, False); FAnchor := Start + MarkerLen; FCaret := Start + MarkerLen + System.Length(Selected); end; +function TMarkdownEditorModel.MarkersSurroundSelection(const Start, Len: Integer; + const Marker: string): Boolean; +begin + Result := False; + + const MarkerLen = System.Length(Marker); + const LeadingStart = Start - MarkerLen + 1; + const TrailingStart = Start + Len + 1; + + if (LeadingStart < 1) or (TrailingStart + MarkerLen - 1 > System.Length(FText)) then + Exit; + + if Copy(FText, LeadingStart, MarkerLen) <> Marker then + Exit; + if Copy(FText, TrailingStart, MarkerLen) <> Marker then + Exit; + + // A marker that continues into more of the same character belongs to a longer + // run, and so to a different construct: the single asterisks either side of a + // word inside '**bold**' are the bold, not an italic to be taken away. + const MarkerChar = Marker[1]; + + if (LeadingStart > 1) and (FText[LeadingStart - 1] = MarkerChar) then + Exit; + + const AfterTrailing = TrailingStart + MarkerLen; + if (AfterTrailing <= System.Length(FText)) and (FText[AfterTrailing] = MarkerChar) then + Exit; + + Result := True; +end; + procedure TMarkdownEditorModel.InsertLink; begin const Start = SelectionStart; diff --git a/Tests/Markdown4D.Editor.Model.Tests.pas b/Tests/Markdown4D.Editor.Model.Tests.pas index 7d00b49..84fdc42 100644 --- a/Tests/Markdown4D.Editor.Model.Tests.pas +++ b/Tests/Markdown4D.Editor.Model.Tests.pas @@ -166,6 +166,15 @@ TMarkdownEditorModelTests = class [TestCase('On bold selection unwraps', '**Hello** world,9,Hello world')] procedure ExecuteBold_Various_TogglesBoldMarkers(const Text: string; const SelectionEnd: Integer; const Expected: string); + [Test] + procedure ExecuteBold_Twice_LeavesTheWordUnchanged; + + [Test] + procedure ExecuteBold_MarkersJustOutsideSelection_RemovesThem; + + [Test] + procedure ExecuteItalic_OnABoldWord_AddsItalicRatherThanEatingTheBold; + [Test] procedure ExecuteLink_InsertsPlaceholderWithCaretInUrl; @@ -764,6 +773,43 @@ procedure TMarkdownEditorModelTests.ExecuteBold_Various_TogglesBoldMarkers(const Assert.AreEqual(Expected, FModel.Text); end; +procedure TMarkdownEditorModelTests.ExecuteBold_Twice_LeavesTheWordUnchanged; +begin + // What a user actually does: select a word, press Bold, press Bold again. The + // first press leaves the selection on the word itself, inside the markers it + // just added, so the second press has to recognise them from there. + FModel.LoadText('Unidentified'); + FModel.SetSelection(0, 12); + + FModel.ExecuteCommand(TEditorCommand.Bold); + Assert.AreEqual('**Unidentified**', FModel.Text, 'after the first press'); + + FModel.ExecuteCommand(TEditorCommand.Bold); + Assert.AreEqual('Unidentified', FModel.Text, 'after the second press'); +end; + +procedure TMarkdownEditorModelTests.ExecuteBold_MarkersJustOutsideSelection_RemovesThem; +begin + FModel.LoadText('a **word** b'); + FModel.SetSelection(4, 4); + + FModel.ExecuteCommand(TEditorCommand.Bold); + + Assert.AreEqual('a word b', FModel.Text); +end; + +procedure TMarkdownEditorModelTests.ExecuteItalic_OnABoldWord_AddsItalicRatherThanEatingTheBold; +begin + // The single asterisks around the selection belong to a bold run, not to an + // italic one, so italic has to add its own rather than strip half the bold. + FModel.LoadText('**word**'); + FModel.SetSelection(2, 4); + + FModel.ExecuteCommand(TEditorCommand.Italic); + + Assert.AreEqual('***word***', FModel.Text); +end; + procedure TMarkdownEditorModelTests.ExecuteLink_InsertsPlaceholderWithCaretInUrl; begin FModel.LoadText('Hello');