From 941e25e5995bcf3d1d584b5aefcaeaf48cdd3870 Mon Sep 17 00:00:00 2001 From: Dmitry Batrak Date: Mon, 23 Apr 2018 17:56:16 +0300 Subject: [PATCH] get rid of recursion on undo (fix EA-119527 - SOE: UndoManagerImpl.undo) --- .../openapi/command/impl/CommandMerger.java | 7 ++++ .../openapi/command/impl/UndoRedo.java | 41 +++++++++---------- .../command/impl/UndoRedoStacksHolder.java | 11 ----- .../src/messages/CommonBundle.properties | 1 + 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/command/impl/CommandMerger.java b/platform/platform-impl/src/com/intellij/openapi/command/impl/CommandMerger.java index 2bfe55571f0e..0d7d5f1e780d 100644 --- a/platform/platform-impl/src/com/intellij/openapi/command/impl/CommandMerger.java +++ b/platform/platform-impl/src/com/intellij/openapi/command/impl/CommandMerger.java @@ -210,6 +210,13 @@ public class CommandMerger { boolean isInsideStartFinishGroup = false; while ((undoRedo = createUndoOrRedo(editor, isUndo)) != null) { + if (editor != null && undoRedo.isBlockedByOtherChanges()) { + UndoRedo blockingChange = createUndoOrRedo(null, isUndo); + if (blockingChange != null && blockingChange.myUndoableGroup != undoRedo.myUndoableGroup) { + if (undoRedo.confirmSwitchTo(blockingChange)) blockingChange.execute(false, true); + break; + } + } if (!undoRedo.execute(false, isInsideStartFinishGroup)) return; isInsideStartFinishGroup = undoRedo.myUndoableGroup.isInsideStartFinishGroup(isUndo, isInsideStartFinishGroup); if (isInsideStartFinishGroup) continue; diff --git a/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedo.java b/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedo.java index 1254253fa901..12df529ff2da 100644 --- a/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedo.java +++ b/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedo.java @@ -16,6 +16,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.ReadonlyStatusHandler; import com.intellij.openapi.vfs.VfsUtil; import com.intellij.openapi.vfs.VirtualFile; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Collection; @@ -79,7 +80,7 @@ abstract class UndoRedo { protected abstract void setBeforeState(EditorAndState state); - public boolean execute(boolean drop, boolean isInsideStartFinishGroup) { + public boolean execute(boolean drop, boolean disableConfirmation) { if (!myUndoableGroup.isUndoable()) { reportCannotUndo(CommonBundle.message("cannot.undo.error.contains.nonundoable.changes.message"), myUndoableGroup.getAffectedDocuments()); @@ -88,14 +89,12 @@ abstract class UndoRedo { Set clashing = getStackHolder().collectClashingActions(myUndoableGroup); if (!clashing.isEmpty()) { - if (!tryFallbackToGlobalUndo()) - reportCannotUndo(CommonBundle.message("cannot.undo.error.other.affected.files.changed.message"), clashing); - + reportCannotUndo(CommonBundle.message("cannot.undo.error.other.affected.files.changed.message"), clashing); return false; } - if (!isInsideStartFinishGroup && myUndoableGroup.shouldAskConfirmation(isRedo()) && !UndoManagerImpl.ourNeverAskUser) { + if (!disableConfirmation && myUndoableGroup.shouldAskConfirmation(isRedo()) && !UndoManagerImpl.ourNeverAskUser) { if (!askUser()) return false; } else { @@ -140,22 +139,6 @@ abstract class UndoRedo { return true; } - private boolean tryFallbackToGlobalUndo() { - UndoableGroup globalUndoableGroup = getStackHolder().findGlobalUndoableGroup(myUndoableGroup); - if (globalUndoableGroup != null) { - if (isRedo()) { - myManager.redo(null); - } - else { - myManager.undo(null); - } - - return true; - } - - return false; - } - protected abstract boolean isRedo(); private Collection collectReadOnlyDocuments() { @@ -212,6 +195,17 @@ abstract class UndoRedo { return isOk[0]; } + boolean confirmSwitchTo(@NotNull UndoRedo other) { + final boolean[] isOk = new boolean[1]; + TransactionGuard.getInstance().submitTransactionAndWait(() -> { + String message = CommonBundle.message("undo.conflicting.change.confirmation.message") + "\n" + + getActionName(other.myUndoableGroup.getCommandName()) + "?"; + isOk[0] = Messages.showOkCancelDialog(myManager.getProject(), message, getActionName(), + Messages.getQuestionIcon()) == Messages.OK; + }); + return isOk[0]; + } + private boolean restore(EditorAndState pair, boolean onlyIfDiffers) { // editor can be invalid if underlying file is deleted during undo (e.g. after undoing scratch file creation) if (pair == null || myEditor == null || !myEditor.isValid() || !pair.canBeAppliedTo(myEditor)) return false; @@ -228,4 +222,9 @@ abstract class UndoRedo { myEditor.setState(pair.getState()); return true; } + + public boolean isBlockedByOtherChanges() { + return myUndoableGroup.isGlobal() && myUndoableGroup.isUndoable() && + !getStackHolder().collectClashingActions(myUndoableGroup).isEmpty(); + } } diff --git a/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedoStacksHolder.java b/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedoStacksHolder.java index 3c7efe9824c6..cd234faac8c7 100644 --- a/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedoStacksHolder.java +++ b/platform/platform-impl/src/com/intellij/openapi/command/impl/UndoRedoStacksHolder.java @@ -12,7 +12,6 @@ import com.intellij.util.containers.WeakList; import gnu.trove.THashMap; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; import java.util.*; @@ -286,14 +285,4 @@ class UndoRedoStacksHolder { } } } - - @Nullable - public UndoableGroup findGlobalUndoableGroup(UndoableGroup undoableGroup) { - for (UndoableGroup group : myGlobalStack) { - if (group == undoableGroup) { - return group; - } - } - return null; - } } diff --git a/platform/platform-resources-en/src/messages/CommonBundle.properties b/platform/platform-resources-en/src/messages/CommonBundle.properties index bf94ecd0f3ea..f2251805e790 100644 --- a/platform/platform-resources-en/src/messages/CommonBundle.properties +++ b/platform/platform-resources-en/src/messages/CommonBundle.properties @@ -133,6 +133,7 @@ profiling.capture.snapshot.error=Failed to capture snapshot: {0} cannot.undo.dialog.title=Cannot Undo cannot.undo.error.other.affected.files.changed.message=Following files affected by this action have been already changed: cannot.undo.error.contains.nonundoable.changes.message=Following files have changes that cannot be undone: +undo.conflicting.change.confirmation.message=Other files affected by this action have been already changed. undo.dialog.title=Undo redo.command.confirmation.text=Redo {0} redo.confirmation.title=Redo