Skip to content

PathString.StartsWithSegments: Clarify trailing slash behavior in doc comments #53924

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

Conversation

david-acker
Copy link
Member

PathString.StartsWithSegments: Clarify trailing slash behavior in doc comments

Description

Adds remarks clarifying some unexpected behavior of trailing slashes with the StartsWithSegments methods on PathString.

Remarks

/// <remarks>
/// When the <paramref name="other"/> parameter contains a trailing slash, the <see cref="PathString"/> being checked
/// must either exactly match or include a trailing slash. For instance, for a <see cref="PathString"/> of "/a/b",
/// this method will return <c>true</c> for "/a", but will return <c>false</c> for "/a/".
/// Whereas, a <see cref="PathString"/> of "/a//b/" will return <c>true</c> when compared with "/a/".
/// </remarks>

Fixes #2713

@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Feb 9, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 9, 2024
@captainsafia
Copy link
Member

@david-acker Doc change looks good! Thanks a ton for digging out this old gem from the backlog and taking a look at it. 😄

Could I trouble you to add a new test case here to document this behavior in test too? I did a look through and haven't seen existing coverage. Regardless, it might be good to have a test method of its own so it's easy to spot.

@david-acker
Copy link
Member Author

@captainsafia Will do! Do you think this warrants four separate test methods for each of the StartsWithSegments overloads? I've added separate tests methods for each of these, but I can pare these down if you'd like.

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

LGTM! I'm OK with the tests for each overload. More tests is always better than less.

@captainsafia captainsafia merged commit 9742e7d into dotnet:main Feb 12, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview2 milestone Feb 12, 2024
@david-acker david-acker deleted the clarify-StartsWithSegments-trailing-slash-behavior branch February 23, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usability: PathString.StartsWithSegments() with a trailing slash is not intuitive behavior
3 participants