Merge pull request #798 from ccutrer/commit_diff_default_args

fix default arguments for Commit#diff when nothing is passed

Author: carlosmn

lib/rugged/commit.rb

@@ -19,10 +19,24 @@ def header_field?(field)
 
     # Return a diff between this commit and its first parent or another commit or tree.
     #
+    # The commit is treated as the new side of the diff by default
+    #
     # See Rugged::Tree#diff for more details.
     def diff(*args)
-      args.unshift(parents.first) if args.size == 1 && args.first.is_a?(Hash)
-      self.tree.diff(*args)
+      raise ArgumentError("wrong number of arguments (given #{args.length}, expected 0..2") if args.length > 2
+      other, opts = args
+      if other.is_a?(Hash)
+        opts = other
+        other = nil
+      end
+      opts ||= {}
+      # if other is not provided at all (as opposed to explicitly nil, or given)
+      # then diff against the prior commit
+      if args.empty? || args.first.is_a?(Hash)
+        other = parents.first
+        opts[:reverse] = !opts[:reverse]
+      end
+      self.tree.diff(other, opts)
     end
 
     # Return a diff between this commit and the workdir.

test/diff_test.rb

@@ -217,23 +217,50 @@ def test_diff_with_parent
     assert_equal 5, deltas.size
     assert_equal 5, patches.size
 
-    assert_equal 0, deltas.select(&:added?).size
-    assert_equal 3, deltas.select(&:deleted?).size
+    assert_equal 3, deltas.select(&:added?).size
+    assert_equal 0, deltas.select(&:deleted?).size
     assert_equal 2, deltas.select(&:modified?).size
 
     assert_equal 4, hunks.size
 
     assert_equal(51, lines.size)
     assert_equal(2, lines.select(&:context?).size)
-    assert_equal(3, lines.select(&:addition?).size)
-    assert_equal(46, lines.select(&:deletion?).size)
+    assert_equal(46, lines.select(&:addition?).size)
+    assert_equal(3, lines.select(&:deletion?).size)
+  end
+
+  def test_diff_with_parent_no_options
+    repo = FixtureRepo.from_libgit2("attr")
+    commit = Rugged::Commit.lookup(repo, "605812a")
+
+    diff = commit.diff
+
+    deltas = diff.deltas
+    patches = diff.patches
+    hunks = patches.map(&:hunks).flatten
+    lines = hunks.map(&:lines).flatten
+
+    assert_equal 5, diff.size
+    assert_equal 5, deltas.size
+    assert_equal 5, patches.size
+
+    assert_equal 3, deltas.select(&:added?).size
+    assert_equal 0, deltas.select(&:deleted?).size
+    assert_equal 2, deltas.select(&:modified?).size
+
+    assert_equal 4, hunks.size
+
+    assert_equal(53, lines.size)
+    assert_equal(4, lines.select(&:context?).size)
+    assert_equal(46, lines.select(&:addition?).size)
+    assert_equal(3, lines.select(&:deletion?).size)
   end
 
   def test_diff_with_parent_for_initial_commit
     repo = FixtureRepo.from_libgit2("attr")
     commit = Rugged::Commit.lookup(repo, "6bab5c79cd5140d0f800917f550eb2a3dc32b0da")
 
-    diff = commit.diff(:context_lines => 1, :interhunk_lines => 1, :reverse => true)
+    diff = commit.diff(:context_lines => 1, :interhunk_lines => 1)
 
     deltas = diff.deltas
     patches = diff.patches