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
Implement file history walk in revwalk #889
Conversation
https://github.com/nodegit/nodegit/blob/master/examples/walk-history-for-file.js needs to be updated as well please. |
0821662
to
b873633
Compare
@tbranyen @johnhaley81 once we get this to pass without some random test timing out, this is ready to go by my account. If you guys wanted to give this the once over before I merge in, that would be greatly appreciated. Notes of interest: The fileHistoryWalk will do a rename detection if it discovers a file was added in the history and will return the old file name and the renamed status along with the commit. This helps history users to not only follow the filepath's history, but also see how the content if the file has changed / branched. |
break; | ||
} | ||
|
||
git_diff *diffs; |
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.
I don't see these getting freed anywhere. Is this memory cleaned up?
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.
I can't find whether or not we're supposed to clean it up or not. It would be super helpful if the docs gave me some insight
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.
If you know explicitly that we're supposed to clean those up, I'll free the. I've been mulling it over myself, because the docs are not clear!
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.
Yes we do need to free it: https://github.com/libgit2/libgit2/blob/v0.23.4/include/git2/diff.h#L23-L28
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.
oh cool! I guess since it's explained there they don't have to explain it in the function documentation for tree to tree.
Other then the potential memory leak above, could you get some comment blocks into |
Only find similar if there is an added entry Free diffs when we are done with them.
1a6486c
to
a31d1eb
Compare
} | ||
} | ||
|
||
git_diff_free(diffs); |
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.
Freeing diffs down here now.
@johnhaley81 @tbranyen what is going on with this test? https://travis-ci.org/nodegit/nodegit/jobs/106996362 |
a31d1eb
to
6338a84
Compare
6338a84
to
11ad5ae
Compare
Implement file history walk in revwalk
Implementing file history walk for revwalk. You pass it a sha, it then traverses until it either finds the 'added' entry for the file OR it hits the max number of commits to search for history.