-
Notifications
You must be signed in to change notification settings - Fork 4
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
Deprecates Pointer::split_at
, adds Pointer::split_at_offset
#89
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we do 0.7 already? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lol, no. I skipped ahead. Thanks! |
||
note = "renamed to `split_at_offset` - `split_at` will become position (index) based by 1.0" | ||
)] | ||
pub fn split_at(&self, offset: usize) -> Option<(&Self, &Self)> { | ||
self.split_at_offset(offset) | ||
} | ||
|
||
/// Splits the `Pointer` at the given offset if the character at the index is | ||
/// a separator backslash (`'/'`), returning `Some((head, tail))`. Otherwise, | ||
/// returns `None`. | ||
/// | ||
/// For the following JSON Pointer, the following splits are possible (0, 4, 8): | ||
/// ```text | ||
/// /foo/bar/baz | ||
/// ↑ ↑ ↑ | ||
/// 0 4 8 | ||
/// ``` | ||
/// All other indices will return `None`. | ||
/// | ||
/// ## Example | ||
/// | ||
/// ```rust | ||
/// # use jsonptr::Pointer; | ||
/// let ptr = Pointer::from_static("/foo/bar/baz"); | ||
/// let (head, tail) = ptr.split_at(4).unwrap(); | ||
/// assert_eq!(head, Pointer::from_static("/foo")); | ||
/// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is safe because it returns an I imagine that one either will have a valid offset to use already, or they'll use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept the signature the same as I'm truly not sure why I opted to go for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Don't beat yourself over it, it wasn't as obvious back then. I also didn't realise we could extend the
Those can opt to use Unless your concern is over a lack of alternative in the interim? I suppose we can also keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. Good points. You're right that it is odd - especially if we don't end up spacing the releases. |
||
if self.0.as_bytes().get(offset).copied() != Some(b'/') { | ||
return None; | ||
} | ||
|
@@ -393,7 +423,7 @@ impl Pointer { | |
} | ||
idx += a.encoded().len() + 1; | ||
} | ||
self.split_at(idx).map_or(self, |(head, _)| head) | ||
self.split_at_offset(idx).map_or(self, |(head, _)| head) | ||
} | ||
|
||
/// Attempts to delete a `serde_json::Value` based upon the path in this | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated
can have its own section.