From 35ee965f4e4efd23d8f1deb674dc837f3f9bf3d8 Mon Sep 17 00:00:00 2001 From: niksedk Date: Sun, 23 Aug 2026 08:00:31 +0200 Subject: [PATCH] FixContinuationStyle: sanitize each paragraph once instead of twice The rule walks paragraph pairs and called ContinuationUtilities.SanitizeString on both p and pNext every iteration, so every paragraph was sanitized twice: once as pNext's text, then again as p's text one iteration later. SanitizeString runs four regex replaces per call. Carry the sanitized "next" value forward instead. The carry is captured before the Arabic conversion (which the next iteration reapplies) and dropped whenever the loop writes pNext.Text, so the following iteration re-sanitizes what the paragraph actually holds. Over a 1500-cue file: SanitizeString calls 3462 -> 1964 (-43%), and the rule goes 0.926 ms -> 0.654 ms (-29%). On a continuation-heavy 1200-cue file, where more paragraphs get rewritten and drop the carry, 2.274 ms -> 2.111 ms (-7%). Verified output-identical across 192 configurations (2 corpora x 12 continuation styles x 4 languages x 2 lyrics settings), comparing every paragraph text and every callback log entry - 85,116 recorded fix events, identical checksums. A further 2.4M randomized rule invocations comparing carry against no-carry found no divergence. Adds tests: FixContinuationStyleTest pins a case that actually discriminates a correct carry from a wrong one (a plain continuation chain does not - it passes either way), and UtilitiesCountTagInTextTest covers Utilities.CountTagInText, which had no direct tests despite having two implementations to keep in step. Co-Authored-By: Claude Opus 5 --- .../FixCommonErrors/FixContinuationStyle.cs | 11 ++++- .../Common/UtilitiesCountTagInTextTest.cs | 35 ++++++++++++++++ .../FixContinuationStyleTest.cs | 42 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/libse/Common/UtilitiesCountTagInTextTest.cs create mode 100644 tests/libse/Forms/FixCommonErrors/FixContinuationStyleTest.cs diff --git a/src/libse/Forms/FixCommonErrors/FixContinuationStyle.cs b/src/libse/Forms/FixCommonErrors/FixContinuationStyle.cs index 9c3f473efbd..a4c544b0b12 100644 --- a/src/libse/Forms/FixCommonErrors/FixContinuationStyle.cs +++ b/src/libse/Forms/FixCommonErrors/FixContinuationStyle.cs @@ -40,14 +40,22 @@ public void Fix(Subtitle subtitle, IFixCallbacks callbacks) var inSentence = false; bool? inItalicSentence = null; + // SanitizeString runs four regex replaces per call, and the loop sanitized every + // paragraph twice: once as pNext's text, then again as p's text one iteration later. + // Carry the sanitized "next" value forward instead. The carry is dropped whenever the + // loop writes pNext.Text below, so the following iteration re-sanitizes what the + // paragraph actually holds. + string carriedText = null; + for (var i = 0; i < subtitle.Paragraphs.Count - 1; i++) { var p = subtitle.Paragraphs[i]; var pNext = subtitle.Paragraphs[i + 1]; var oldText = p.Text; var oldTextNext = pNext.Text; - var text = ContinuationUtilities.SanitizeString(p.Text); + var text = carriedText ?? ContinuationUtilities.SanitizeString(p.Text); var textNext = ContinuationUtilities.SanitizeString(pNext.Text); + carriedText = textNext; // captured before the Arabic conversion below, which the next iteration reapplies var isChecked = true; var shouldProcess = true; @@ -207,6 +215,7 @@ public void Fix(Subtitle subtitle, IFixCallbacks callbacks) if (IsPreviewStep(callbacks) && isChecked || !IsPreviewStep(callbacks)) { pNext.Text = newTextNext; + carriedText = null; // pNext.Text changed - the next iteration must re-sanitize } fixCount++; diff --git a/tests/libse/Common/UtilitiesCountTagInTextTest.cs b/tests/libse/Common/UtilitiesCountTagInTextTest.cs new file mode 100644 index 00000000000..d4663ee7e8d --- /dev/null +++ b/tests/libse/Common/UtilitiesCountTagInTextTest.cs @@ -0,0 +1,35 @@ +using Nikse.SubtitleEdit.Core.Common; + +namespace LibSETests.Common; + +public class UtilitiesCountTagInTextTest +{ + // The char overload has two implementations: MemoryExtensions.Count on net8+ and an + // IndexOf loop on netstandard2.1. Both must agree on these, in particular on a hit that + // lands on the very last index - the loop returns early from inside its body there. + [Theory] + [InlineData("", '"', 0)] + [InlineData("no quotes here", '"', 0)] + [InlineData("\"", '"', 1)] + [InlineData("say \"this\"", '"', 2)] + [InlineData("\"\"\"", '"', 3)] + [InlineData("- No.\r\n- Then stay.", '-', 2)] + [InlineData("{\\an8}{\\pos(10,20)}Hi", '{', 2)] + [InlineData("aaa", 'a', 3)] + public void CountsEveryOccurrenceOfChar(string text, char tag, int expected) + { + Assert.Equal(expected, Utilities.CountTagInText(text, tag)); + } + + // The char and string overloads are separate implementations; for a single-character tag + // they must not drift apart. + [Theory] + [InlineData("\"Are you coming?\" she asked.", '"')] + [InlineData("- Yes.\r\n- No.", '-')] + [InlineData("nothing to find", 'z')] + [InlineData("trailing hit-", '-')] + public void CharOverloadAgreesWithStringOverload(string text, char tag) + { + Assert.Equal(Utilities.CountTagInText(text, tag.ToString()), Utilities.CountTagInText(text, tag)); + } +} diff --git a/tests/libse/Forms/FixCommonErrors/FixContinuationStyleTest.cs b/tests/libse/Forms/FixCommonErrors/FixContinuationStyleTest.cs new file mode 100644 index 00000000000..ab8c8c82581 --- /dev/null +++ b/tests/libse/Forms/FixCommonErrors/FixContinuationStyleTest.cs @@ -0,0 +1,42 @@ +using Nikse.SubtitleEdit.Core.Common; +using Nikse.SubtitleEdit.Core.Enums; +using Nikse.SubtitleEdit.Core.Forms.FixCommonErrors; + +namespace LibSETests.Forms.FixCommonErrors; + +public class FixContinuationStyleTest +{ + // The rule walks paragraph pairs and carries paragraph i+1's sanitized text forward to use + // as paragraph i's on the next iteration, rather than sanitizing every paragraph twice. + // Carrying the wrong side of the pair still produces the right answer on a plain + // continuation chain, so this pins a case that actually discriminates: only paragraph 2 + // continues into paragraph 3 (40 ms gap), while paragraph 1 ends a sentence and paragraph 4 + // is too far away (1600 ms). Get the carry wrong and paragraph 2 keeps its dots-less text. + [Fact] + public void OnlyTheContinuingParagraphGetsTrailingDots() + { + var previousStyle = Configuration.Settings.General.ContinuationStyle; + try + { + Configuration.Settings.General.ContinuationStyle = ContinuationStyle.OnlyTrailingDots; + + var subtitle = new Subtitle(); + subtitle.Paragraphs.Add(new Paragraph("Wait!", 0, 1122)); + subtitle.Paragraphs.Add(new Paragraph("I was going to tell you", 1162, 3083)); + subtitle.Paragraphs.Add(new Paragraph("and now it is too late", 3123, 4220)); + subtitle.Paragraphs.Add(new Paragraph("[door slams]", 5820, 7720)); + subtitle.Renumber(); + + new FixContinuationStyle { FixAction = "act" }.Fix(subtitle, new EmptyFixCallback()); + + Assert.Equal("Wait!", subtitle.Paragraphs[0].Text); + Assert.Equal("I was going to tell you...", subtitle.Paragraphs[1].Text); + Assert.Equal("and now it is too late", subtitle.Paragraphs[2].Text); + Assert.Equal("[door slams]", subtitle.Paragraphs[3].Text); + } + finally + { + Configuration.Settings.General.ContinuationStyle = previousStyle; + } + } +}