Added conformance to Hashable #19
Open
Conversation
This file contains 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
Before it was performing the same job as `testInitialize_MIMEType` and `testInitialize_filenameExtension`. Now, it actually tests the requirements of the Equatable protocol (and equivalence relations, more generally).
|
@cockscomb Hi Hiroki, could you please review this PR? |
cockscomb
requested changes
Dec 5, 2019
| // `Equatable` are logically valid, we need to assert that `==` and `UTTypeEqual` behave | ||
| // equally | ||
| let expected = UTTypeEqual(lhs.utiString as CFString, rhs.utiString as CFString) | ||
| let actual = lhs.utiString == rhs.utiString |
There was a problem hiding this comment.
A quick check shows that UTTypeEqual is not a simple string match. It seems to be case-insensitive.
UTTypeEqual("public.image" as CFString, "public.image" as CFString) // true
UTTypeEqual("public.image" as CFString, "PUBLIC.IMAGE" as CFString) // trueHowever, I couldn't find any documentation on how it was implemented.
There was a problem hiding this comment.
Getting a canonical identifier from the declaration may work. You also need to fall back to utiString for dynamic UTI.
extension UTI: Hashable {
public func hash(into hasher: inout Hasher) {
hasher.combine(self.declaration.identifier ?? self.utiString.lowercased())
}
}There was a problem hiding this comment.
Hey, thanks for getting back on this! Really busy right now, but I'll fix these up when I get a chance and repsuh!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
And added related tests, improved Equatable tests, and fixed up some minor grammar mistakes