-
Notifications
You must be signed in to change notification settings - Fork 14k
Implement ByteStr and ByteString types
#135073
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
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
b539a8c to
f035281
Compare
This comment has been minimized.
This comment has been minimized.
|
Well, that's definitive. This is ready for review. |
BurntSushi
left a comment
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 awesome!
Was there discussion on ByteStr verus BStr? I guess the latter is a bit more opaque, so I think ByteStr is probably the right choice for std. I do like the succinctness of BStr though. (And similarly for ByteString versus BString.)
|
Could an |
Thank you!
I like the succinctness of BStr and BString as well, but others in the libs-api meeting felt those were too opaque. The deciding factor that led me to not push further for the shorter names was that the longer names don't conflict with the bstr crate, making it safer to (for instance) place them in the prelude in the future, and making it easier for crates that need to provide impls for both. |
Sure, I can do that. |
|
I noticed that |
|
@Skgland No objection to adding that, but that would be a complex change on top of this, so I would prefer to defer it to another PR. |
|
@bors r+ |
Approved ACP: rust-lang/libs-team#502
Tracking issue: #134915
These types represent human-readable strings that are conventionally,
but not always, UTF-8. The
Debugimpl prints non-UTF-8 bytes usingescape sequences, and the
Displayimpl uses the Unicode replacementcharacter.
This is a minimal implementation of these types and associated trait
impls. It does not add any helper methods to other types such as
[u8]or
Vec<u8>.I've omitted a few implementations of
AsRef,AsMut,Borrow,From, andPartialOrd, when those would be the second implementationfor a type (counting the
Timpl) or otherwise may cause inferencefailures. These impls are important, but we can attempt to add them
later in standalone commits, and run them through crater.
In addition to the
bstrfeature, I've added abstr_internalsfeaturefor APIs provided by
corefor use byallocbut not currentlyintended for stabilization.
This API and its implementation are based heavily on the
bstrcrateby Andrew Gallant (@BurntSushi).
r? @BurntSushi