Skip to content

Add StringBuilderExtensions, consolidate StringBuilder helpers - #13372

Open
ivandrofly wants to merge 1 commit into
SubtitleEdit:mainfrom
ivandrofly:add-stringbuilder-extensions
Open

Add StringBuilderExtensions, consolidate StringBuilder helpers#13372
ivandrofly wants to merge 1 commit into
SubtitleEdit:mainfrom
ivandrofly:add-stringbuilder-extensions

Conversation

@ivandrofly

Copy link
Copy Markdown
Member

Summary

  • New Nikse.SubtitleEdit.Core.Common.StringBuilderExtensions class consolidating generic StringBuilder helpers that were scattered as private/protected statics
  • Trim() — moved from SubtitleFormat.TrimBuilder (in-place trim, avoids the sb.ToString().Trim() extra allocation); call sites in SubRip and AdvancedSubStationAlpha updated
  • StartsWith(char) / EndsWith(char) — moved (not copied, to avoid ambiguous-call errors) from StringExtensions; existing call sites unaffected since the namespace is unchanged
  • CountChar(char) — moved from MergeAndSplitHelper; StringBuilder.GetChunks() turned out to be missing from the netstandard2.1 reference assemblies, so that target falls back to an indexer loop behind the existing #if NET8_0_OR_GREATER guard pattern
  • AppendNumber(int, int) — moved from AdvancedSubStationAlpha (zero-padded append matching "{0:00}")
  • Adds StringBuilderExtensionsTest (21 tests), including a cross-chunk CountChar case
  • Behavior is unchanged; bodies were moved verbatim. Format-specific helpers (AppendTimeCode, EBU/MP4 tag helpers) stay where they are

Test plan

  • dotnet build src/libse/LibSE.csproj — both netstandard2.1 and net10.0 targets compile
  • dotnet test tests/libse/LibSETests.csproj — full suite passes (956/956 locally)
  • Save a subtitle as SubRip and as ASSA and confirm output is unchanged (leading/trailing whitespace trimmed, ASSA time codes still h:mm:ss.cc)

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates scattered StringBuilder helper logic into a new Nikse.SubtitleEdit.Core.Common.StringBuilderExtensions extension class, and updates call sites to use these shared helpers while adding unit tests to validate behavior.

Changes:

  • Added StringBuilderExtensions with Trim, StartsWith/EndsWith(char), CountChar(char), and AppendNumber(int,int) helpers.
  • Updated format writers (SubRip, AdvancedSubStationAlpha) and translation merge logic (MergeAndSplitHelper) to use the new extensions.
  • Added StringBuilderExtensionsTest coverage for the new helpers.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/libse/Core/StringBuilderExtensionsTest.cs Adds unit tests for the new StringBuilder extension methods.
src/libuilogic/Translate/MergeAndSplitHelper.cs Switches merged-row end-character counting to StringBuilder.CountChar.
src/libse/SubtitleFormats/SubtitleFormat.cs Removes the previous protected TrimBuilder helper (now consolidated into extensions).
src/libse/SubtitleFormats/SubRip.cs Replaces TrimBuilder(sb) with sb.Trim().
src/libse/SubtitleFormats/AdvancedSubStationAlpha.cs Replaces TrimBuilder/AppendNumber helpers with sb.Trim() and sb.AppendNumber(...).
src/libse/Common/StringExtensions.cs Removes StringBuilder-specific StartsWith/EndsWith extensions (moved to StringBuilderExtensions).
src/libse/Common/StringBuilderExtensions.cs Introduces the consolidated StringBuilder extension methods.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/libse/Common/StringBuilderExtensions.cs
Comment thread tests/libse/Core/StringBuilderExtensionsTest.cs
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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants