Deprecates Pointer::split_at, adds Pointer::split_at_offset
#89
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
| @@ -233,7 +233,37 @@ impl Pointer { | |||
| /// assert_eq!(tail, Pointer::from_static("/bar/baz")); | |||
| /// assert_eq!(ptr.split_at(3), None); | |||
| /// ``` | |||
| #[deprecated( | |||
| since = "0.8.0", | |||
There was a problem hiding this comment.
Lol, no. I skipped ahead. Thanks!
| /// assert_eq!(tail, Pointer::from_static("/bar/baz")); | ||
| /// assert_eq!(ptr.split_at_offset(3), None); | ||
| /// ``` | ||
| pub fn split_at_offset(&self, offset: usize) -> Option<(&Self, &Self)> { |
There was a problem hiding this comment.
This one is safe because it returns an Option, but should we maybe make it unsafe (and not check the boundary condition)?
I imagine that one either will have a valid offset to use already, or they'll use split_at instead. Seems unlikely they'll try some offset they're unsure is correct.
There was a problem hiding this comment.
I kept the signature the same as split_at assuming we'd deprecate it and phase it out. Then perhaps re-introduce it using the position instead.
I'm truly not sure why I opted to go for offset in hindsight. It's like I spaced on the fact that I had get etc that were index based. Rather annoying mistake on my part.
There was a problem hiding this comment.
What about adding split_at_offset_unchecked? The reason I'm hesitant is solely for crates that avoid unsafe entirely.
There was a problem hiding this comment.
I'm truly not sure why I opted to go for offset in hindsight. It's like I spaced on the fact that I had get etc that were index based. Rather annoying mistake on my part.
Don't beat yourself over it, it wasn't as obvious back then. I also didn't realise we could extend the get method to achieve what we really wanted to do with that; if I had, I'd have pushed back more.
What about adding split_at_offset_unchecked? The reason I'm hesitant is solely for crates that avoid unsafe entirely.
Those can opt to use split_at ;)
Unless your concern is over a lack of alternative in the interim? I suppose we can also keep split_at_offset with the current signature, though we'd probably deprecate it too longer term. Feels a bit odd to add something we may remove later, but I think it makes sense in this case.
There was a problem hiding this comment.
Hmm. Good points. You're right that it is odd - especially if we don't end up spacing the releases.
|
|
||
| ### Changed | ||
|
|
||
| - Changed signature of `PathBuf::parse` to avoid requiring allocation. | ||
| - Bumps minimum Rust version to 1.79. | ||
| - `Pointer::get` now accepts ranges and can produce `Pointer` segments as output (similar to | ||
| `slice::get`). | ||
| - Deprecates `Pointer::split_at` |
There was a problem hiding this comment.
Deprecated can have its own section.
Solves #88
Pointer::split_atPointer::split_at_offset