Continuous if sentence

[Aimeast]

A bit of changes.

[nulltoken]

This loop scares me.

How about this? (Patch to be applied on top of your commit)

diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs
index 90b567f..3cb4e8f 100644
--- a/LibGit2Sharp/RepositoryExtensions.cs
+++ b/LibGit2Sharp/RepositoryExtensions.cs
@@ -374,7 +374,14 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
             {
                 singleReturnValue = DereferenceToCommit(repo, ((Reference) identifier).CanonicalName);
             }
-            else if (identifier is IEnumerable)
+
+            if (singleReturnValue != null)
+            {
+                yield return singleReturnValue;
+                yield break;
+            }
+
+            if (identifier is IEnumerable)
             {
                 foreach (object entry in (IEnumerable)identifier)
                 {
@@ -387,16 +394,11 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
                 yield break;
             }

-            if (singleReturnValue != null)
-            {
-                yield return singleReturnValue;
-                yield break;
-            }
-
             if (throwIfNotFound)
             {
                 throw new LibGit2SharpException(string.Format(CultureInfo.InvariantCulture, "Unexpected kind of identifier '{0}'.", identifier));
             }
+
             yield return null;
         }

@@ -408,10 +410,6 @@ internal static IEnumerable<ObjectId> Committishes(this Repository repo, object
         /// <returns>An <see cref="ObjectId"/> for a commit object.</returns>
         internal static ObjectId Committish(this Repository repo, object identifier)
         {
-            while (!(identifier is string) && identifier is IEnumerable)
-            {
-                identifier = ((IEnumerable)identifier).Cast<object>().FirstOrDefault();
-            }
             return repo.Committishes(identifier, true).First();
         }
     }

I think there may exist some way to refactor this further, but I'm not sure how to do it

• while preserving the "mockability" of types
• without over-engineering it

@dahlbyk Thoughts?

[Aimeast]

I think there may exist some way to refactor this further, but I'm not sure how to do it

something like table driven in CommitFilter ?

[nulltoken]

I think there may exist some way to refactor this further, but I'm not sure how to do it

something like table driven in CommitFilter ?

I was indeed thinking about some Dictionary<Type, Func> based something, but I'm afraid this would also require a specific IComparer to allow comparison of derived/mocked types...

[nulltoken]

@Aimeast Anyway, unless someone comes up with a brilliant and simple idea, let's have this merged. Could you please squash the commits and maybe improve the commit message (eg. Internal refactoring)

[dahlbyk]

Extracting SingleCommittish (a41f87a) is a simpler way to achieve the else if change here.

And orthogonal to the short-circuiting, 15e5426 includes an OO-ish way to move some of this logic down into the various committish types. With a bit more cleverness that interface could probably be adjusted such that it would apply to Commit and Branch as well.

[nulltoken]

Anyway, unless someone comes up with a brilliant and simple idea

@dahlbyk made this happen with #667. Nice!

[Aimeast]

@dahlbyk made this happen with #667. Nice!

👍

How about close this PR?

[nulltoken]

How about close this PR?

Done. Long live #667

[nulltoken]

Hmmm.. Looks like #667 may require some more time/thought.

@Aimeast Could you please resurrect your branch, squash your commits so that we can merge this meanwhile?

[dahlbyk]

@Aimeast Could you please resurrect your branch, squash your commits so that we can merge this meanwhile?

No need, I've dropped the extra commit from #667

[nulltoken]

@dahlbyk ❤️