rugged icon indicating copy to clipboard operation
rugged copied to clipboard

Is Rugged::Commit.diff right?

Open yehaha9876 opened this issue 9 years ago • 1 comments

I use walker find a commit but when i get it`s diff, i think the result is wrong.

rugged/lib/rugged/commit.rb

    def diff(*args)
      args.unshift(parents.first) if args.size == 1 && args.first.is_a?(Hash)
      self.tree.diff(*args)
    end

rugged/lib/rugged/tree.rb

    def diff(other = nil, options = nil)
      Tree.diff(repo, self, other, options)
    end

but rugged/ext/rugged/rugged_tree.c

 *  call-seq:
 *    Tree.diff(repo, tree, diffable[, options]) -> diff
 *
 *  Returns a diff between the `tree` and the diffable object that was given.
 *  +diffable+ can either be a +Rugged::Commit+, a +Rugged::Tree+, a +Rugged::Index+,
 *  or +nil+.
 *
 *  The +tree+ object will be used as the "old file" side of the diff, while the
 *  parent tree or the +diffable+ object will be used for the "new file" side.

so all seams right, but as the comment say The +tree+ object will be used as the "old file" side of the diff, while the parent tree or the +diffable+ object will be used for the "new file" side, is parent used as new side right?

yehaha9876 avatar Jan 05 '16 09:01 yehaha9876

Yup. I just ran into this. First of all, it was broken completely if you didn't pass either an other or an options. But it's still counter-intuitive that calling commit.diff will return a diff of that commit being reverted, not applied. Worse still, if you get the commit.diff of the first commit, it somehow automatically swaps things, and returns a forward diff (as you would expect). https://github.com/libgit2/rugged/pull/798 fixes both issues - you can call it with no args, while preserving the current behavior if you explicitly pass something to compare it to (especially since this is how Repository#diff is implemented)

ccutrer avatar May 24 '19 18:05 ccutrer