From 83995c6a77d68cfb84a2229f168efe8dc2de0a55 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 10 Feb 2011 13:00:58 +0300 Subject: [PATCH] IDEA-65126 Move method and folding bug 1. Move statement up/down processing now tries to preserve fold regions state; 2. Corresponding test is added; --- .../moveUpDown/MoverWrapper.java | 65 +++++++++++++++++++ .../folding/CodeFoldingManager.java | 10 +++ .../folding/impl/CodeFoldingManagerImpl.java | 5 ++ .../impl/UpdateFoldRegionsOperation.java | 23 +++++-- 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/codeInsight/editorActions/moveUpDown/MoverWrapper.java b/platform/lang-impl/src/com/intellij/codeInsight/editorActions/moveUpDown/MoverWrapper.java index 4d3ecc2c2a1f..8554a2e83b1d 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/editorActions/moveUpDown/MoverWrapper.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/editorActions/moveUpDown/MoverWrapper.java @@ -16,6 +16,7 @@ package com.intellij.codeInsight.editorActions.moveUpDown; +import com.intellij.codeInsight.folding.CodeFoldingManager; import com.intellij.openapi.editor.*; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.TextRange; @@ -27,6 +28,7 @@ import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; class MoverWrapper { protected final boolean myIsDown; @@ -82,6 +84,28 @@ class MoverWrapper { // to prevent flicker caretModel.moveToOffset(0); + // There is a possible case that the user performs, say, method move. It's also possible that one (or both) of moved methods + // are folded. We want to preserve their states then. The problem is that folding processing is based on PSI element pointers + // and the pointers behave as following during move up/down: + // method1() {} + // method2() {} + // Pointer for the fold region from method1 points to 'method2()' now and vice versa (check range markers processing on + // document change for further information). I.e. information about fold regions statuses holds the data swapped for + // 'method1' and 'method2'. Hence, we want to apply correct 'collapsed' status. + FoldRegion topRegion = null; + FoldRegion bottomRegion = null; + for (FoldRegion foldRegion : editor.getFoldingModel().getAllFoldRegions()) { + if (!foldRegion.isValid() || (!contains(myInfo.range1, foldRegion) && !contains(myInfo.range2, foldRegion))) { + continue; + } + if (contains(myInfo.range1, foldRegion) && !contains(topRegion, foldRegion)) { + topRegion = foldRegion; + } + else if (contains(myInfo.range2, foldRegion) && !contains(bottomRegion, foldRegion)) { + bottomRegion = foldRegion; + } + } + document.insertString(myInfo.range1.getStartOffset(), textToInsert2); document.deleteString(myInfo.range1.getStartOffset()+textToInsert2.length(), myInfo.range1.getEndOffset()); @@ -91,6 +115,21 @@ class MoverWrapper { final Project project = file.getProject(); PsiDocumentManager.getInstance(project).commitAllDocuments(); + // Swap fold regions status if necessary. + if (topRegion != null && bottomRegion != null) { + final FoldRegion finalTopRegion = topRegion; + final FoldRegion finalBottomRegion = bottomRegion; + editor.getFoldingModel().runBatchFoldingOperation(new Runnable() { + @Override + public void run() { + boolean topExpanded = finalTopRegion.isExpanded(); + finalTopRegion.setExpanded(finalBottomRegion.isExpanded()); + finalBottomRegion.setExpanded(topExpanded); + } + }); + } + CodeFoldingManager.getInstance(project).allowFoldingOnCaretLine(editor); + if (hasSelection) { restoreSelection(editor, selectionStart, selectionEnd, start, myInfo.range2.getStartOffset()); } @@ -105,6 +144,32 @@ class MoverWrapper { editor.getScrollingModel().scrollToCaret(ScrollType.RELATIVE); } + /** + * Allows to check if text range defined by the given range marker completely contains text range of the given fold region. + * + * @param rangeMarker range marker to check + * @param foldRegion fold region to check + * @return true if text range defined by the given range marker completely contains text range + * of the given fold region; false otherwise + */ + private static boolean contains(@NotNull RangeMarker rangeMarker, @NotNull FoldRegion foldRegion) { + return rangeMarker.getStartOffset() <= foldRegion.getStartOffset() && rangeMarker.getEndOffset() >= foldRegion.getEndOffset(); + } + + /** + * Allows to check if given 'region2' is nested to 'region1' + * + * @param region1 'outer' region candidate + * @param region2 'inner' region candidate + * @return true if 'region2' is nested to 'region1'; false otherwise + */ + private static boolean contains(@Nullable FoldRegion region1, @NotNull FoldRegion region2) { + if (region1 == null) { + return false; + } + return region1.getStartOffset() <= region2.getStartOffset() && region1.getEndOffset() >= region2.getEndOffset(); + } + private static void indentLinesIn(final Editor editor, final PsiFile file, final Document document, final Project project, RangeMarker range) { final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project); int line1 = editor.offsetToLogicalPosition(range.getStartOffset()).line; diff --git a/platform/lang-impl/src/com/intellij/codeInsight/folding/CodeFoldingManager.java b/platform/lang-impl/src/com/intellij/codeInsight/folding/CodeFoldingManager.java index 401ee19a2194..7c539366ac26 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/folding/CodeFoldingManager.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/folding/CodeFoldingManager.java @@ -49,4 +49,14 @@ public abstract class CodeFoldingManager { public abstract void releaseFoldings(Editor editor); public abstract void buildInitialFoldings(Editor editor); + + /** + * Asks do not perform automatic expansion of fold region the is located on the caret line during the next fold regions update. + *

+ * This method is necessary because fold regions that are located on caret line are automatically expanded to prevent problem + * from IDEA-64687. However, we don't want such an expansion when we, for example, move method up or down. + * + * @param editor target editor + */ + public abstract void allowFoldingOnCaretLine(@NotNull Editor editor); } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/CodeFoldingManagerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/CodeFoldingManagerImpl.java index 2cd30bfbe867..ac433b3b39c0 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/CodeFoldingManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/CodeFoldingManagerImpl.java @@ -296,4 +296,9 @@ public class CodeFoldingManagerImpl extends CodeFoldingManager implements Projec document.putUserData(FOLDING_STATE_INFO_IN_DOCUMENT_KEY, null); } } + + @Override + public void allowFoldingOnCaretLine(@NotNull Editor editor) { + editor.putUserData(UpdateFoldRegionsOperation.ALLOW_FOLDING_ON_CARET_LINE_KEY, true); + } } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/UpdateFoldRegionsOperation.java b/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/UpdateFoldRegionsOperation.java index 9563a448477c..852ae2c1d92b 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/UpdateFoldRegionsOperation.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/folding/impl/UpdateFoldRegionsOperation.java @@ -25,6 +25,7 @@ import com.intellij.openapi.editor.FoldingGroup; import com.intellij.openapi.editor.ex.FoldingModelEx; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Key; import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; @@ -41,6 +42,9 @@ import static com.intellij.util.containers.CollectionFactory.newTroveMap; * @author cdr */ class UpdateFoldRegionsOperation implements Runnable { + + static final Key ALLOW_FOLDING_ON_CARET_LINE_KEY = Key.create("AllowFoldingOnCaretLine.KEY"); + private final Project myProject; private final Editor myEditor; private final boolean myApplyDefaultState; @@ -68,6 +72,9 @@ class UpdateFoldRegionsOperation implements Runnable { List newRegions = addNewRegions(info, foldingModel, rangeToExpandStatusMap, shouldExpand, groupExpand); applyExpandStatus(newRegions, shouldExpand, groupExpand); + + // Reset the key. + myEditor.putUserData(ALLOW_FOLDING_ON_CARET_LINE_KEY, false); } private static void applyExpandStatus(List newRegions, Map shouldExpand, Map groupExpand) { @@ -125,11 +132,17 @@ class UpdateFoldRegionsOperation implements Runnable { } private boolean shouldExpandNewRegion(PsiElement element, TextRange range, Map rangeToExpandStatusMap) { - final Document document = myEditor.getDocument(); - final int firstLine = document.getLineNumber(range.getStartOffset()); - final int lastLine = document.getLineNumber(range.getEndOffset()); - final int currentLine = document.getLineNumber(myEditor.getCaretModel().getOffset()); - boolean caretInside = firstLine <= currentLine && currentLine <= lastLine; + boolean caretInside; + if (myEditor.getUserData(ALLOW_FOLDING_ON_CARET_LINE_KEY) == Boolean.TRUE) { + caretInside = FoldingUtil.caretInsideRange(myEditor, range); + } + else { + final Document document = myEditor.getDocument(); + final int firstLine = document.getLineNumber(range.getStartOffset()); + final int lastLine = document.getLineNumber(range.getEndOffset()); + final int currentLine = document.getLineNumber(myEditor.getCaretModel().getOffset()); + caretInside = firstLine <= currentLine && currentLine <= lastLine; + } if (myApplyDefaultState) { return caretInside || !FoldingPolicy.isCollapseByDefault(element);