-
Notifications
You must be signed in to change notification settings - Fork 3
ABI-compatible and fixed-capacity strings #28
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
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
src/containers/generic/string.rs
Outdated
| /// | ||
| /// If the string has sufficient spare capacity, the operation succeeds and a reference to the new characters is returned; | ||
| /// otherwise, `Err(InsufficientCapacity)` is returned. | ||
| pub fn push_str(&mut self, other: &str) -> Result<&mut str, InsufficientCapacity> { |
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.
why we return &mut str instead () ?
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.
This is for consistency with the vector and queue which return &mut T, and where push-element-then-modify-by-reference can sometimes be quite useful. For a string this is probably rarely useful, so if you think it's confusing, I can change it to ().
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.
ye I would remove it, very inlinke usage
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.
Done
| } | ||
|
|
||
| /// Returns the current number of bytes in the string. | ||
| pub fn len(&self) -> usize { |
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.
Shall be more precise in all doc this is not char and it's not what one can think it is. IE 😄 would be 1 but its 4 i guess.
So it's not printable len at the end.
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.
Done
267b65e to
9bea0d8
Compare
|
@awillenbuecher-xq-tec we can rebase and ask @4og to merge if no more reviewers willing to review |
"InsufficientCapacity" is a more precise description than "VectorFull" or similar error names.
9bea0d8 to
f647dc7
Compare
|
Rebased onto |
This PR adds two data structures to the Rust base libs: an inline-storage (ABI-compatible) and a fixed-capacity UTF-8 string. Refer to the feature request for more information about ABI compatibility.
It also adds fallible
try_new()methods to the heap-allocated containers to provide non-panicking constructors.Pre-Review Checklist for the PR Author
Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References