Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StringSegment behavior #55501

Merged
merged 6 commits into from
Jul 15, 2021
Merged

StringSegment behavior #55501

merged 6 commits into from
Jul 15, 2021

Conversation

hrrrrustic
Copy link
Contributor

fix #45021
fix #53727

@ghost
Copy link

ghost commented Jul 12, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

fix #45021
fix #53727

Author: hrrrrustic
Assignees: -
Labels:

area-Extensions-Primitives

Milestone: -

@ghost ghost added this to Active PRs in ML, Extensions, Globalization, etc, POD. Jul 12, 2021
@hrrrrustic hrrrrustic changed the title String segment behavior StringSegment behavior Jul 12, 2021
Co-authored-by: Günther Foidl <gue@korporal.at>
@GrabYourPitchforks
Copy link
Member

@maryamariyan You marked #45021 as up-for-grabs but never specified what you believe the correct behavior should be: consistently return -1 or consistently throw an exception.

/cc @halter73

@hrrrrustic
Copy link
Contributor Author

I guess -1 is the correct way as it described in all of this methods docs 😄

/// <returns>The zero-based index position of <paramref name="c"/> from the beginning of the <see cref="StringSegment"/> if that character is found, or -1 if it is not.</returns>

/// was found; -1 if no character in <paramref name="anyOf"/> was found.</returns>

/// <returns>The zero-based index position of value if that character is found, or -1 if it is not.</returns>

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

I think this looks good. @GrabYourPitchforks - any thoughts/concerns?

@eerhardt
Copy link
Member

Linq.Expression failures are #55536. Merging.

@eerhardt eerhardt merged commit 188c270 into dotnet:main Jul 15, 2021
ML, Extensions, Globalization, etc, POD. automation moved this from Active PRs to Done Jul 15, 2021
@eerhardt
Copy link
Member

Thanks @hrrrrustic for the contribution here!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StringSegment IndexOf behavior StringSegment doesn't handle null buffers consistently
5 participants