-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Reorganize the various encodings in libextra #8310
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
Comments
I'm probably missing something here, but how would the |
That is a good point yeah, I suppose it would look something like: // src/libextra/encoding/base64.rs
struct Base64;
impl encoding::Text for Base64 { ... }
// src/libextra/encoding/hex.rs
struct Hex;
impl encoding::Text for Hex { ... }
// src/libextra/encoding/mod.rs
impl SomeHelperTrait for &'self str {
fn decode<E: Text>(self) -> ~[u8] {
Text::decode(self)
}
}
impl SomeHelperTrait for &'self [u8] {
fn encode<E: Text>(self) -> ~str {
Text::encode(self)
}
} Albeit that is a bit roundabout, and to use those methods you'd have to both use Regardless though I think the reorganization would be nice (into an |
Looking in I think there will need to be some changes made to rustdoc before this is a viable proposal. Submodules are currently only shown at the bottom of the table of contents of their super model (e.g. |
I doubt whether we should unify "bytes to strings and back" ones and "generic serialization format" ones into one modules, say (Summary: Base64 handles octets but JSON does numbers, strings, arrays, etc.) On the one hand, both share the same essential concept and use case: transforming data and they are used for e.g. saving data in a file, sending data through the Internet, etc. On the other hand, "bytes to strings and back" ones assume that there is already a representation of data and transform it but "generic serialization format" ones handle objects in the environment where a program is running: of course they may be represented as sequences of bytes (or words) in the real world but in theory they are abstract objects. |
The name |
Out of a lack of impetus and enthusiasm for this, I'm going to close this. We can revisit this if it becomes a problem later. |
Remove unnecessary `Symbol` stringification This should (slightly) improve performance and enhance code quality. changelog: none
Currently (pending #8287), libextra will have the following encodings
And in theory we should add these in the future:
(or at least those are the ones I could think of off the top of my head). Right now they're all placed directly inside the
libextra
folder, but they would probably benefit from a better organization scheme.Currently all of the "encodings" can generally fall into two categories. One is "bytes to strings and back" and the other is a "generic serialization format".
Right now I believe that the "generic serialization format" is captured nicely via the
serialize::{Encoder, Decoder}
traits, and it unifies json/ebml. @omasanori has brought up concerns about the naming scheme, and my proposal below might alleviate these concerns.Otherwise, the "bytes to strings and back" type of encoding isn't very well unified at this point. There should be a trait (in possibly a new
extra::encoding
module) which unifies base64/base32/hex and whatever else we come across. In my opinion, the trait should look like:This makes it clear what the "encode" procedure is and what the "decode" procedure is. Additionally, it's explicitly clear that bytes live on one side, and strings live on the other, there's no intermingling between the two of them.
Finally, I think we should move everything around into:
extra::encoding::base64
extra::encoding::base32
extra::encoding::hex
extra::encoding::json
extra::encoding::ebml
And then
encoding/mod.rs
would define the traitsText
(is the name too short? if you reference it byencoding::Text
I think it's clear but that's not always guaranteed)Serializer
(serialize::Encoder
today)Deserializer
(serialize::Decoder
today)What do others think about this? If this looks good, I'd be more than happy to reorganize everything!
The text was updated successfully, but these errors were encountered: