Skip to content
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

Replace Vec<Ascii> and [Ascii] with newtypes #6

Merged
merged 2 commits into from
Apr 7, 2015
Merged

Conversation

tomprogrammer
Copy link
Owner

The ascii crate now provides types for owned ascii strings and borrowed
ascii slices. Their implementation should allow them to be used like
String and str.

Changes:

  • AsciiString replaces Vec<Ascii>
  • AsciiStr replaces [Ascii]
  • Remove Ascii::to_lowercase() and Ascii::to_uppercase()
  • Remove traits IntoString, IntoBytes and AsciiStr

Migration (for details see new implementations):

  • You can't construct AsciiString and AsciiStr like a collection
    anymore, use the from_bytes or from_str method instead.
  • FromStr::from_str for AsciiString and AsciiStr::from_str for
    construction from string slices.
  • To convert from the ascii types to bytes or strings please use the
    new generic conversion traits Into and AsRef instead of IntoString, IntoBytes or AsciiStr.
  • Use AsciiExt::to_ascii_lowercase() and AsciiExt::to_ascii_uppercase() instead of the removed methods which where implemented directly on Ascii.
  • The AsciiCast and OwnedAsciiCast traits are unchanged for now.

New implementations:

  • AsciiString::from_bytes and AsciiStr::from_bytes for construction from
    types which can be represented as a byte slice.
  • Implements fmt::{Debug, Display} like String does
  • AsciiString derefs to AsciiStr, additionally DerefMut is implemented so
    AsciiExt is fully useable
  • Implements Hash, Borrow and ToOwned so it can easily be used in
    collections.
  • Implements PartialEq, PartialOrd, Eq and Ord for itself and
    str
  • AsciiString implements Into<String>, Into<Vec<u8>> and
    AsRef<AsciiStr>
  • AsciiStr implements AsRef<[u8]> and AsRef<str>

[breaking-change]

Fixes: #5

As of now documentation and more tests for the new functionality are missing.

@tomprogrammer
Copy link
Owner Author

I don't know if I missed to implement important traits. Due to the missing tests I'm not sure if everything works as it should.

@frewsxcv Do you want to test this in your code for feedback?

@frewsxcv
Copy link

frewsxcv commented Apr 3, 2015

So I'm a little confused, why did all of this stuff get changed?

@tomprogrammer
Copy link
Owner Author

I planned to replace Vec<Ascii> and [Ascii] with custom types some time ago because it was not possible to impl fmt::Debug for Vec<Ascii> in a custom way, due to conflicting generic impl's in std::collections.

Now the new coherence rules also made it difficult to do something like impl AsciiExt for Vec<Ascii> because Vec<Ascii> is not considered a crate local type. Custom crate local types are an easy way to solve this. Custom ascii types do have the advantage, that they can be used like String/str so it's easier to use them. There is another set of types following this: OsString/OsStr

The new generic conversation traits just replaced some specialized traits in this library, similar to what happened in std. I just found some traits I want to implement additionally like FromIterator, Extend and so on.

}
}

impl PartialEq for AsciiString {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use derive(PartialEq) for this?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, also Eq, PartialOrd, Ord and Hash can be derived on AsciiString. On AsciiStr only Hash can be derived because of rust-lang/rust#23649.

@frewsxcv
Copy link

frewsxcv commented Apr 4, 2015

Looks alright with me

@tomprogrammer tomprogrammer force-pushed the rust-head branch 3 times, most recently from dec4b19 to 50bc785 Compare April 4, 2015 22:45
@frewsxcv
Copy link

frewsxcv commented Apr 6, 2015

@tomprogrammer Need any help with getting the tests to pass?

@tomprogrammer
Copy link
Owner Author

@frewsxcv Yes, that would be great. I can't work on it at the moment, exams want to be done.

I learned that the types PartialEq is implemented for must match, there is no auto-dereference. That confused me at beginning.

@tomprogrammer
Copy link
Owner Author

I've decided not to throw away Ascii::to_lowercase() and Ascii::to_uppercase() because AsciiExt isn't stable in beta 1.0 yet. Actually this preserves the functionality if we ship a beta 1.0 compatible build of this crate.

Thomas Bahn added 2 commits April 7, 2015 23:29
The ascii crate now provides types for owned ascii strings and borrowed
ascii slices. Their implementation should allow them to be used like
`String` and `str`.

Changes:
 - `AsciiString` replaces `Vec<Ascii>`
 - `AsciiStr` replaces `[Ascii]`
 - Remove traits `IntoString`, `IntoBytes` and `AsciiStr`

Migration (for details see new implementations):
 - You can't construct `AsciiString` and `AsciiStr` like a collection anymore,
   use the `from_bytes` or `from_str` method instead.
 - To convert from the ascii types to bytes or strings please use the new
   generic conversion traits `Into` and `AsRef` instead of `IntoString`,
   `IntoBytes` or `AsciiStr`.
 - The AsciiCast and OwnedAsciiCast traits are unchanged for now.

New implementations:
 - `AsciiString::from_bytes` and `AsciiStr::from_bytes` for construction from
   types which can be represented as a byte slice.
 - `FromStr::from_str for AsciiString` and `AsciiStr::from_str` for
   construction from string slices.
 - Implements `fmt::{Debug, Display}` like `String` does
 - `AsciiString` derefs to `AsciiStr`, additionally `DerefMut` is implemented
   so `AsciiExt` is fully useable
 - Implements `Hash`, `Borrow` and `ToOwned` so it can easily be used in
   collections.
 - Implements `PartialEq`, `PartialOrd`, `Eq` and `Ord`.
 - `AsciiString` implements `Into<String>`, `Into<Vec<u8>>` and
   `AsRef<AsciiStr>`.
 - `AsciiStr` implements `AsRef<[u8]>` and `AsRef<str>`.

[breaking-change]
@SimonSapin
Copy link
Collaborator

Hum, AsciiExt and AsciiExt::to_*case are stable. It’s OwnedAsciiExt that’s not, because it’s meant to be replaced by AsciiExt::make_*case but that requires String to implement DerefMut to be useful. (AsciiExt::make_* take &mut str)

@tomprogrammer
Copy link
Owner Author

oh, that is correct. I screwed something up in my head.

Sorry for the frequent breakage, I update ascii again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not compile with the latest Rust
3 participants