SplitString: fix platform-dependent newline split, retarget net10.0 - #2167
Open
vladimir-pecanac-main wants to merge 1 commit into
Open
SplitString: fix platform-dependent newline split, retarget net10.0#2167vladimir-pecanac-main wants to merge 1 commit into
vladimir-pecanac-main wants to merge 1 commit into
Conversation
The newline sample split a \n-only literal on Environment.NewLine, so on Windows it found no separator and returned one element while the test asserted five. It passed on Linux CI only. Both sides now split on ["\r\n", "\n"], which is honest on every platform. - SplitStringUsingStringArrayWithoptions takes StringSplitOptions (default None) so the sample can demonstrate RemoveEmptyEntries on an input that has empty entries. - Adds SplitStringWithOptions, covering the four StringSplitOptions results the article tabulates, and CountSplitRanges, the allocation-free ReadOnlySpan<char>.Split into a stack-allocated Span<Range>. - net6.0 -> net10.0; MSTest 4.3.3, Microsoft.NET.Test.Sdk 18.9.0, coverlet.collector 10.0.1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sample for Different Ways to Split a String in C#, which is being republished.
The defect this PR exists for
SplitStringIntoNewLineswas called withnew[] { Environment.NewLine }against a literal whose only line endings are\n. On Linux that separator is"\n", so the split returns five elements and the test passes. On WindowsEnvironment.NewLineis"\r\n", the input holds no\r, no separator is ever found, and the split returns one element holding the whole string.WhenSplittingAStringIntoNewLine_ThenReturnArrayOfSubstringsassertedLength == 5against exactly that, so CI was green on a test that fails on a Windows developer's machine. Measured on this box (SDK 10.0.302) before the fix:Both sides now split on
["\r\n", "\n"], so the assertion is true on every platform rather than on the one the runner happens to use. A second test covers genuinely mixed\r\n/\ninput.Also in here
SplitStringUsingStringArrayWithoptionstakes aStringSplitOptionsparameter defaulting toNone, so the existing test keeps its exact behaviour whileMaincan passRemoveEmptyEntrieson an input that actually has empty entries ("apple,,banana;;kiwi"). The old input had none, so the option it was demonstrating did nothing visible.SplitStringWithOptionsplus four tests, covering all fourStringSplitOptionsresults on the runtime's own documented input"a,,b, c, , d ,e". Run on net10.0:Note that
RemoveEmptyEntrieson its own keeps the whitespace-only" ", which is the point of the pair.CountSplitRanges, one method demonstratingReadOnlySpan<char>.Splitinto a stack-allocatedSpan<Range>, which allocates no substrings at all.net6.0->net10.0on both projects. Packages: MSTest.TestAdapter / MSTest.TestFramework 2.2.8 -> 4.3.3, Microsoft.NET.Test.Sdk 17.1.0 -> 18.9.0, coverlet.collector 3.1.2 -> 10.0.1 (all read from the NuGet flat container today).Verification
Windows, SDK 10.0.302: