-
Notifications
You must be signed in to change notification settings - Fork 14
Add UID handling with base tests #53
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
Add UID handling with base tests #53
Conversation
Add support for UID type in plist encoding/decoding - Support UID type to handle unique object identifiers in binary plists, used by CoreFoundation Keyed Archiver. - binary parsing, decoding, encoding, and XML writing logic to support the UID values - very basic tests for UID marshaling
|
Thanks! I'll take a deeper look later. I think the other folks did this a fair bit ago: DHowett/go-plist@e9d7568. Also DHowett/go-plist#54 is interesting and possibly related (and alternate encoding?). On super duper quick glance I'd wonder about the specific |
Add a test for TestDecodeUIDInvalidSize, load from testdata file
|
I gave this a full review and like the approach so far. @headmin two things I'd like to see:
@jessepeterson, I think a separate UID type is the right approach. That's how python does it. UID has special handling separate from a raw int/number, both in the binary and XML formats. |
Modified `parseDict`, will detect CF$UID dicts and return as `UID` values Update encode_test.go with tests
|
@korylprince added as requested, please have a look |
Yeah thinking about it again this seems reasonable for encoding. Though you could have an additional field tag to indicate a more intricate type as an alternative strategy. For decoding I'm not convinced it makes much difference. Separately the type naming doesn't quite seem consistent. UIDKind vs. UID doesn't quite look/feel like the rest of the code. But really more of a nit than anything. |
Replaces the UIDKind constant and references to CFUID.
|
@jessepeterson you can review again |
korylprince
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, not sure if @jessepeterson still wants to review.
Add initial support for UID type in plist encoding/decoding