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

Slicing API for Pointer #84

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Slicing API for Pointer #84

merged 3 commits into from
Oct 21, 2024

Conversation

asmello
Copy link
Collaborator

@asmello asmello commented Oct 20, 2024

Follow up of some ideas in #42.

This is effectively an extension of the split methods that can slice and dice Pointer types without allocation, similar to how slice::get works.

We use token indices as the range bounds, which ensures that any valid bound produces a valid &Pointer as a result.

This is not a breaking change.

@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.2%. Comparing base (6630744) to head (4e67712).

Additional details and impacted files
Files with missing lines Coverage Δ
src/pointer.rs 98.2% <100.0%> (-0.1%) ⬇️
src/pointer/slice.rs 100.0% <100.0%> (ø)

@asmello asmello requested a review from chanced October 20, 2024 11:52
@chanced
Copy link
Owner

chanced commented Oct 20, 2024

SliceIndex did work!? That's awesome! You are damn good at this man.

I'm still on mobile, out and about, but I'll look at it tonight, hopefully.

@chanced
Copy link
Owner

chanced commented Oct 21, 2024

Man, this language is awesome.

This solves #42. We already have a way to get a token by index, this allows for the rest (splitting, ranging, etc). Moving forward, we can reference tokens by index or range if we ever add additional nice-to-have methods.

#42 covered more than the range or being able to split at tokens. I'm going to split off a couple more tickets from it and reduce it down to simply being able to split at an index.

@chanced chanced merged commit b423489 into chanced:main Oct 21, 2024
19 checks passed
@asmello
Copy link
Collaborator Author

asmello commented Oct 21, 2024

SliceIndex did work!? That's awesome! You are damn good at this man.

Nope, it didn't! You weren't wrong about that remark, it's just that we're not actually meant to implement SliceIndex (it's even a sealed trait, for good reason). I introduced our own trait instead.

Man, this language is awesome.

I tend to agree, it's pretty neat I was able to express what I wanted with the trait system.

@asmello
Copy link
Collaborator Author

asmello commented Oct 21, 2024

Looking again I think SlicePointer is a bit awkward as a name, I meant it as "you slice a pointer with it", but that's probably not how most people will read it. PointerSlice probably makes more sense. Will follow up with a quick rename before we release.

@asmello asmello deleted the am/slice branch October 21, 2024 16:05
@asmello
Copy link
Collaborator Author

asmello commented Oct 21, 2024

Actually, I'm thinking about it all wrong. It's SliceIndex, so it should be PointerIndex, duh.

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

Successfully merging this pull request may close these issues.

3 participants