diff --git a/plugins/git4idea/src/git4idea/cherrypick/GitCherryPicker.java b/plugins/git4idea/src/git4idea/cherrypick/GitCherryPicker.java index 0730a1821f75..b5fe83c00c31 100644 --- a/plugins/git4idea/src/git4idea/cherrypick/GitCherryPicker.java +++ b/plugins/git4idea/src/git4idea/cherrypick/GitCherryPicker.java @@ -21,6 +21,7 @@ import com.intellij.notification.NotificationListener; import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vcs.FilePath; import com.intellij.openapi.vcs.VcsNotifier; import com.intellij.openapi.vcs.changes.*; @@ -42,11 +43,15 @@ import git4idea.merge.GitConflictResolver; import git4idea.repo.GitRepository; import git4idea.util.UntrackedFilesNotifier; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import javax.swing.event.HyperlinkEvent; import java.io.File; import java.io.IOException; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -80,18 +85,19 @@ public class GitCherryPicker { } public void cherryPick(@NotNull Map> commitsInRoots) { - List successfulCommits = new ArrayList(); + List successfulCommits = ContainerUtil.newArrayList(); + List alreadyPicked = ContainerUtil.newArrayList(); DvcsUtil.workingTreeChangeStarted(myProject); try { for (Map.Entry> entry : commitsInRoots.entrySet()) { GitRepository repository = entry.getKey(); - boolean result = cherryPick(repository, entry.getValue(), successfulCommits); + boolean result = cherryPick(repository, entry.getValue(), successfulCommits, alreadyPicked); repository.update(); if (!result) { return; } } - notifySuccess(successfulCommits); + notifyResult(successfulCommits, alreadyPicked); } finally { DvcsUtil.workingTreeChangeFinished(myProject); @@ -100,7 +106,7 @@ public class GitCherryPicker { // return true to continue with other roots, false to break execution private boolean cherryPick(@NotNull GitRepository repository, @NotNull List commits, - @NotNull List successfulCommits) { + @NotNull List successfulCommits, @NotNull List alreadyPicked) { for (VcsFullCommitDetails commit : commits) { GitSimpleEventDetector conflictDetector = new GitSimpleEventDetector(CHERRY_PICK_CONFLICT); GitSimpleEventDetector localChangesOverwrittenDetector = new GitSimpleEventDetector(LOCAL_CHANGES_OVERWRITTEN_BY_CHERRY_PICK); @@ -115,7 +121,7 @@ public class GitCherryPicker { } else { boolean committed = updateChangeListManagerShowCommitDialogAndRemoveChangeListOnSuccess(repository, commitWrapper, - successfulCommits); + successfulCommits, alreadyPicked); if (!committed) { notifyCommitCancelled(commitWrapper, successfulCommits); return false; @@ -129,7 +135,7 @@ public class GitCherryPicker { if (mergeCompleted) { boolean committed = updateChangeListManagerShowCommitDialogAndRemoveChangeListOnSuccess(repository, commitWrapper, - successfulCommits); + successfulCommits, alreadyPicked); if (!committed) { notifyCommitCancelled(commitWrapper, successfulCommits); return false; @@ -156,6 +162,10 @@ public class GitCherryPicker { commitWrapper, successfulCommits); return false; } + else if (isNothingToCommitMessage(result)) { + alreadyPicked.add(commitWrapper); + return true; + } else { notifyError(result.getErrorOutputAsHtmlString(), commitWrapper, successfulCommits); return false; @@ -164,10 +174,23 @@ public class GitCherryPicker { return true; } + private static boolean isNothingToCommitMessage(@NotNull GitCommandResult result) { + if (!result.getErrorOutputAsJoinedString().isEmpty()) { + return false; + } + String stdout = result.getOutputAsJoinedString(); + return stdout.contains("nothing to commit") || stdout.contains("previous cherry-pick is now empty"); + } + private boolean updateChangeListManagerShowCommitDialogAndRemoveChangeListOnSuccess(@NotNull GitRepository repository, @NotNull GitCommitWrapper commit, - @NotNull List successfulCommits) { + @NotNull List successfulCommits, + @NotNull List alreadyPicked) { CherryPickData data = updateChangeListManager(commit.getCommit()); + if (data == null) { + alreadyPicked.add(commit); + return true; + } boolean committed = showCommitDialogAndWaitForCommit(repository, commit, data.myChangeList, data.myCommitMessage); if (committed) { myChangeListManager.removeChangeList(data.myChangeList); @@ -199,15 +222,16 @@ public class GitCherryPicker { VcsNotifier.getInstance(myProject).notifyMinorWarning("Cherry-pick cancelled", description, null); } + @Nullable private CherryPickData updateChangeListManager(@NotNull final VcsFullCommitDetails commit) { final Collection paths = ChangesUtil.getPaths(commit.getChanges()); refreshChangedFiles(paths); final String commitMessage = createCommitMessage(commit); LocalChangeList changeList = createChangeListAfterUpdate(commit, paths, commitMessage); - return new CherryPickData(changeList, commitMessage); + return changeList == null ? null : new CherryPickData(changeList, commitMessage); } - @NotNull + @Nullable private LocalChangeList createChangeListAfterUpdate(@NotNull final VcsFullCommitDetails commit, @NotNull final Collection paths, @NotNull final String commitMessage) { final AtomicReference changeList = new AtomicReference(); @@ -338,9 +362,37 @@ public class GitCherryPicker { return description; } - private void notifySuccess(@NotNull List successfulCommits) { - String description = getCommitsDetails(successfulCommits); - VcsNotifier.getInstance(myProject).notifySuccess("Cherry-pick successful", description); + private void notifyResult(@NotNull List successfulCommits, @NotNull List alreadyPicked) { + if (alreadyPicked.isEmpty()) { + VcsNotifier.getInstance(myProject).notifySuccess("Cherry-pick successful", getCommitsDetails(successfulCommits)); + } + else if (!successfulCommits.isEmpty()) { + String title = String.format("Cherry-picked %d commits from %d", successfulCommits.size(), + successfulCommits.size() + alreadyPicked.size()); + String description = getCommitsDetails(successfulCommits) + "

" + formAlreadyPickedDescription(alreadyPicked, true); + VcsNotifier.getInstance(myProject).notifyImportantWarning(title, description); + } + else { + VcsNotifier.getInstance(myProject).notifyImportantWarning("Nothing to cherry-pick", + formAlreadyPickedDescription(alreadyPicked, false)); + } + } + + @NotNull + private static String formAlreadyPickedDescription(@NotNull List alreadyPicked, boolean but) { + + String hashes = StringUtil.join(alreadyPicked, new Function() { + @Override + public String fun(GitCommitWrapper commit) { + return commit.getCommit().getId().toShortString(); + } + }, ", "); + if (but) { + String wasnt = alreadyPicked.size() == 1 ? "wasn't" : "weren't"; + String it = alreadyPicked.size() == 1 ? "it" : "them"; + return String.format("%s %s picked, because all changes from %s have already been applied.", hashes, wasnt, it); + } + return String.format("All changes from %s have already been applied", hashes); } @NotNull @@ -365,9 +417,10 @@ public class GitCherryPicker { } })); VfsUtil.markDirtyAndRefresh(false, false, false, ArrayUtil.toObjectArray(virtualFiles, VirtualFile.class)); + VcsDirtyScopeManager.getInstance(myProject).filePathsDirty(filePaths, null); } - @NotNull + @Nullable private LocalChangeList createChangeList(@NotNull VcsFullCommitDetails commit, @NotNull String commitMessage) { Collection changes = commit.getChanges(); String changeListName = createNameForChangeList(commitMessage, 0).replace('\n', ' '); @@ -375,7 +428,11 @@ public class GitCherryPicker { if (!changes.isEmpty()) { myChangeListManager.moveChangesTo(changeList, changes.toArray(new Change[changes.size()])); } - return changeList; + if (!changeList.getChanges().isEmpty()) { + return changeList; + } + myChangeListManager.removeChangeList(changeList); + return null; } @NotNull @@ -393,10 +450,10 @@ public class GitCherryPicker { } private static class CherryPickData { - private final LocalChangeList myChangeList; - private final String myCommitMessage; + @NotNull private final LocalChangeList myChangeList; + @NotNull private final String myCommitMessage; - private CherryPickData(LocalChangeList list, String message) { + private CherryPickData(@NotNull LocalChangeList list, @NotNull String message) { myChangeList = list; myCommitMessage = message; } diff --git a/plugins/git4idea/test-features/git4idea/cherry-pick-auto-commit.feature b/plugins/git4idea/test-features/git4idea/cherry-pick-auto-commit.feature index ae5bdbfde0ce..1d85fd5022be 100644 --- a/plugins/git4idea/test-features/git4idea/cherry-pick-auto-commit.feature +++ b/plugins/git4idea/test-features/git4idea/cherry-pick-auto-commit.feature @@ -4,12 +4,11 @@ Background: Given enabled auto-commit in the settings Given new committed files file.txt, a.txt, conflict.txt with initial content Given branch feature - Given commit f5027a3 on branch feature """ fix #1 Author: John Bro - M file.txt "feature changes" + M file.txt "initial content\nfeature changes" """ Scenario: Simple cherry-pick @@ -220,53 +219,53 @@ Background: """ And merge dialog should be shown - #Scenario: Notify if changes have already been applied (IDEA-73548) - # Given commit eef9832 on branch master - # """ - # fix #1 manually incorporated - # M file.txt "feature changes" - # """ - # When I cherry-pick the commit f5027a3 - # Then the last commit is eef9832 - # And warning notification is shown 'Nothing to cherry-pick' - # """ - # All changes from f5027a3 fix #1 have already been applied - # """ - # - #Scenario: Cherry-pick 3 commits, second commit have already been applied (IDEA-73548) - # Given commit c123abc on branch feature - # """ - # fix #2 - # M file.txt "feature changes\nmore feature changes" - # """ - # Given commit d123abc on branch feature - # """ - # fix #3 - # M file.txt "feature changes\nmore feature changes\nmore feature changes" - # """ - # Given commit e123abc on branch feature - # """ - # fix for f2 - # M a.txt "feature changes" - # """ - # Given commit e098fed on branch master - # """ - # fix for f2 manually incorporated - # M a.txt "feature changes" - # """ - # When I cherry-pick commits c123abc, d123abc and e123abc - # Then `git log -2` should return - # """ - # fix #3 - # (cherry picked from commit c123abc) - # ----- - # fix #2 - # (cherry picked from commit f5027a3) - # """ - # And warning notification is shown 'Cherry-picked 2 commits' - # """ - # c123abc fix #2 - # d123abc fix #3 - #

- # Commit e123abc wasn't picked, because all changes from it have already been applied. - # """ + Scenario: Notify if changes have already been applied (IDEA-73548) + Given commit eef9832 on branch master + """ + fix #1 manually incorporated + M file.txt "feature changes" + """ + When I cherry-pick the commit f5027a3 + Then the last commit is eef9832 + And warning notification is shown 'Nothing to cherry-pick' + """ + All changes from f5027a3 fix #1 have already been applied + """ + + Scenario: Cherry-pick 3 commits, second commit have already been applied (IDEA-73548) + Given commit c123abc on branch feature + """ + fix #2 + M file.txt "feature changes\nmore feature changes" + """ + Given commit d123abc on branch feature + """ + fix #3 + M file.txt "feature changes\nmore feature changes\nmore feature changes" + """ + Given commit e123abc on branch feature + """ + fix for f2 + M a.txt "feature changes" + """ + Given commit e098fed on branch master + """ + fix for f2 manually incorporated + M a.txt "feature changes" + """ + When I cherry-pick commits c123abc, d123abc and e123abc + Then `git log -2` should return + """ + fix #3 + (cherry picked from commit c123abc) + ----- + fix #2 + (cherry picked from commit f5027a3) + """ + And warning notification is shown 'Cherry-picked 2 commits' + """ + c123abc fix #2 + d123abc fix #3 +
+ Commit e123abc wasn't picked, because all changes from it have already been applied. + """ diff --git a/plugins/git4idea/test-stepdefs/git4idea/GeneralStepdefs.java b/plugins/git4idea/test-stepdefs/git4idea/GeneralStepdefs.java index 5195f40abc23..f70f2417d5d3 100644 --- a/plugins/git4idea/test-stepdefs/git4idea/GeneralStepdefs.java +++ b/plugins/git4idea/test-stepdefs/git4idea/GeneralStepdefs.java @@ -70,14 +70,15 @@ public class GeneralStepdefs { notificationType.equals("error") ? NotificationType.ERROR : null; Notification actualNotification = lastNotification(); assertNotNull("Notification should be shown", actualNotification); - assertEquals("Notification type is incorrect", type, actualNotification.getType()); - assertEquals("Notification title is incorrect", title, actualNotification.getTitle()); + assertEquals("Notification type is incorrect in " + actualNotification, type, actualNotification.getType()); + assertEquals("Notification title is incorrect in" + actualNotification, title, actualNotification.getTitle()); assertNotificationContent(content, actualNotification.getContent()); } private static void assertNotificationContent(String expected, String actual) { expected = virtualCommits.replaceVirtualHashes(expected); - assertEquals("Notification content is incorrect", StringUtil.convertLineSeparators(expected), StringUtil.convertLineSeparators(adjustNotificationContent(actual))); + assertEquals("Notification content is incorrect", StringUtil.convertLineSeparators(expected), + StringUtil.convertLineSeparators(adjustNotificationContent(actual))); } private static String adjustNotificationContent(String content) { diff --git a/plugins/git4idea/test-stepdefs/git4idea/GitCherryPickStepdefs.java b/plugins/git4idea/test-stepdefs/git4idea/GitCherryPickStepdefs.java index e43d5edbc037..6d5cc31c012e 100644 --- a/plugins/git4idea/test-stepdefs/git4idea/GitCherryPickStepdefs.java +++ b/plugins/git4idea/test-stepdefs/git4idea/GitCherryPickStepdefs.java @@ -15,33 +15,30 @@ */ package git4idea; -import com.intellij.mock.MockVirtualFile; -import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.util.Condition; -import com.intellij.openapi.util.ThrowableComputable; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.openapi.vcs.FilePathImpl; -import com.intellij.openapi.vcs.changes.Change; +import com.intellij.openapi.vcs.VcsException; import com.intellij.openapi.vcs.changes.LocalChangeList; -import com.intellij.openapi.vcs.history.VcsRevisionNumber; -import com.intellij.openapi.vfs.newvfs.impl.NullVirtualFile; +import com.intellij.openapi.vfs.VirtualFile; import com.intellij.testFramework.vcs.MockChangeListManager; -import com.intellij.testFramework.vcs.MockContentRevision; +import com.intellij.util.ArrayUtil; +import com.intellij.util.Function; import com.intellij.util.containers.ContainerUtil; -import com.intellij.vcs.log.Hash; import com.intellij.vcs.log.VcsFullCommitDetails; -import com.intellij.vcs.log.VcsLogObjectsFactory; -import com.intellij.vcs.log.impl.HashImpl; import cucumber.annotation.en.And; import cucumber.annotation.en.Given; import cucumber.annotation.en.Then; import cucumber.annotation.en.When; import git4idea.cherrypick.GitCherryPicker; import git4idea.config.GitVersionSpecialty; +import git4idea.history.GitHistoryUtils; import git4idea.test.MockVcsHelper; import org.jetbrains.annotations.NotNull; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import static com.intellij.openapi.vcs.Executor.echo; import static git4idea.GitCucumberWorld.*; @@ -63,7 +60,7 @@ public class GitCherryPickStepdefs { } @When("^I cherry-pick the commit (\\w+)$") - public void I_cherry_pick_the_commit(String hash) { + public void I_cherry_pick_the_commit(String hash) throws VcsException { cherryPick(hash); } @@ -232,36 +229,27 @@ public class GitCherryPickStepdefs { assertEquals("Default changelist is not active", DEFAULT, myChangeListManager.getDefaultChangeList().getName()); } - private static void cherryPick(List virtualHashes) { - List commits = ContainerUtil.newArrayList(); - for (String virtualHash : virtualHashes) { - commits.add(createMockCommit(virtualHash)); - } + private static void cherryPick(final List virtualHashes) throws VcsException { + List commits = loadDetails(ContainerUtil.map(virtualHashes, new Function() { + @Override + public String fun(String virtualHash) { + return virtualCommits.getRealCommit(virtualHash).getHash(); + } + }), myProjectDir); + new GitCherryPicker(myProject, myGit, myPlatformFacade, mySettings.isAutoCommitOnCherryPick()) - .cherryPick(Collections.singletonMap(myRepository, commits)); + .cherryPick(Collections.singletonMap(myRepository, commits)); } - private static void cherryPick(String... virtualHashes) { + private static List loadDetails(List hashes, @NotNull VirtualFile root) throws VcsException { + String noWalk = GitVersionSpecialty.NO_WALK_UNSORTED.existsIn(myVcs.getVersion()) ? "--no-walk=unsorted" : "--no-walk"; + List params = new ArrayList(); + params.add(noWalk); + params.addAll(hashes); + return new ArrayList(GitHistoryUtils.history(myProject, root, ArrayUtil.toStringArray(params))); + } + + private static void cherryPick(String... virtualHashes) throws VcsException { cherryPick(Arrays.asList(virtualHashes)); } - - private static VcsFullCommitDetails createMockCommit(String virtualHash) { - CommitDetails realCommit = virtualCommits.getRealCommit(virtualHash); - return mockCommit(realCommit.getHash(), realCommit.getMessage()); - } - - private static VcsFullCommitDetails mockCommit(String hash, String message) { - final List changes = new ArrayList(); - changes.add(new Change(null, new MockContentRevision(new FilePathImpl(new MockVirtualFile("name")), VcsRevisionNumber.NULL))); - return ServiceManager.getService(myProject, VcsLogObjectsFactory.class).createFullDetails( - HashImpl.build(hash), Collections.emptyList(), 0, NullVirtualFile.INSTANCE, message, "John Smith", "john@mail.com", message, - "John Smith", "john@mail.com", 0, new ThrowableComputable, Exception>() { - @Override - public Collection compute() throws Exception { - return changes; - } - } - ); - } - } \ No newline at end of file