From 9f1b28b40ae26cba2440dc8e71cbb262eac9efe6 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 27 Oct 2013 19:51:36 +0400 Subject: [PATCH] [log] IDEA-115480 Correctly identify details of selected rows DetailsPanel, Actions: Don't ask the Graph about nodes or details of the selected rows, because in the case of non-graph filter the result will be incorrect. Instead take the value from the table model. Let the CommitCell hold the Hash value. In some cases it might be null (for example, when there are "empty" lines in the table containing only graph fragments). Let DataGetter return value for the hash, not only for the node. This can be slow potentially, will be optimized or moved to the background later. Let Copy Hash action operate on Hash instead of VcsFullCommitDetails. Make Cherry-Pick and Open on GitHub be unavailable if details of the selected hashes were not loaded yet. --- .../api/src/com/intellij/vcs/log/VcsLog.java | 11 ++++- .../vcs/log/graph/render/CommitCell.java | 15 ++++++- .../vcs/log/graph/render/GraphCommitCell.java | 8 +++- .../com/intellij/vcs/log/data/DataGetter.java | 41 +++++++++++++++---- .../com/intellij/vcs/log/impl/VcsLogImpl.java | 39 +++++++++--------- .../vcs/log/ui/VcsLogCopyHashAction.java | 10 ++--- .../vcs/log/ui/frame/DetailsPanel.java | 33 ++++++++------- .../vcs/log/ui/tables/GraphTableModel.java | 5 ++- .../vcs/log/ui/tables/NoGraphTableModel.java | 2 +- .../cherrypick/GitCherryPickAction.java | 28 ++++++++----- ...ithubShowCommitInBrowserFromLogAction.java | 14 +++++-- 11 files changed, 138 insertions(+), 68 deletions(-) diff --git a/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLog.java b/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLog.java index 7331691e9b09..ab79e3305b06 100644 --- a/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLog.java +++ b/platform/vcs-log/api/src/com/intellij/vcs/log/VcsLog.java @@ -30,7 +30,16 @@ public interface VcsLog { * Returns commits currently selected in the log. */ @NotNull - List getSelectedCommits(); + List getSelectedCommits(); + + /** + * Returns details of the given commit, if they have been already loaded. + * In most cases they are already in the cache, and will be returned. + * Otherwise null is returned. + * Asynchronous loading of the details which are not yet available is done automatically from the log table component. + */ + @Nullable + VcsFullCommitDetails getDetailsIfAvailable(@NotNull Hash hash); /** * Returns names of branches which contain the given commit, or null if this information is unavailable. diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/CommitCell.java b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/CommitCell.java index 677975b5f020..0d540421d071 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/CommitCell.java +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/CommitCell.java @@ -1,6 +1,9 @@ package com.intellij.vcs.log.graph.render; +import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsRef; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.Collection; @@ -11,8 +14,14 @@ public class CommitCell { private final String text; private final Collection refsToThisCommit; + private Hash myHash; - public CommitCell(String text, Collection refsToThisCommit) { + /** + * Hash can be null, if, for example, this is a cell which doesn't contain a commit, but contains only a part of the graph + * (such situations may appear, for example, if graph is filtered by branch, as described in IDEA-115442). + */ + public CommitCell(@Nullable Hash hash, @NotNull String text, @NotNull Collection refsToThisCommit) { + myHash = hash; this.text = text; this.refsToThisCommit = refsToThisCommit; } @@ -25,4 +34,8 @@ public class CommitCell { return refsToThisCommit; } + @Nullable + public Hash getHash() { + return myHash; + } } diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/GraphCommitCell.java b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/GraphCommitCell.java index 70bf238da683..00a895020589 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/GraphCommitCell.java +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/render/GraphCommitCell.java @@ -1,7 +1,10 @@ package com.intellij.vcs.log.graph.render; +import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsRef; import com.intellij.vcs.log.printmodel.GraphPrintCell; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.Collection; @@ -13,8 +16,9 @@ public class GraphCommitCell extends CommitCell { private final GraphPrintCell row; - public GraphCommitCell(GraphPrintCell row, String text, Collection refsToThisCommit) { - super(text, refsToThisCommit); + public GraphCommitCell(@Nullable Hash hash, @NotNull GraphPrintCell row, @NotNull String text, + @NotNull Collection refsToThisCommit) { + super(hash, text, refsToThisCommit); this.row = row; } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/DataGetter.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/DataGetter.java index d7023d9861a3..47d51fbd90d5 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/DataGetter.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/DataGetter.java @@ -64,18 +64,43 @@ public abstract class DataGetter implements Dis public T getCommitData(@NotNull final Node node) { assert EventQueue.isDispatchThread(); Hash hash = node.getCommitHash(); + T details = getFromCache(hash); + if (details != null) { + return details; + } + return loadingDetails(node, hash); + } + + @NotNull + private T loadingDetails(Node node, Hash hash) { + T loadingDetails = (T)new LoadingDetails(hash); + runLoadAroundCommitData(node); + return loadingDetails; + } + + @NotNull + public T getCommitData(@NotNull Hash hash) { + assert EventQueue.isDispatchThread(); + T details = getFromCache(hash); + if (details != null) { + return details; + } + Node node = myDataHolder.getDataPack().getNodeByHash(hash); // TODO this may possibly be slow => need to add to the Task as well + return loadingDetails(node, hash); + } + + @Nullable + public T getCommitDataIfAvailable(@NotNull Hash hash) { + return getFromCache(hash); + } + + @Nullable + private T getFromCache(@NotNull Hash hash) { T details = myCache.get(hash); if (details != null) { return details; } - details = (T)myDataHolder.getTopCommitDetails(hash); - if (details != null) { - return details; - } - - T loadingDetails = (T)new LoadingDetails(hash); - runLoadAroundCommitData(node); - return loadingDetails; + return (T)myDataHolder.getTopCommitDetails(hash); } @Nullable diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogImpl.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogImpl.java index e4e4c84cd5cb..7467c47c2c88 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogImpl.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogImpl.java @@ -15,14 +15,15 @@ */ package com.intellij.vcs.log.impl; +import com.intellij.ui.table.JBTable; import com.intellij.util.containers.ContainerUtil; import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsFullCommitDetails; import com.intellij.vcs.log.VcsLog; -import com.intellij.vcs.log.data.LoadingDetails; import com.intellij.vcs.log.data.VcsLogDataHolder; -import com.intellij.vcs.log.graph.elements.Node; +import com.intellij.vcs.log.graph.render.CommitCell; import com.intellij.vcs.log.ui.VcsLogUI; +import com.intellij.vcs.log.ui.tables.AbstractVcsLogTableModel; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -50,17 +51,25 @@ public class VcsLogImpl implements VcsLog { return myDataHolder != null && myUi != null; } - @NotNull @Override - public List getSelectedCommits() { - List selectedDetails = ContainerUtil.newArrayList(); - for (int row : myUi.getTable().getSelectedRows()) { - VcsFullCommitDetails data = getDetailsAtRow(row); - if (data != null) { - selectedDetails.add(data); + @NotNull + public List getSelectedCommits() { + List hashes = ContainerUtil.newArrayList(); + JBTable table = myUi.getTable(); + for (int row : table.getSelectedRows()) { + CommitCell cell = (CommitCell)table.getModel().getValueAt(row, AbstractVcsLogTableModel.COMMIT_COLUMN); + Hash hash = cell.getHash(); + if (hash != null) { + hashes.add(hash); } } - return selectedDetails; + return hashes; + } + + @Override + @Nullable + public VcsFullCommitDetails getDetailsIfAvailable(@NotNull final Hash hash) { + return myDataHolder.getCommitDetailsGetter().getCommitDataIfAvailable(hash); } @Nullable @@ -69,14 +78,4 @@ public class VcsLogImpl implements VcsLog { return null; } - @Nullable - private VcsFullCommitDetails getDetailsAtRow(int row) { - Node commitNode = myDataHolder.getDataPack().getGraphModel().getGraph().getCommitNodeInRow(row); - if (commitNode == null) { - return null; - } - VcsFullCommitDetails details = myDataHolder.getCommitDetailsGetter().getCommitData(commitNode); - return details instanceof LoadingDetails ? null : details; - } - } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/VcsLogCopyHashAction.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/VcsLogCopyHashAction.java index 552492bd6451..1d9c8b520944 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/VcsLogCopyHashAction.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/VcsLogCopyHashAction.java @@ -23,7 +23,7 @@ import com.intellij.openapi.project.DumbAwareAction; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.Function; -import com.intellij.vcs.log.VcsFullCommitDetails; +import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsLog; import org.jetbrains.annotations.Nullable; @@ -42,15 +42,15 @@ public class VcsLogCopyHashAction extends DumbAwareAction { if (log == null) { return; } - List commits = log.getSelectedCommits(); + List commits = log.getSelectedCommits(); if (commits.isEmpty()) { return; } - String hashes = StringUtil.join(commits, new Function() { + String hashes = StringUtil.join(commits, new Function() { @Override - public String fun(VcsFullCommitDetails details) { - return details.getHash().asString(); + public String fun(Hash hash) { + return hash.asString(); } }, "\n"); CopyPasteManager.getInstance().setContents(new StringSelection(hashes)); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/frame/DetailsPanel.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/frame/DetailsPanel.java index 036326820f8c..350dbf72968b 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/frame/DetailsPanel.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/frame/DetailsPanel.java @@ -15,10 +15,11 @@ import com.intellij.vcs.log.VcsFullCommitDetails; import com.intellij.vcs.log.VcsRef; import com.intellij.vcs.log.data.LoadingDetails; import com.intellij.vcs.log.data.VcsLogDataHolder; -import com.intellij.vcs.log.graph.elements.Node; -import com.intellij.vcs.log.ui.VcsLogColorManager; +import com.intellij.vcs.log.graph.render.CommitCell; import com.intellij.vcs.log.graph.render.PrintParameters; +import com.intellij.vcs.log.ui.VcsLogColorManager; import com.intellij.vcs.log.ui.render.RefPainter; +import com.intellij.vcs.log.ui.tables.AbstractVcsLogTableModel; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -75,25 +76,21 @@ class DetailsPanel extends JPanel implements ListSelectionListener { public void valueChanged(@Nullable ListSelectionEvent notUsed) { int[] rows = myGraphTable.getSelectedRows(); if (rows.length < 1) { - myLoadingPanel.stopLoading(); - ((CardLayout)getLayout()).show(this, MESSAGE_LAYER); - myMessagePanel.setText("Nothing selected"); + showMessage("Nothing selected"); } else if (rows.length > 1) { - myLoadingPanel.stopLoading(); - ((CardLayout)getLayout()).show(this, MESSAGE_LAYER); - myMessagePanel.setText("Several commits selected"); + showMessage("Several commits selected"); } else { ((CardLayout)getLayout()).show(this, STANDARD_LAYER); - Node node = myLogDataHolder.getDataPack().getNode(rows[0]); - if (node == null) { - LOG.info("Couldn't find node for row " + rows[0] + - ". All nodes: " + myLogDataHolder.getDataPack().getGraphModel().getGraph().getNodeRows()); + CommitCell cell = (CommitCell)myGraphTable.getModel().getValueAt(rows[0], AbstractVcsLogTableModel.COMMIT_COLUMN); + Hash hash = cell.getHash(); + if (hash == null) { + showMessage("Nothing selected"); return; } - Hash hash = node.getCommitHash(); - VcsFullCommitDetails commitData = myLogDataHolder.getCommitDetailsGetter().getCommitData(node); + + VcsFullCommitDetails commitData = myLogDataHolder.getCommitDetailsGetter().getCommitData(hash); if (commitData instanceof LoadingDetails) { myLoadingPanel.startLoading(); myDataPanel.setData(null); @@ -102,11 +99,17 @@ class DetailsPanel extends JPanel implements ListSelectionListener { else { myLoadingPanel.stopLoading(); myDataPanel.setData(commitData); - myRefsPanel.setRefs(sortRefs(hash, node.getBranch().getRepositoryRoot())); + myRefsPanel.setRefs(sortRefs(hash, commitData.getRoot())); } } } + private void showMessage(String text) { + myLoadingPanel.stopLoading(); + ((CardLayout)getLayout()).show(this, MESSAGE_LAYER); + myMessagePanel.setText(text); + } + @NotNull private List sortRefs(@NotNull Hash hash, @NotNull VirtualFile root) { Collection refs = myLogDataHolder.getDataPack().getRefsModel().refsToCommit(hash); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/GraphTableModel.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/GraphTableModel.java index 4097afe1856a..74930db59073 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/GraphTableModel.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/GraphTableModel.java @@ -5,6 +5,7 @@ import com.intellij.openapi.util.EmptyRunnable; import com.intellij.openapi.vcs.changes.Change; import com.intellij.openapi.vcs.changes.committed.CommittedChangesTreeBrowser; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsFullCommitDetails; import com.intellij.vcs.log.VcsRef; import com.intellij.vcs.log.VcsShortCommitDetails; @@ -114,11 +115,13 @@ public class GraphTableModel extends AbstractVcsLogTableModel { GraphPrintCell graphPrintCell = myDataPack.getPrintCellModel().getGraphPrintCell(rowIndex); String message = ""; List refs = Collections.emptyList(); + Hash hash = null; if (details != null) { + hash = details.getHash(); message = details.getSubject(); refs = (List)myDataPack.getRefsModel().refsToCommit(details.getHash()); } - return new GraphCommitCell(graphPrintCell, message, refs); + return new GraphCommitCell(hash, graphPrintCell, message, refs); } @NotNull diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/NoGraphTableModel.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/NoGraphTableModel.java index 1d4c10fac244..124bd7fc976a 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/NoGraphTableModel.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/ui/tables/NoGraphTableModel.java @@ -96,7 +96,7 @@ public class NoGraphTableModel extends AbstractVcsLogTableModel { subject = details.getSubject(); refs = myRefsModel.refsToCommit(details.getHash()); } - return new CommitCell(subject, refs); + return new CommitCell(myCommits.get(index).getHash(), subject, refs); } @NotNull diff --git a/plugins/git4idea/src/git4idea/cherrypick/GitCherryPickAction.java b/plugins/git4idea/src/git4idea/cherrypick/GitCherryPickAction.java index 5878c074f229..3d938889da1c 100644 --- a/plugins/git4idea/src/git4idea/cherrypick/GitCherryPickAction.java +++ b/plugins/git4idea/src/git4idea/cherrypick/GitCherryPickAction.java @@ -24,7 +24,6 @@ import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.Task; import com.intellij.openapi.project.DumbAwareAction; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Condition; import com.intellij.util.Function; import com.intellij.util.containers.ContainerUtil; import com.intellij.vcs.log.Hash; @@ -175,20 +174,27 @@ public class GitCherryPickAction extends DumbAwareAction { if (project == null) { return null; } - VcsLog log = getVcsLog(project); + final VcsLog log = getVcsLog(project); if (log == null) { return null; } - List selectedCommits = log.getSelectedCommits(); - // don't allow to cherry-pick if a non-Git commit was selected - // we could cherry-pick just Git commits filtered from the list, but it might provide confusion - boolean nonGitCommitSelected = ContainerUtil.find(selectedCommits, new Condition() { - @Override - public boolean value(VcsFullCommitDetails details) { - return myPlatformFacade.getRepositoryManager(project).getRepositoryForRoot(details.getRoot()) == null; + + List selectedCommits = log.getSelectedCommits(); + List selectedDetails = ContainerUtil.newArrayList(); + for (Hash commit : selectedCommits) { + VcsFullCommitDetails details = log.getDetailsIfAvailable(commit); + if (details == null) { // let the action be unavailable until all details are loaded + return null; } - }) != null; - return nonGitCommitSelected ? null : selectedCommits; + GitRepository root = myPlatformFacade.getRepositoryManager(project).getRepositoryForRoot(details.getRoot()); + // don't allow to cherry-pick if a non-Git commit was selected + // we could cherry-pick just Git commits filtered from the list, but it might provide confusion + if (root == null) { + return null; + } + selectedDetails.add(details); + } + return selectedDetails; } private static List convertHeavyCommitToFullDetails(List commits) { diff --git a/plugins/github/src/org/jetbrains/plugins/github/GithubShowCommitInBrowserFromLogAction.java b/plugins/github/src/org/jetbrains/plugins/github/GithubShowCommitInBrowserFromLogAction.java index aadde26ee3dc..b3b2e12982f5 100644 --- a/plugins/github/src/org/jetbrains/plugins/github/GithubShowCommitInBrowserFromLogAction.java +++ b/plugins/github/src/org/jetbrains/plugins/github/GithubShowCommitInBrowserFromLogAction.java @@ -22,7 +22,11 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.Function; import com.intellij.util.containers.ContainerUtil; -import com.intellij.vcs.log.*; +import com.intellij.vcs.log.Hash; +import com.intellij.vcs.log.VcsLog; +import com.intellij.vcs.log.VcsLogObjectsFactory; +import com.intellij.vcs.log.VcsShortCommitDetails; +import com.intellij.vcs.log.impl.VcsLogImpl; import git4idea.GitUtil; import git4idea.GitVcs; import git4idea.history.browser.GitHeavyCommit; @@ -80,9 +84,13 @@ public class GithubShowCommitInBrowserFromLogAction extends GithubShowCommitInBr return factory.createShortDetails(factory.createHash(heavyCommit.getHash().getValue()), parents, heavyCommit.getAuthorTime(), heavyCommit.getRoot(), heavyCommit.getSubject(), heavyCommit.getAuthor()); } - List selectedCommits = ServiceManager.getService(e.getProject(), VcsLog.class).getSelectedCommits(); + VcsLog log = ServiceManager.getService(e.getProject(), VcsLog.class); + if (log == null || !((VcsLogImpl)log).isReady()) { + return null; + } + List selectedCommits = log.getSelectedCommits(); if (selectedCommits.size() == 1) { - return selectedCommits.get(0); + return log.getDetailsIfAvailable(selectedCommits.get(0)); } return null; }