Revparse #200

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@ben
Member
ben commented Aug 2, 2012

Fixes #199

This adds rev-parse functionality to libgit2sharp methods which take a shaOrReferenceName parameter, making them much more powerful.

@travisbot

This pull request fails (merged 11a912df into c191d7e).

@travisbot

This pull request fails (merged e31a5d35 into c191d7e).

@travisbot

This pull request fails (merged 50bf147c into c191d7e).

@travisbot

This pull request fails (merged 650f395c into c191d7e).

@travisbot

This pull request passes (merged af54a01e into c191d7e).

@travisbot

This pull request passes (merged d37a5884 into c191d7e).

@travisbot

This pull request passes (merged be8589c6 into c191d7e).

@ben
Member
ben commented Aug 4, 2012

@nulltoken I think this is ready for review. Was there anything else you wanted from this feature?

@travisbot

This pull request passes (merged 4d649ab8 into c191d7e).

@nulltoken nulltoken and 1 other commented on an outdated diff Aug 5, 2012
LibGit2Sharp/Repository.cs
/// <param name = "type">The kind of <see cref = "GitObject" /> being looked up</param>
/// <returns>The <see cref = "GitObject" /> or null if it was not found.</returns>
- public GitObject Lookup(string shaOrReferenceName, GitObjectType type = GitObjectType.Any)
+ public GitObject Lookup(string objectSpec, GitObjectType type = GitObjectType.Any)
@nulltoken
nulltoken Aug 5, 2012 Member

Can you rename this objectish?

@ben
ben Aug 5, 2012 Member

Done! Is the documentation for this parameter okay? The git documentation doesn't say anything about rev-parse when it mentions "treeish"es or "commitish"es, but it seems to be the right thing.

@nulltoken nulltoken and 1 other commented on an outdated diff Aug 5, 2012
LibGit2Sharp/Repository.cs
{
- ObjectId id;
+ var m = Regex.Match(spec, "[^@^ ]*:(.*)");
@nulltoken
nulltoken Aug 5, 2012 Member

Maybe should we guard against ':/commitmessage' and ':2:blobname' patterns, don't you think so?

@ben
ben Aug 5, 2012 Member

I've fixed this, but there's no way to write a unit test for it. It doesn't seem like this should be a public static method for Repository. Should it go somewhere else, or just leave it without a real unit test?

@nulltoken nulltoken and 1 other commented on an outdated diff Aug 5, 2012
LibGit2Sharp/Repository.cs
- if (id == null)
+ GitObjectSafeHandle sh;
@nulltoken
nulltoken Aug 5, 2012 Member

I think this one should be .SafeDispose()d once used.

@ben
ben Aug 5, 2012 Member

I think de799b12f51a9ff7a98e18bd8d59ae428558bcc7 does this correctly. Am I wrong?

@nulltoken nulltoken and 1 other commented on an outdated diff Aug 5, 2012
LibGit2Sharp/RepositoryExtensions.cs
/// <returns></returns>
- public static T Lookup<T>(this IRepository repository, string shaOrRef) where T : GitObject
+ public static T Lookup<T>(this IRepository repository, string objectSpec) where T : GitObject
@nulltoken
nulltoken Aug 5, 2012 Member

Can you rename this objectish?

@ben
ben Aug 5, 2012 Member

Done!

@travisbot

This pull request passes (merged de799b12 into c191d7e).

@travisbot

This pull request passes (merged 3ac989b2 into c191d7e).

@nulltoken
Member

@benstraub This all looks pretty nice! Could you please rebase on top of vNext and squash this into a single commit?

Was there anything else you wanted from this feature?

I think CommitLog.QueryBy() and some of the methods of the ReferenceCollection namespace may also benefit from some revpars-ing, but let's merge this first.

@travisbot

This pull request passes (merged 0b87a4ab into 4bda9ff).

@nulltoken
Member

@benstraub

I think it'd be nice to have revparse return E_AMBIGUOUS where it makes sense. I don't know if there are other causes than a too short oid, though.

So I'd tend to add a Skip alteration to the Fact attribute.

I add the revparse fix to my Todo list.

@ben Ben Straub Revparse support in Repository.Lookup.
Now running lookups through git_revparse_single.
8c39bd3
@ben
Member
ben commented Aug 14, 2012

I've added the E_AMBIGUOUS topic to my TODO list. :)

Squash complete. Merge at will. 😺

@travisbot

This pull request passes (merged 8c39bd3 into 4bda9ff).

@nulltoken
Member

Boom! Merged into vNext.

First, there was nothing. Then native... And now, managed... Awesome job! ❤️

I've added the E_AMBIGUOUS topic to my TODO list. :)

Thief! You'd better make it merged sooner rather than later, man. Otherwise, I'll find you... ;-)

@nulltoken nulltoken closed this Aug 14, 2012
@nulltoken
Member

@benstraub No more excuse ;p

diff --git a/tests-clar/refs/revparse.c b/tests-clar/refs/revparse.c
index 02acb88..14bd9fb 100644
--- a/tests-clar/refs/revparse.c
+++ b/tests-clar/refs/revparse.c
@@ -442,3 +442,12 @@ void test_refs_revparse__disambiguation(void)
     */
    test_object("e90810", "e90810b8df3e80c413d903f631643c716887138d");
 }
+
+void test_refs_revparse__a_too_short_objectid_returns_EAMBIGUOUS(void)
+{
+   int result;
+   
+   result = git_revparse_single(&g_obj, g_repo, "e90");
+   
+   cl_assert_equal_i(GIT_EAMBIGUOUS, result);
+}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment