search

FredrikNoren / ungit

The easiest way to use git. On any platform. Anywhere.
MIT License
10.45k stars 638 forks source link

Diff shown is for previous commit #545

Closed tengwar closed 9 years ago

tengwar commented 9 years ago

Diff summary is OK, but the diff itself is from a previous commit. I tried this on 2 repos. See the screenshot: ungit_diff_bug

Ungit 0.9.2 on Windows 8.1 x64.

jung-kim commented 9 years ago

I'm very sad that diff is not doing very well in recent releases... Sorry about that....

Could you do me a favor and capture and share the git command that is being executed via ungit? Two ways to do this. if you have .ungitrc you can add logGitCommands: true or you can do ungit --logGitCommands

tengwar commented 9 years ago

http://pastebin.com/qycspZm9 http://pastebin.com/0KvuYcLq

jung-kim commented 9 years ago

git diff HEAD^^! -- src/twofish/Main.java should be equivalent to git diff HEAD~1:src/twofish/Main.java HEAD:src/twofish/Main.java. I think some how wrong sha8 hash is being passed in or something.

  1. Does this happen to all commits? If not do you see any patterns for the troubled ones?
  2. Could you compare ungit commit nodes vs git log? Does commit msg and sha8 hash match?
Ajedi32 commented 9 years ago

Does HEAD^^! even make sense to use with git diff? HEAD^^! specifies a range of commits, and commit ranges can't be used with git diff as far as I know.

See: https://www.kernel.org/pub/software/scm/git/docs/gitrevisions.html#_specifying_revisions https://www.kernel.org/pub/software/scm/git/docs/git-diff.html

tengwar commented 9 years ago

It happens to all commits; message, hash and even lines added/removed count in Ungit matches with log. Diff doesn't. BTW now I have trouble getting diff at all from that repo - nothing happens when I click the bar. It works in other repo though.

jung-kim commented 9 years ago

I'm not sure if HEAD^! qualify as range. And this method hasn't been problematic yet, until this issue that is.

A suffix ^ followed by an exclamation mark is the same as giving commit <rev> and then all its parents prefixed with ^ to exclude them (and their ancestors).

@tengwar So this problem is isolated to a specific repo but work in other repos correctly? this is getting stranger...

tengwar commented 9 years ago

No, this problem happens in all repos. But I have encountered a second problem that happens in one repo: nothing happens when I click the diff bar. (It should expand and show a diff.)

I just tried on openSUSE and correct diffs are shown in all repos, so this problem seems to be Windows-only.

But that second problem seems to happen randomly - on openSUSE it doesn't happen in that Twofish repo, but it does happen on the initial commit in my other repo: https://github.com/tengwar/haskell-learning . I have cloned this Haskell repo into a different folder, restarted Ungit and it still happens, so you might be able to reproduce that second problem. Do you want me to file a separate issue for it?

Ajedi32 commented 9 years ago

@codingtwinky

I'm not sure if HEAD^! qualify as range. And this method hasn't been problematic yet, until this issue that is.

A suffix ^ followed by an exclamation mark is the same as giving commit <rev> and then all its parents prefixed with ^ to exclude them (and their ancestors).

Well, that text you quoted there is taken from the section on "specifying ranges", and the equivalent syntax mentioned in that quote is a syntax for including and excluding commits from a range.

I don't get an error when trying that syntax with git diff, but the docs for git diff do say that ""diff" is about comparing two endpoints, not ranges", which makes sense to me. Maybe using commit ranges with git diff is undefined behavior, and git is just treating it differently depending on platform? What behavior were you expecting to get out of git diff by using that syntax?

Ajedi32 commented 9 years ago

Also...

git diff HEAD^^! -- src/twofish/Main.java should be equivalent to git diff HEAD~1:src/twofish/Main.java HEAD:src/twofish/Main.java

Assuming I'm reading the docs right and parsing this statement in my head correctly, HEAD^^! will be interpreted as HEAD^^! => (HEAD^ ^((HEAD^)^@)) => (HEAD^1 ^((HEAD^1)^1 (HEAD^1)^2 (HEAD^1)^3 etc...)) which specifies a range containing only the first parent of HEAD, with no other commits. (E.g. the set of commits: {HEAD^1})

The way I see it, git diff interpreting that as "display the diff of the changes made in the parent of HEAD" would actually be pretty reasonable.

jung-kim commented 9 years ago

@tengwar very first commit diff not showing up is documented at #548 and it is fixed in diff refactoring at #514.

@Ajedi32 hmm, I guess I was expecting git diff HEAD^^! -- [file] to be a short hand version of git diff HEAD~1:[file] HEAD:[file]. I see what you mean and I have no idea where this was picked up from.

Wording on git diff is confusing though,

"For a more complete list of ways to spell <commit>, see "SPECIFYING REVISIONS" section in gitrevisions(7). However, "diff" is about comparing two endpoints, not ranges, and the range notations ("<commit>..<commit>" and "<commit>...<commit>") do not mean a range as defined in the "SPECIFYING RANGES" section in gitrevisions(7)."

I guess in another words diff, do not accept ranges. However diff do accept "..." or ".." as diff do not treat "..." or ".." as ranges.

514 will be fixed to include more proper diff method, git diff <rev>~1:[file] <rev>:[file].

Ajedi32 commented 9 years ago

Yeah, it's really confusing that git diff allows the ... syntax, but defines it to mean something completely different from what it normally means with other commands.

campersau commented 9 years ago

I think that https://github.com/FredrikNoren/ungit/pull/563 might fixed this issue too. I had the same problems on windows before that some diffs were from previous commits and some just didn't show up.

FredrikNoren commented 9 years ago

Should be fixed in #514