Added two new objective c wrappers for libgit2 #435
Conversation
0x4a616e
commented
Jan 18, 2015
0x4a616e
commented
- A patch can directly be copied into a buffer using git_patch_to_buf method
- A tree entry can be looked up by path using git_tree_entry_bypath method. The found object is freed when the GTTreeEntry is deallocated.
| @@ -50,6 +50,9 @@ | |||
| /// Returns the raw size of the delta, in bytes. | |||
| - (NSUInteger)sizeWithContext:(BOOL)includeContext hunkHeaders:(BOOL)includeHunkHeaders fileHeaders:(BOOL)includeFileHeaders; | |||
|
|
|||
| /// Returns the raw patch data as buffer. | |||
| - (NSData*)toBuffer; | |||
There was a problem hiding this comment.
patchData feels like a more "conventional" name.
|
The different ownership semantics make me wonder if we should just |
|
That should solve this, I'll change it |
… respective git_tree_entry and is responsible for freeing it. Therefore, the type of the field has been changed to git_tree_entry* (without the const). To easily create a GTTreeEntry from a managed const git_tree_entry*, a factory method exists to duplicate git_tree_entry and create a new GTTreeEntry
|
Pushed a new version incorporating your feedback |
| @@ -50,6 +50,9 @@ | |||
| /// Returns the raw size of the delta, in bytes. | |||
| - (NSUInteger)sizeWithContext:(BOOL)includeContext hunkHeaders:(BOOL)includeHunkHeaders fileHeaders:(BOOL)includeFileHeaders; | |||
|
|
|||
| /// Returns the raw patch data. | |||
| - (NSData*)patchData; | |||
There was a problem hiding this comment.
Style note (here and elsewhere): space between the type and the asterisk.
|
I'd be interested in seeing what others think about this. My fear is that the current interface still makes it too easy to mess up the ownership of the tree entry. |
| @@ -64,6 +64,13 @@ typedef NS_ENUM(NSInteger, GTTreeEnumerationOptions) { | |||
| /// returns a GTTreeEntry or nil if there is nothing with the specified name | |||
| - (GTTreeEntry *)entryWithName:(NSString *)name; | |||
|
|
|||
| /// Get a entry by path | |||
There was a problem hiding this comment.
That was actually a copy-paste error ;-) I'll fix the other occurrences as well.
|
I agree with @joshaber that we should stick to one ownership semantic for this class. It's simpler, even if slightly suboptimal for some use cases. |
| git_tree_entry *entry = NULL; | ||
| int gitError = git_tree_entry_bypath(&entry, self.git_tree, path.UTF8String); | ||
| if (error != GIT_OK) { | ||
| *error = [NSError git_errorFor:gitError description:@"Failed to get tree ntry %@", path]; |
There was a problem hiding this comment.
If NULL is passed you'll crash.
|
Agreed about the ownership thing. Though the fact is that some functions return externally-owned tree entries while others don't. I propose |
But if we call |
|
What do you think about switching the interface back to |
|
Sounds good 👍 |
|
My goal with the "private" method was that if there's someone out there who uses libgit2 directly and has to wrap one of the externally-owned entries itself, they it will have to duplicate what we're doing internally. But yeah, always duping is good, as it actually will prevent some future weird crashes where the underlying owner gets freed (thus freeing entries) while there's still some |
…g initialisation. The resulting git_tree_entry is thus solely owned (and freed) by GTTreeEntry
|
In the latest version, the git_tree_entry is always duplicated. What do you think? :) |
| git_buf buf = GIT_BUF_INIT_CONST(0, NULL); | ||
| git_patch_to_buf(&buf, self.git_patch); | ||
|
|
||
| NSData* buffer = [[NSData alloc] initWithBytes:buf.ptr length:buf.size]; |
| self = [super init]; | ||
| if (self == nil) return nil; | ||
|
|
||
| git_tree_entry* copyOfEntry = nil; |
There was a problem hiding this comment.
Style: git_tree_entry *copyOfEntry
|
Awesome, thanks! I didn't realize the changes were made. Just a couple more style notes. |
|
Style changed |
|
🤘 |
Added two new objective c wrappers for libgit2
