adds len and is_empty to Pointer
#69
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
|
@asmello we had talked about adding |
| /// assert_eq!(ptr.len(), 11); | ||
| /// | ||
| /// ``` | ||
| pub fn len(&self) -> usize { |
There was a problem hiding this comment.
I think this is sane. One can always do ptr.tokens().count() for the alternative definition of "length".
I see that you've copied the doc-comment from str::len. This is mostly accurate, since we do indeed just delegate to that method, but the semantics is not quite right. I think we could go with something simpler instead, along the lines of:
/// This length expresses the byte count of the underlying string that represents the RFC 6091 Pointer. See also [`std::str::len`].
Although the caveat with char/graphemes applies, it's less important for us than the relationship with std::str.
|
Yea, that makes sense.
…On Mon, Sep 30, 2024 at 2:28 PM André Mello ***@***.***> wrote:
***@***.**** commented on this pull request.
Leaving a comment on docs we can fix later.
------------------------------
In src/pointer.rs
<#69 (comment)>:
> @@ -463,7 +463,7 @@ impl Pointer {
buf
}
- /// Creates an owned [`Pointerbuf`] like `self` but with `other` appended to
+ /// Creates an owned [`PointerBuf`] like `self` but with `other` appended to
Had to look like 5x to notice what changed, omg.
------------------------------
In src/pointer.rs
<#69 (comment)>:
> +
+ // Returns the length of `self` in encoded format.
+ ///
+ /// This length is in bytes, not [`char`]s or graphemes. In other words, it might
+ /// not be what a human considers the length of the string.
+ ///
+ /// ## Examples
+ /// ```
+ /// let mut ptr = jsonptr::Pointer::from_static("/foo/bar").to_buf();
+ /// assert_eq!(ptr.len(), 8);
+ ///
+ /// ptr.push_back("~");
+ /// assert_eq!(ptr.len(), 11);
+ ///
+ /// ```
+ pub fn len(&self) -> usize {
I think this is sane. One can always do ptr.tokens().count() for the
alternative definition of "length".
I see that you've copied the doc-comment from str::len. This is mostly
accurate, since we do indeed just delegate to that method, but the
semantics is not quite right. I think we could go with something simpler
instead, along the lines of:
/// This length expresses the byte count of the underlying string that represents the RFC 6091 Pointer. See also [`std::str::len`].
Although the caveat with char/graphemes applies, it's less important for
us than the relationship with std::str.
—
Reply to this email directly, view it on GitHub
<#69 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIGFZEM26LKUPGFQ4RXY4DZZGJ5JAVCNFSM6AAAAABPDP5GZWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZYGI4DSOBYGA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
|
Ugh, I accidentally added them to both It would be a breaking change to delete them from |
I did wonder. I don't think it is, because |
Adds
lenandis_emptytoPointer.