From cfe40e4b1803e9fccc49c17b041c00a2791f7c6a Mon Sep 17 00:00:00 2001 From: Ivandro Jao Date: Sat, 8 Aug 2026 11:13:53 +0100 Subject: [PATCH 1/2] Add StringBuilderExtensions, consolidate StringBuilder helpers Move scattered generic StringBuilder helpers into a new extension class Nikse.SubtitleEdit.Core.Common.StringBuilderExtensions: - Trim() - was SubtitleFormat.TrimBuilder (in-place trim without the sb.ToString().Trim() allocation) - StartsWith(char)/EndsWith(char) - moved from StringExtensions - CountChar(char) - was private in MergeAndSplitHelper; GetChunks is missing from the netstandard2.1 reference assemblies, so that target falls back to an indexer loop - AppendNumber(int, int) - was private in AdvancedSubStationAlpha All call sites updated; behavior unchanged. Adds xUnit coverage for the new class. Co-Authored-By: Claude Fable 5 --- src/libse/Common/StringBuilderExtensions.cs | 95 +++++++++ src/libse/Common/StringExtensions.cs | 10 - .../AdvancedSubStationAlpha.cs | 28 +-- src/libse/SubtitleFormats/SubRip.cs | 2 +- src/libse/SubtitleFormats/SubtitleFormat.cs | 23 --- .../Translate/MergeAndSplitHelper.cs | 25 +-- .../libse/Core/StringBuilderExtensionsTest.cs | 186 ++++++++++++++++++ 7 files changed, 290 insertions(+), 79 deletions(-) create mode 100644 src/libse/Common/StringBuilderExtensions.cs create mode 100644 tests/libse/Core/StringBuilderExtensionsTest.cs diff --git a/src/libse/Common/StringBuilderExtensions.cs b/src/libse/Common/StringBuilderExtensions.cs new file mode 100644 index 00000000000..5457e0be29d --- /dev/null +++ b/src/libse/Common/StringBuilderExtensions.cs @@ -0,0 +1,95 @@ +using System.Text; + +namespace Nikse.SubtitleEdit.Core.Common +{ + public static class StringBuilderExtensions + { + /// + /// Trims leading/trailing whitespace inside the builder - the "sb.ToString().Trim()" + /// idiom allocates the whole output an extra time. + /// + public static void Trim(this StringBuilder sb) + { + while (sb.Length > 0 && char.IsWhiteSpace(sb[sb.Length - 1])) + { + sb.Length--; + } + + var start = 0; + while (start < sb.Length && char.IsWhiteSpace(sb[start])) + { + start++; + } + + if (start > 0) + { + sb.Remove(0, start); + } + } + + public static bool StartsWith(this StringBuilder sb, char c) + { + return sb.Length > 0 && sb[0] == c; + } + + public static bool EndsWith(this StringBuilder sb, char c) + { + return sb.Length > 0 && sb[sb.Length - 1] == c; + } + + // Same count as scanning sb.ToString() without materializing the whole + // accumulated text into a fresh string. + public static int CountChar(this StringBuilder sb, char c) + { + var count = 0; +#if NET8_0_OR_GREATER + foreach (var chunk in sb.GetChunks()) + { + foreach (var ch in chunk.Span) + { + if (ch == c) + { + count++; + } + } + } +#else + // GetChunks is missing from the netstandard2.1 reference assemblies. + for (var i = 0; i < sb.Length; i++) + { + if (sb[i] == c) + { + count++; + } + } +#endif + return count; + } + + // Matches "{0:00}"/"{0:000}"-style formatting: sign first, then the absolute value + // padded with leading zeros to minDigits. Negates on a long so int.MinValue does + // not overflow. + public static void AppendNumber(this StringBuilder sb, int value, int minDigits) + { + var v = (long)value; + if (v < 0) + { + sb.Append('-'); + v = -v; + } + + var digitCount = 1; + for (var rest = v; rest >= 10; rest /= 10) + { + digitCount++; + } + + for (; digitCount < minDigits; digitCount++) + { + sb.Append('0'); + } + + sb.Append(v); + } + } +} diff --git a/src/libse/Common/StringExtensions.cs b/src/libse/Common/StringExtensions.cs index 49922a6f6a9..6f70c350878 100644 --- a/src/libse/Common/StringExtensions.cs +++ b/src/libse/Common/StringExtensions.cs @@ -91,21 +91,11 @@ public static bool StartsWith(this string s, char c) return s.Length > 0 && s[0] == c; } - public static bool StartsWith(this StringBuilder sb, char c) - { - return sb.Length > 0 && sb[0] == c; - } - public static bool EndsWith(this string s, char c) { return s.Length > 0 && s[s.Length - 1] == c; } - public static bool EndsWith(this StringBuilder sb, char c) - { - return sb.Length > 0 && sb[sb.Length - 1] == c; - } - public static bool Contains(this string source, char value) { return source.IndexOf(value) >= 0; diff --git a/src/libse/SubtitleFormats/AdvancedSubStationAlpha.cs b/src/libse/SubtitleFormats/AdvancedSubStationAlpha.cs index bec69a5c23d..fc85951a4aa 100644 --- a/src/libse/SubtitleFormats/AdvancedSubStationAlpha.cs +++ b/src/libse/SubtitleFormats/AdvancedSubStationAlpha.cs @@ -310,7 +310,7 @@ string HeaderTemplate() => headerTemplate ??= $@"[Script Info] // Trim inside the builder instead of "sb.ToString().Trim() + newline", which // allocated the whole multi-megabyte output twice more. - TrimBuilder(sb); + sb.Trim(); return sb.Append(Environment.NewLine).ToString(); } @@ -327,13 +327,13 @@ private static StringBuilder AppendTimeCode(StringBuilder sb, TimeCode timeCode) fragment = 0; } - AppendNumber(sb, ts.Days * 24 + ts.Hours, 1); + sb.AppendNumber(ts.Days * 24 + ts.Hours, 1); sb.Append(':'); - AppendNumber(sb, ts.Minutes, 2); + sb.AppendNumber(ts.Minutes, 2); sb.Append(':'); - AppendNumber(sb, ts.Seconds, 2); + sb.AppendNumber(ts.Seconds, 2); sb.Append('.'); - AppendNumber(sb, fragment, 2); + sb.AppendNumber(fragment, 2); return sb; } @@ -355,24 +355,6 @@ private static void AppendTextWithAssaNewLines(StringBuilder sb, string text) sb.Append(span); } - // Matches "{0:00}"-style formatting: sign first, then the absolute value padded with - // leading zeros to minDigits. - private static void AppendNumber(StringBuilder sb, int value, int minDigits) - { - if (value < 0) - { - sb.Append('-'); - value = -value; - } - - if (minDigits >= 2 && value < 10) - { - sb.Append('0'); - } - - sb.Append(value); - } - public static string GetHeaderAndStylesFromSubStationAlpha(string header) { var scriptInfo = string.Empty; diff --git a/src/libse/SubtitleFormats/SubRip.cs b/src/libse/SubtitleFormats/SubRip.cs index efcd7d8e51b..7c3d2630eeb 100644 --- a/src/libse/SubtitleFormats/SubRip.cs +++ b/src/libse/SubtitleFormats/SubRip.cs @@ -72,7 +72,7 @@ public override string ToText(Subtitle subtitle, string title) sb.Append(p.Text).Append(Environment.NewLine).Append(Environment.NewLine); } - TrimBuilder(sb); + sb.Trim(); return sb.Append(Environment.NewLine).Append(Environment.NewLine).ToString(); } diff --git a/src/libse/SubtitleFormats/SubtitleFormat.cs b/src/libse/SubtitleFormats/SubtitleFormat.cs index b34333e7dc2..b08628aa4c0 100644 --- a/src/libse/SubtitleFormats/SubtitleFormat.cs +++ b/src/libse/SubtitleFormats/SubtitleFormat.cs @@ -551,29 +551,6 @@ public static int FramesToMillisecondsMax999(double frames) public bool BatchMode { get; set; } public double? BatchSourceFrameRate { get; set; } - /// - /// Trims leading/trailing whitespace inside the builder - the "sb.ToString().Trim()" - /// idiom in ToText implementations allocates the whole output an extra time. - /// - protected static void TrimBuilder(StringBuilder sb) - { - while (sb.Length > 0 && char.IsWhiteSpace(sb[sb.Length - 1])) - { - sb.Length--; - } - - var start = 0; - while (start < sb.Length && char.IsWhiteSpace(sb[start])) - { - start++; - } - - if (start > 0) - { - sb.Remove(0, start); - } - } - public static string ToUtf8XmlString(XmlDocument xml, bool omitXmlDeclaration = false) { var settings = new XmlWriterSettings diff --git a/src/libuilogic/Translate/MergeAndSplitHelper.cs b/src/libuilogic/Translate/MergeAndSplitHelper.cs index 96beadf0575..2b6c298cadb 100644 --- a/src/libuilogic/Translate/MergeAndSplitHelper.cs +++ b/src/libuilogic/Translate/MergeAndSplitHelper.cs @@ -488,25 +488,6 @@ private static void InitializeFirstItem(MergeResult result, MergeContext context } } - // Same count as Utilities.CountTagInText(builder.ToString(), c) without materializing the - // whole accumulated text into a fresh string per merged row. - private static int CountChar(StringBuilder builder, char c) - { - var count = 0; - foreach (var chunk in builder.GetChunks()) - { - foreach (var ch in chunk.Span) - { - if (ch == c) - { - count++; - } - } - } - - return count; - } - private static readonly int NewLineUrlEncodeLength = Utilities.UrlEncodeLength(Environment.NewLine); private static bool ExceedsMaxSize(MergeResult result, MergeContext context, TranslateRow currentRow, int maxTextSize) @@ -590,7 +571,7 @@ private static void ProcessEmptyRow(MergeResult result, MergeContext context, in context.CurrentItem.TextIndexStart = result.Text.Length; context.CurrentItem.TextIndexEnd = result.Text.Length; context.CurrentItem.EndChar = endChar; - context.CurrentItem.EndCharOccurrences = CountChar(context.TextBuilder, endChar); + context.CurrentItem.EndCharOccurrences = context.TextBuilder.CountChar(endChar); result.MergeResultItems.Add(context.CurrentItem); } @@ -621,7 +602,7 @@ private static void ProcessSentenceEndingRow(MergeResult result, MergeContext co { var endChar = result.Text[^1]; context.CurrentItem.EndChar = endChar; - context.CurrentItem.EndCharOccurrences = CountChar(context.TextBuilder, endChar); + context.CurrentItem.EndCharOccurrences = context.TextBuilder.CountChar(endChar); context.CurrentItem.TextIndexEnd = result.Text.Length; } @@ -681,7 +662,7 @@ private static void FinalizeResult(MergeResult result, MergeContext context) { var endChar = result.Text[^1]; context.CurrentItem.EndChar = endChar; - context.CurrentItem.EndCharOccurrences = CountChar(context.TextBuilder, endChar); + context.CurrentItem.EndCharOccurrences = context.TextBuilder.CountChar(endChar); } } diff --git a/tests/libse/Core/StringBuilderExtensionsTest.cs b/tests/libse/Core/StringBuilderExtensionsTest.cs new file mode 100644 index 00000000000..690c945030b --- /dev/null +++ b/tests/libse/Core/StringBuilderExtensionsTest.cs @@ -0,0 +1,186 @@ +using System.Text; +using Nikse.SubtitleEdit.Core.Common; + +namespace LibSETests.Core; + +public class StringBuilderExtensionsTest +{ + [Fact] + public void TrimLeadingAndTrailing() + { + var sb = new StringBuilder(" \t\r\n Hello world \r\n "); + sb.Trim(); + Assert.Equal("Hello world", sb.ToString()); + } + + [Fact] + public void TrimLeadingOnly() + { + var sb = new StringBuilder(" Hello"); + sb.Trim(); + Assert.Equal("Hello", sb.ToString()); + } + + [Fact] + public void TrimTrailingOnly() + { + var sb = new StringBuilder("Hello "); + sb.Trim(); + Assert.Equal("Hello", sb.ToString()); + } + + [Fact] + public void TrimWhiteSpaceOnly() + { + var sb = new StringBuilder(" \t\r\n "); + sb.Trim(); + Assert.Equal(0, sb.Length); + } + + [Fact] + public void TrimEmpty() + { + var sb = new StringBuilder(); + sb.Trim(); + Assert.Equal(0, sb.Length); + } + + [Fact] + public void TrimNoWhiteSpace() + { + var sb = new StringBuilder("Hello"); + sb.Trim(); + Assert.Equal("Hello", sb.ToString()); + } + + [Fact] + public void StartsWithEmpty() + { + var sb = new StringBuilder(); + Assert.False(sb.StartsWith('a')); + } + + [Fact] + public void StartsWithMatch() + { + var sb = new StringBuilder("abc"); + Assert.True(sb.StartsWith('a')); + } + + [Fact] + public void StartsWithNoMatch() + { + var sb = new StringBuilder("abc"); + Assert.False(sb.StartsWith('b')); + } + + [Fact] + public void EndsWithEmpty() + { + var sb = new StringBuilder(); + Assert.False(sb.EndsWith('c')); + } + + [Fact] + public void EndsWithMatch() + { + var sb = new StringBuilder("abc"); + Assert.True(sb.EndsWith('c')); + } + + [Fact] + public void EndsWithNoMatch() + { + var sb = new StringBuilder("abc"); + Assert.False(sb.EndsWith('b')); + } + + [Fact] + public void CountCharEmpty() + { + var sb = new StringBuilder(); + Assert.Equal(0, sb.CountChar('a')); + } + + [Fact] + public void CountCharNoMatch() + { + var sb = new StringBuilder("Hello world"); + Assert.Equal(0, sb.CountChar('z')); + } + + [Fact] + public void CountCharMultiple() + { + var sb = new StringBuilder("Hello world"); + Assert.Equal(3, sb.CountChar('l')); + } + + [Fact] + public void CountCharAcrossChunks() + { + var sb = new StringBuilder("a", 1); + for (var i = 0; i < 100; i++) + { + sb.Append("bab"); + } + + Assert.Equal(101, sb.CountChar('a')); + } + + [Fact] + public void AppendNumberNoPadding() + { + var sb = new StringBuilder(); + sb.AppendNumber(5, 1); + Assert.Equal("5", sb.ToString()); + } + + [Fact] + public void AppendNumberPadsSingleDigit() + { + var sb = new StringBuilder(); + sb.AppendNumber(5, 2); + Assert.Equal("05", sb.ToString()); + } + + [Fact] + public void AppendNumberDoesNotPadTwoDigits() + { + var sb = new StringBuilder(); + sb.AppendNumber(42, 2); + Assert.Equal("42", sb.ToString()); + } + + [Fact] + public void AppendNumberZeroPadded() + { + var sb = new StringBuilder(); + sb.AppendNumber(0, 2); + Assert.Equal("00", sb.ToString()); + } + + [Fact] + public void AppendNumberNegative() + { + var sb = new StringBuilder(); + sb.AppendNumber(-5, 2); + Assert.Equal("-05", sb.ToString()); + } + + [Fact] + public void AppendNumberPadsToThreeDigits() + { + var sb = new StringBuilder(); + sb.AppendNumber(5, 3); + Assert.Equal("005", sb.ToString()); + } + + [Fact] + public void AppendNumberIntMinValue() + { + var sb = new StringBuilder(); + sb.AppendNumber(int.MinValue, 2); + Assert.Equal("-2147483648", sb.ToString()); + } +} From 9cefb46b35f0e6596cfb87e3d14ac307cf18ec91 Mon Sep 17 00:00:00 2001 From: niksedk Date: Mon, 17 Aug 2026 15:13:04 +0200 Subject: [PATCH 2/2] Update SubStationAlpha for the TrimBuilder move main gained a new TrimBuilder(sb) call site in SubStationAlpha.ToText after this branch was cut; point it at the sb.Trim() extension so the merge builds. Co-Authored-By: Claude Opus 5 --- src/libse/SubtitleFormats/SubStationAlpha.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libse/SubtitleFormats/SubStationAlpha.cs b/src/libse/SubtitleFormats/SubStationAlpha.cs index 9de2e74be1d..da794a85356 100644 --- a/src/libse/SubtitleFormats/SubStationAlpha.cs +++ b/src/libse/SubtitleFormats/SubStationAlpha.cs @@ -187,7 +187,7 @@ [V4 Styles] // Trim inside the builder instead of "sb.ToString().Trim() + newline", which // allocated the whole output twice more (same fix as [V4+ Styles]). - TrimBuilder(sb); + sb.Trim(); return sb.Append(Environment.NewLine).ToString(); }