This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Add ROSpan ToLower and ToUpper string-like APIs with CultureInfo #27193
Conversation
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
ahsonkhan
added
blocked
stephentoub
reviewed
Feb 16, 2018
| return -1; | ||
|
|
||
| string sourceString = source.ToString(); | ||
| string resultString = sourceString.ToLower(culture); |
Member
There was a problem hiding this comment.
It's unfortunate we need to allocate here (potentially twice), but I don't have a better answer (unless we want to make the API netcoreapp-specific).
stephentoub
reviewed
Feb 16, 2018
| [Fact] | ||
| public static void Test_ToLower_Culture() | ||
| { | ||
| foreach (var testdata in ToLower_Culture_TestData()) |
Member
There was a problem hiding this comment.
Is it possible to use a Theory?
Author
There was a problem hiding this comment.
Probably. I borrowed this from the existing string tests and tweaked it a bit.
stephentoub
approved these changes
Feb 16, 2018
| /// <param name="culture">An object that supplies culture-specific casing rules.</param> | ||
| /// <exception cref="System.ArgumentNullException"> | ||
| /// Thrown when <paramref name="culture"/> is null. | ||
| /// </exception> |
Member
There was a problem hiding this comment.
Could you add remarks saying whether the source and destination can be overlapping (or the same span)
Author
|
@dotnet-bot test this please |
Author
|
ToLower(CultureInfo) was added in netstandard2.0. What can we do for netstandard1.1? |
Member
|
@ahsonkhan you can use CultureInfo.TextInfo.ToLower |
Author
|
|
Author
|
@dotnet-bot test Windows x64 Debug Build |
Author
|
@dotnet-bot test this please |
Author
|
@dotnet-bot test this please |
Author
|
@dotnet-bot test Linux x64 Release Build |
Author
|
Linux x64 Release Build |
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
…net/corefx#27193) * Add ROSpan ToLower and ToUpper string-like APIs with CultureInfo * Add comment about overlapping spans + test. * Respond to recent change AsReadOnlySpan -> AsSpan and use Theory * Add System.Globalization for netstandard1.1 * Use TextInfo.ToLower/ToUpper for netstandard1.1 * Fix XML comment Commit migrated from dotnet/corefx@865f086
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
Part of https://github.com/dotnet/corefx/issues/21395#issuecomment-359906138
Depends on: dotnet/coreclr#16379 and dotnet/coreclr#16496
cc @jkotas, @stephentoub, @KrzysztofCwalina, @tarekgh, @JeremyKuhne, @joshfree