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.

Sign up for GitHub

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

Jump to bottom

Handle misformatted versions with a nicer error message. #53

Merged
merged 1 commit into from
Jun 25, 2014
Merged

Handle misformatted versions with a nicer error message. #53

merged 1 commit into from
Jun 25, 2014

Conversation

huonw
Copy link
Member

@huonw huonw commented Jun 25, 2014

Fixes #61.

@huonw
Copy link
Member Author

huonw commented Jun 25, 2014

I'm not really a fan of how this turned out... theoretically this wants to be just using a decodable instance for Version (or a wrapper around version), but that would basically have to be parsing a string, and I think it would hit rust-lang/rust#15036 (that is, it would mean the Decodable impl for TomlProject needs a more precise error type to be able to return the version parsing error, not just the generic E in impl<E, D: Decoder<E>> ..., meaning #[deriving(Decodable)] wouldn't work).

@huonw huonw mentioned this pull request Jun 25, 2014
Closed
@alexcrichton
Copy link
Member

Could you add an integration test as well as part of the tests folder? Just to make sure the output from cargo build on a bad url/version is somewhat sane.

@alexcrichton
Copy link
Member

Just waiting on travis for this now...

@huonw huonw mentioned this pull request Jun 25, 2014
Closed
@alexcrichton
Copy link
Member

Thanks!

alexcrichton added a commit that referenced this pull request Jun 25, 2014
@alexcrichton
Merge pull request #53 from huonw/version-error
5971c3a 
Handle misformatted versions with a nicer error message.
@alexcrichton alexcrichton merged commit 5971c3a into rust-lang:master Jun 25, 2014
@huonw huonw deleted the version-error branch June 25, 2014 13:30
steveklabnik
steveklabnik reviewed Jun 25, 2014
View reviewed changes
src/cargo/core/package_id.rs

trait ToVersion {
fn to_version(self) -> Option<semver::Version>;
fn to_version(self) -> Result<semver::Version, String>;
Copy link
Member

Choose a reason for hiding this comment

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

I think this pattern will become quite common in Rust code.

alexcrichton added a commit to alexcrichton/cargo that referenced this pull request Sep 2, 2014
@alexcrichton
Merge pull request rust-lang#53 from huonw/version-error
ff4c189 
Handle misformatted versions with a nicer error message.
@asomers asomers mentioned this pull request Aug 28, 2016
Closed
ehuss pushed a commit to ehuss/cargo that referenced this pull request Nov 19, 2023
@bors
Merge rust-lang#53
a71ff8a 
53: Add test for ambiguous `<` r=oli-obk a=killercup

Inspired by https://twitter.com/b0rk/status/954366146505052160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

version parse error
3 participants
@huonw @alexcrichton @steveklabnik