Repository navigation
Commit.getDiff is backwards? #368
Description
Using commit.getDiff in a recent project, and the diff I'm getting appears to be the reverse of what is expected. It would seem that this line...
return thisTree.diff(parentTree);should in fact be
return parentTree(thisTree);Mostly because, in my expectation, getDiff would give me the changes made in that commit from the previous, not the changes required to undo the commit.
Basically, I believe the current method is performing something like
git diff sha..parentwhen I would expect
git diff parent..shaAm I reading this right, or am I just going crazy? If I'm right, I can put in a PR to fix, I just wanted to make sure I'm understanding before I put in the change.
Activity
johnhaley81 commentedon Jan 26, 2015
Hah, oh man... Yeah, that's exactly what it's doing. So it originally was this way but some people (including me) thought that it was backwards so we flipped it to it's current state in #348. I'm trying to find the conversation we had about it but I can't seem to either in the issues, the PRs and the gitter channel. @kmctown do you remember where we talked about that?
Regardless, maybe we should have a better named function? Or perhaps it could be a flag? I'd really like to find why we decided what we did because now it doesn't seem right at all.
orderedlist commentedon Jan 26, 2015
Oh oh oh ... so yeah, it's wrong (per the above expectations) in the latest release, but fixed in master. I'll just pay attention to the next release and update.
kmctown commentedon Jan 26, 2015
@johnhaley81 We didn't actually talk about it, I just added it to the PR. Spotted it as I was working on the workdir diffs.
johnhaley81 commentedon Jan 26, 2015
Cool! So the next update (0.3.0) might be awhile :(
The biggest thing is that the change to authentication in libgit2. That's going to break how we do the auth now and will have to be fixed up. I'd love a hand if you have time :D
johnhaley81 commentedon Jan 26, 2015
@kmctown ok, it must have been maxkorp and I then. I remember talking to somebody about it sometime though. Thanks man!
kmctown commentedon Jan 26, 2015
Yeah, basically it's desirable to diff the
sha..parent. If the target sha has for example 5 lines added, then those lines were not in the parent, therefore +5 lines. If diffingparent..sha, and the sha adds 5 lines, the diff would report 5 lines deleted, which is not great :)I thought I was crazy too when I first encountered it, but was convinced enough to push the fix.