Commit.getParents working with merge commits

bjornarg · bjornarg · commit 00a62f813f9e · 2015-01-19T21:44:35.000+01:00
getParents failed when more than one commit was present, since it attempted
to get the second parent of commit's parent instead of it's own second
parent.

The promise for the second parent was also pushed onto the array for
Promise.all() too late to actually have an effect.

Limit was not working properly as a limit, since it attempted to
retrieve the set limit regardless of how many parents a commit had,
failing if the limit was set to more than Commit.parentcount().
diff --git a/lib/commit.js b/lib/commit.js
@@ -116,7 +116,6 @@ Commit.prototype.history = function() {
  */
 Commit.prototype.getParents = function(limit, callback) {
   var parents = [];
-  var i = 0;
 
   // Shift arguments.
   if (typeof limit === "function") {
@@ -125,28 +124,13 @@ Commit.prototype.getParents = function(limit, callback) {
 
   // If no limit was set, default to the maximum parents.
   limit = typeof limit === "number" ? limit : this.parentcount();
+  limit = Math.min(limit, this.parentcount());
 
-  function processParents(commit, callback) {
-    var oid = commit.parentId(i);
+  for (var i = 0; i < limit; i++) {
+    var oid = this.parentId(i);
+    var parent = this.repo.getCommit(oid);
 
-    var parent = commit.repo.getCommit(oid).then(function(parent) {
-      if (--limit) {
-        i = i + 1;
-        return processParents(parent, callback);
-      }
-
-      return parent;
-    });
-
-    // Add this parent to the list.
     parents.push(parent);
-
-    return parent;
-  }
-
-  // Process all the parents.
-  if (limit) {
-    processParents(this, callback);
   }
 
   // Wait for all parents to complete, before returning.
diff --git a/test/tests/commit.js b/test/tests/commit.js
@@ -215,6 +215,25 @@ describe("Commit", function() {
     });
   });
 
+  it("can specify limit higher than actual parents", function() {
+    return this.commit.getParents(2).then(function(parents) {
+      assert.equal(parents.length, 1);
+    });
+  });
+
+  it("can fetch parents of a merge commit", function () {
+    return NodeGit.Repository.open(reposPath)
+    .then(function (repo) {
+      return repo.getCommit("bf1da765e357a9b936d6d511f2c7b78e0de53632");
+    })
+    .then(function (commit) {
+      return commit.getParents();
+    })
+    .then(function (parents) {
+      assert.equal(parents.length, 2);
+    });
+  });
+
   it("has a parent count", function() {
     assert.equal(1, this.commit.parentcount());
   });
