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

Add more AsSpan overloads to StringSegment #50428

Closed
BrennanConroy opened this issue Mar 30, 2021 · 4 comments · Fixed by #53463
Closed

Add more AsSpan overloads to StringSegment #50428

BrennanConroy opened this issue Mar 30, 2021 · 4 comments · Fixed by #53463
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Primitives
Milestone

Comments

@BrennanConroy
Copy link
Member

Background and Motivation

Many classes provide AsSpan methods and most provide at least the 3 basic methods, AsSpan(), AsSpan(int start), AsSpan(int start, int length).

The StringSegment class only provides AsSpan(), but it should provide the other two for consistency and ease of use.

Proposed API

namespace Microsoft.Extensions.Primitives
{
    public readonly struct StringSegment : IEquatable<StringSegment>, IEquatable<string>
    {
        public ReadOnlySpan<char> AsSpan();
+       public ReadOnlySpan<char> AsSpan(int start);
+       public ReadOnlySpan<char> AsSpan(int start, int length);
    }
}

Usage Examples

// input.AsSpan().Slice(startIndex, typeLength);
input.AsSpan(startIndex, typeLength);

Alternative Designs

No, other classes that provide AsSpan have similar overloads

Risks

None

@BrennanConroy BrennanConroy added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 30, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Memory untriaged New issue has not been triaged by the area owner labels Mar 30, 2021
@ghost
Copy link

ghost commented Mar 30, 2021

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

Issue Details

Background and Motivation

Many classes provide AsSpan methods and most provide at least the 3 basic methods, AsSpan(), AsSpan(int start), AsSpan(int start, int length).

The StringSegment class only provides AsSpan(), but it should provide the other two for consistency and ease of use.

Proposed API

namespace Microsoft.Extensions.Primitives
{
    public readonly struct StringSegment : IEquatable<StringSegment>, IEquatable<string>
    {
        public ReadOnlySpan<char> AsSpan();
+       public ReadOnlySpan<char> AsSpan(int start);
+       public ReadOnlySpan<char> AsSpan(int start, int length);
    }
}

Usage Examples

// input.AsSpan().Slice(startIndex, typeLength);
input.AsSpan(startIndex, typeLength);

Alternative Designs

No, other classes that provide AsSpan have similar overloads

Risks

None

Author: BrennanConroy
Assignees: -
Labels:

api-suggestion, area-System.Memory, untriaged

Milestone: -

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Mar 30, 2021
@ghost
Copy link

ghost commented Mar 30, 2021

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

Issue Details

Background and Motivation

Many classes provide AsSpan methods and most provide at least the 3 basic methods, AsSpan(), AsSpan(int start), AsSpan(int start, int length).

The StringSegment class only provides AsSpan(), but it should provide the other two for consistency and ease of use.

Proposed API

namespace Microsoft.Extensions.Primitives
{
    public readonly struct StringSegment : IEquatable<StringSegment>, IEquatable<string>
    {
        public ReadOnlySpan<char> AsSpan();
+       public ReadOnlySpan<char> AsSpan(int start);
+       public ReadOnlySpan<char> AsSpan(int start, int length);
    }
}

Usage Examples

// input.AsSpan().Slice(startIndex, typeLength);
input.AsSpan(startIndex, typeLength);

Alternative Designs

No, other classes that provide AsSpan have similar overloads

Risks

None

Author: BrennanConroy
Assignees: -
Labels:

api-ready-for-review, area-Extensions-Primitives

Milestone: -

@maryamariyan maryamariyan added this to the 6.0.0 milestone Apr 23, 2021
@bartonjs
Copy link
Member

bartonjs commented May 27, 2021

Video

Looks good as proposed.

namespace Microsoft.Extensions.Primitives
{
    partial struct StringSegment : IEquatable<StringSegment>, IEquatable<string>
    {
       public ReadOnlySpan<char> AsSpan(int start);
       public ReadOnlySpan<char> AsSpan(int start, int length);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 27, 2021
@gfoidl
Copy link
Member

gfoidl commented May 28, 2021

I'd like to add these overloads.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 29, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 3, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Primitives
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants