Add GitOid::new_from_url and TryFrom.
#33
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds the ability to try to construct a `GitOid` from a `url::Url` by parsing the parts of the `Url` to ensure they contain all the required data, and then constructing a `GitOid` if they do. This includes additions to enable parsing `ObjectType` and `HashAlgorithm` from a string slice, plus new error variants (with either `String` or `Box<Url>` fields) to represent different modes of failure for parsing. This commit also renames `GitOid::uri` to `GitOid::url`. The new name is a more-specific explanation of what we're dealing with, and matches the type returned (`url::Url`). This change also makes `GitOid::url` consistent with `GitOid::new_from_url`. I also added new tests to validate that the correct errors are produced for different types of invalid `Url`s. Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
alilleybrinker
added
type: feature
This commit removes the boxing around `Url` in some error variants. I'd added that when I was expecting the error variants might be much larger than the others, but I don't really think it's an issue now, so removing the boxing is preferable. Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
nellshamrell
approved these changes
Sep 29, 2022
Collaborator
nellshamrell
left a comment
nellshamrell
left a comment
There was a problem hiding this comment.
This looks great! I always love getting a code contribution that I learn new things from (I may have been aware of the eprintln macro to io::stderr at some point, but it had escaped my memory and I'm glad to have it there again!) I also appreciate how you thought through the various potential error situation and created custom error text for them. There will likely be more that we discover, and this sets up a great foundation for adding them.
| /// The expected and actual length of the data being read didn't | ||
| /// match, indicating something has likely gone wrong. | ||
| BadLength { expected: usize, actual: usize }, | ||
| /// Tried to construct a `GitOid` from a `Url` with a scheme besides `gitoid`. |
Collaborator
There was a problem hiding this comment.
I appreciate all these error variations!
Collaborator
|
Merging and cutting a release :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds the ability to try to construct a
GitOidfrom aurl::Urlby parsing the parts of theUrlto ensure they contain all the required data, and then constructing aGitOidif they do.This includes additions to enable parsing
ObjectTypeandHashAlgorithmfrom a string slice, plus new error variants (with eitherStringorBox<Url>fields) to represent different modes of failure for parsing.This commit also renames
GitOid::uritoGitOid::url.The new name is a more-specific explanation of what we're dealing with, and matches the type returned (
url::Url). This change also makesGitOid::urlconsistent withGitOid::new_from_url. I also added new tests to validate that the correct errors are produced for different types of invalidUrls.Signed-off-by: Andrew Lilley Brinker alilleybrinker@gmail.com