-
Notifications
You must be signed in to change notification settings - Fork 899
Add revwalk globs #184
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 revwalk globs #184
Conversation
@@ -98,7 +98,6 @@ public void QueryingTheCommitHistoryWithUnknownShaOrInvalidEntryPointThrows() | |||
{ | |||
Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new Filter { Since = Constants.UnknownSha }).Count()); | |||
Assert.Throws<LibGit2SharpException>(() => repo.Commits.QueryBy(new Filter { Since = "refs/heads/deadbeef" }).Count()); | |||
Assert.Throws<ArgumentNullException>(() => repo.Commits.QueryBy(new Filter { Since = null }).Count()); |
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.
Why removing this? What should the following statements be expected to return?
repo.Commits.QueryBy(new Filter { Since = null })
repo.Commits.QueryBy(new Filter { Since = string.Empty })
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.
Good catch - see aec0563
I like the refactoring proposal which pushes the logic inside the |
Rather than adding a This would allow such construct [Fact]
public void CanEnumerateCommitsUsingGlob()
{
AssertEnumerationOfCommits(
repo => new Filter { Since = repo.Refs.FromGlob("heads") },
new[]
{
"4c062a6", "e90810b", "6dcf9bf", "a4a7dce", "be3563a", "c47800c", "9fd738e", "4a202b3", "41bc8c6", "5001298", "5b5b025", "8496071"
});
} |
@nulltoken isn't that going to be expensive in terms of having to lookup all the refs that match the glob pattern? Wouldn't you rather have the existing libgit2 apis for pushing and hiding a glob do that? |
Well, behind the scene, this is what git_revwalk_push_glob() does. I'd really like to have this unfolded and allow the API consumer to retrieve a list of refs from a glob, which could be eventually used to retrieve a commit log (or used to a different end). The fast way would be to quickly implement that in C# on top of the current Refs enumerator (porting the libgit2 normalization algorithm and regex love). If you feel performance may become a concern, instead of the Refs enumerator, one could work against the result of Eventually, once this code is proven ok and covered with some tests, it'll be pushed down in libgit2. |
Very nice proposal and awesome refactoring! Manually merged in vNext. |
Awesome! Thanks @nulltoken! |
This adds in the ability to filter your revwalk by pushing or hiding globs enabling functionality like:
git log --not --remotes
orgit log --branches
I'm open to suggestions on the filter API. The glob patterns didn't make sense to add in with the rest of the list of stuff that
Since
andUntil
know how to deal with because they are never really git objects of any sort. Improving theFilter
object gave me the chance to refactor how it was use internally a little bit and pass the filter down a few more layers to get rid of a few ctors with growing parameter lists.