LastIndexOf breaking change #21375
LastIndexOf breaking change #21375
Conversation
includes/core-changes/corefx/5.0/lastindexof-improved-handing-of-empty-values.md
Show resolved
Hide resolved
includes/core-changes/corefx/5.0/lastindexof-improved-handing-of-empty-values.md
Show resolved
Hide resolved
| Console.WriteLine(span.LastIndexOf("")); // prints '5' (correct) | ||
| ``` | ||
|
|
||
| In these examples, `5` is the correct answer because `"Hello".Substring(5)` and `"Hello".AsSpan().Slice(5)` both produce an empty string, which is trivially equal to the empty substring that is sought. |
There was a problem hiding this comment.
The API reference needs to reflect this.
There was a problem hiding this comment.
What do you mean by this comment? True, the sentence I included in the breaking change notification is a drastic simplification of how IndexOf and LastIndexOf work, and there are some counterexamples. But as a high-level overview it's a useful way to think about the API.
See https://github.com/dotnet/runtime/blob/a057f27e860c1a59aa12e7dd9be028fba82b037f/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs#L223 if you're curious as to the real logic that underlies these APIs.
There was a problem hiding this comment.
@GrabYourPitchforks I meant this, and this, etc..
Currently it says that the method returns the following:
The zero-based starting index position of value if that string is found, or -1 if it is not. If value is Empty, the return value is the last index position in this instance.
Note the bolded part isn't correct (similar wording is there for different overloads).
There was a problem hiding this comment.
Ah, you're referring to the remarks at https://docs.microsoft.com/dotnet/api/system.string.lastindexof. I don't think those files are in this repo, so they'd not be included in this PR.
There was a problem hiding this comment.
Yep, they belong to dotnet-api-docs repo.
There was a problem hiding this comment.
I'll update the API docs now.
includes/core-changes/corefx/5.0/lastindexof-improved-handing-of-empty-values.md
Show resolved
Hide resolved
BillWagner
left a comment
BillWagner
left a comment
There was a problem hiding this comment.
This LGTM @gewarren
You can decide if any changes are warranted from the conversations.
5 participants
Fixes #21372.
Preview link.