From a73461dcde720a8b9d43caeff827cb8ef40237f0 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 28 Jul 2012 17:35:42 +0400 Subject: [PATCH] IDEA-89376 Git integration doesn't show change diff for moved file Moved file has other path in the parent revision => constructing a fake GitFileRevision using the same file path is incorrect approach. Modified the DiffFromHistoryHandler to pass the previous revision from the file history panel (and renamed the interface methods to avoid confusion between them). This is somewhat incorrect, because the previous revision in the list is not necessarily a direct parent of current revision (it is an ancestor though; the file can be not changed in the parent revision). At best, we should query Git (git show ) to know the previous part, but it would require one more Git command. Current approach is acceptable until file history gets the graph log. Merge commits, however, should be handled separately. The fix will come in the next commit. --- .../vcs/history/DiffFromHistoryHandler.java | 14 +++++--- .../vcs/history/FileHistoryPanelImpl.java | 33 ++++++++++--------- .../actions/GitCompareWithBranchAction.java | 2 +- .../history/GitDiffFromHistoryHandler.java | 23 ++++--------- 4 files changed, 34 insertions(+), 38 deletions(-) diff --git a/platform/vcs-api/src/com/intellij/openapi/vcs/history/DiffFromHistoryHandler.java b/platform/vcs-api/src/com/intellij/openapi/vcs/history/DiffFromHistoryHandler.java index e7edc3a16f26..f49fc4d3bc84 100644 --- a/platform/vcs-api/src/com/intellij/openapi/vcs/history/DiffFromHistoryHandler.java +++ b/platform/vcs-api/src/com/intellij/openapi/vcs/history/DiffFromHistoryHandler.java @@ -32,11 +32,15 @@ public interface DiffFromHistoryHandler { /** * Show diff when a single revision is selected in the file history panel. * - * @param e AnActionEvent which happened, when user invoked "Show Diff". - * @param filePath the file which history is shown. - * @param revision the revision selected in the file history panel. + * @param e AnActionEvent which happened, when user invoked "Show Diff". + * @param filePath the file which history is shown. + * @param previousRevision the previous revision in the list displayed file history panel, may be {@link VcsFileRevision#NULL}. + * @param revision the revision selected in the file history panel. */ - void showDiff(@NotNull AnActionEvent e, @NotNull FilePath filePath, @NotNull VcsFileRevision revision); + void showDiffForOne(@NotNull AnActionEvent e, + @NotNull FilePath filePath, + @NotNull VcsFileRevision previousRevision, + @NotNull VcsFileRevision revision); /** * Show diff for 2 revisions selected from the file history panel, @@ -47,6 +51,6 @@ public interface DiffFromHistoryHandler { * @param revision1 one of the selected revisions. * @param revision2 another selected revision. */ - void showDiff(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2); + void showDiffForTwo(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2); } diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/history/FileHistoryPanelImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/history/FileHistoryPanelImpl.java index ca14240cb1e3..84781e12ed5c 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/history/FileHistoryPanelImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/history/FileHistoryPanelImpl.java @@ -859,12 +859,23 @@ public class FileHistoryPanelImpl extends PanelWithActionsAndCloseButton { int selectionSize = sel.size(); if (selectionSize > 1) { - myDiffHandler.showDiff(e, myFilePath, sel.get(0).getRevision()); + myDiffHandler.showDiffForTwo(myFilePath, sel.get(0), sel.get(sel.size() - 1)); } else if (selectionSize == 1) { + final TableView flatView = myDualView.getFlatView(); + final int selectedRow = flatView.getSelectedRow(); VcsFileRevision revision = getFirstSelectedRevision(); + + VcsFileRevision previousRevision; + if (selectedRow == (flatView.getRowCount() - 1)) { + // no previous + previousRevision = myBottomRevisionForShowDiff != null ? myBottomRevisionForShowDiff : VcsFileRevision.NULL; + } else { + previousRevision = flatView.getRow(selectedRow + 1); + } + if (revision != null) { - myDiffHandler.showDiff(e, myFilePath, revision); + myDiffHandler.showDiffForOne(e, myFilePath, previousRevision, revision); } } } @@ -907,7 +918,7 @@ public class FileHistoryPanelImpl extends PanelWithActionsAndCloseButton { final VcsRevisionNumber currentRevisionNumber = myHistorySession.getCurrentRevisionNumber(); VcsFileRevision selectedRevision = getFirstSelectedRevision(); if (currentRevisionNumber != null && selectedRevision != null) { - myDiffHandler.showDiff(myFilePath, selectedRevision, new CurrentRevision(myFilePath.getVirtualFile(), currentRevisionNumber)); + myDiffHandler.showDiffForTwo(myFilePath, selectedRevision, new CurrentRevision(myFilePath.getVirtualFile(), currentRevisionNumber)); } } @@ -1814,21 +1825,13 @@ public class FileHistoryPanelImpl extends PanelWithActionsAndCloseButton { private class StandardDiffFromHistoryHandler implements DiffFromHistoryHandler { @Override - public void showDiff(@NotNull AnActionEvent e, @NotNull FilePath filePath, @NotNull VcsFileRevision revision) { - final TableView flatView = myDualView.getFlatView(); - final int selectedRow = flatView.getSelectedRow(); - if (selectedRow == (flatView.getRowCount() - 1)) { - // no previous - VcsHistoryUtil.showDifferencesInBackground(myVcs.getProject(), filePath, - myBottomRevisionForShowDiff != null ? myBottomRevisionForShowDiff : VcsFileRevision.NULL, - revision, true); - } else { - VcsHistoryUtil.showDifferencesInBackground(myVcs.getProject(), myFilePath, flatView.getRow(selectedRow + 1), revision, true); - } + public void showDiffForOne(@NotNull AnActionEvent e, @NotNull FilePath filePath, + @NotNull VcsFileRevision previousRevision, @NotNull VcsFileRevision revision) { + VcsHistoryUtil.showDifferencesInBackground(myVcs.getProject(), myFilePath, previousRevision, revision, true); } @Override - public void showDiff(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2) { + public void showDiffForTwo(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2) { VcsHistoryUtil.showDifferencesInBackground(myProject, myFilePath, revision1, revision2, true); } } diff --git a/plugins/git4idea/src/git4idea/actions/GitCompareWithBranchAction.java b/plugins/git4idea/src/git4idea/actions/GitCompareWithBranchAction.java index d6a347fe9f42..691d5e08c88f 100644 --- a/plugins/git4idea/src/git4idea/actions/GitCompareWithBranchAction.java +++ b/plugins/git4idea/src/git4idea/actions/GitCompareWithBranchAction.java @@ -206,7 +206,7 @@ public class GitCompareWithBranchAction extends DumbAwareAction { final VcsFileRevision compareRevision = new GitFileRevision(project, filePath, new GitRevisionNumber(branchToCompare, compareRevisionNumber.getTimestamp()), false); CurrentRevision currentRevision = new CurrentRevision(file, new GitRevisionNumber(head, currentRevisionNumber.getTimestamp())); - new GitDiffFromHistoryHandler(project).showDiff(new FilePathImpl(file), compareRevision, currentRevision); + new GitDiffFromHistoryHandler(project).showDiffForTwo(new FilePathImpl(file), compareRevision, currentRevision); } private static void fileDoesntExistInBranchError(Project project, VirtualFile file, String branchToCompare) { diff --git a/plugins/git4idea/src/git4idea/history/GitDiffFromHistoryHandler.java b/plugins/git4idea/src/git4idea/history/GitDiffFromHistoryHandler.java index 00bc7ff011c8..c8f9fb8a6139 100644 --- a/plugins/git4idea/src/git4idea/history/GitDiffFromHistoryHandler.java +++ b/plugins/git4idea/src/git4idea/history/GitDiffFromHistoryHandler.java @@ -59,8 +59,8 @@ import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; /** - * {@link DiffFromHistoryHandler#showDiff(FilePath, VcsFileRevision, VcsFileRevision) "Show Diff" for 2 revision} calls the common code. - * {@link DiffFromHistoryHandler#showDiff(AnActionEvent, FilePath, VcsFileRevision) "Show diff" for 1 revision} + * {@link DiffFromHistoryHandler#showDiffForTwo(FilePath, VcsFileRevision, VcsFileRevision) "Show Diff" for 2 revision} calls the common code. + * {@link DiffFromHistoryHandler#showDiffForOne(com.intellij.openapi.actionSystem.AnActionEvent, com.intellij.openapi.vcs.FilePath, com.intellij.openapi.vcs.history.VcsFileRevision, com.intellij.openapi.vcs.history.VcsFileRevision) "Show diff" for 1 revision} * behaves differently for merge commits: for them it shown a popup displaying the parents of the selected commit. Selecting a parent * from the popup shows the difference with this parent. * If an ordinary (not merge) revision with 1 parent, it is the same as usual: just compare with the parent; @@ -82,11 +82,12 @@ public class GitDiffFromHistoryHandler implements DiffFromHistoryHandler { } @Override - public void showDiff(@NotNull AnActionEvent e, @NotNull FilePath filePath, @NotNull VcsFileRevision revision) { + public void showDiffForOne(@NotNull AnActionEvent e, @NotNull FilePath filePath, + @NotNull VcsFileRevision previousRevision, @NotNull VcsFileRevision revision) { GitFileRevision rev = (GitFileRevision)revision; Collection parents = rev.getParents(); if (parents.size() < 2) { - showDiffWithParent(revision, filePath, parents); + doShowDiff(filePath, previousRevision, revision, false); } else { // merge showDiffForMergeCommit(e, filePath, rev, parents); @@ -94,7 +95,7 @@ public class GitDiffFromHistoryHandler implements DiffFromHistoryHandler { } @Override - public void showDiff(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2) { + public void showDiffForTwo(@NotNull FilePath filePath, @NotNull VcsFileRevision revision1, @NotNull VcsFileRevision revision2) { doShowDiff(filePath, revision1, revision2, true); } @@ -283,18 +284,6 @@ public class GitDiffFromHistoryHandler implements DiffFromHistoryHandler { return new ShowDiffWithParentAction(filePath, rev, parent); } - private void showDiffWithParent(@NotNull VcsFileRevision revision, @NotNull FilePath filePath, @NotNull Collection parents) { - VcsFileRevision parentRevision; - if (parents.size() == 1) { - String parent = parents.iterator().next(); - parentRevision = makeRevisionFromHash(filePath, parent); - } - else { - parentRevision = VcsFileRevision.NULL; - } - doShowDiff(filePath, parentRevision, revision, false); - } - @NotNull private GitFileRevision makeRevisionFromHash(@NotNull FilePath filePath, @NotNull String hash) { return new GitFileRevision(myProject, filePath, new GitRevisionNumber(hash), false);