diff --git a/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtilTest.groovy b/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtilTest.groovy index 5437624113ca..cd4e63ae9961 100644 --- a/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtilTest.groovy +++ b/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtilTest.groovy @@ -17,6 +17,7 @@ package com.intellij.application.options.codeStyle.arrangement import com.intellij.psi.codeStyle.arrangement.model.ArrangementSettingType import com.intellij.psi.codeStyle.arrangement.model.ArrangementSettingsAtomNode +import gnu.trove.TIntIntHashMap import org.jetbrains.annotations.NotNull import org.jetbrains.annotations.Nullable import org.junit.Test @@ -41,10 +42,8 @@ one = '1' { '2' { '3'() four = '4'() - '5'() - } - '6'() - } + '5'()} + '6'()} } // Modify. @@ -59,22 +58,55 @@ four = '4'() '0' { '1' { '2' { - '3'() - } + '3'()} '4'() '2' { - '5'() - } + '5'()} + '6'()} + } + assertNodesEqual(expected, initial) + checkRowMappings([5 : 6, 6 : 7], rowMappings) + } + + @Test + void replaceWithMergeToNodeAbove() { + // Init. + def from; + def to; + def initial = new TreeNodeBuilder(). + '0' { + '1'() { + '2'() + '3'()} +from = '4' { +to = '5'() + '6'() + } + + } + + // Modify. + def replacement = new TreeNodeBuilder(). + '1' { + '5'() + } + def rowMappings = doReplace(initial, from, to, replacement) + + // Check. + def expected = new TreeNodeBuilder(). + '0' { + '1' { + '2'() + '3'() + '5'()} + '4' { '6'() } } assertNodesEqual(expected, initial) - - assertEquals(2, rowMappings.size()) - assertEquals(6, rowMappings.get(5)) - assertEquals(7, rowMappings.get(6)) + checkRowMappings([:], rowMappings) } - + @Test void addWithoutMergeAbove() { def initial = new TreeNodeBuilder(). @@ -242,6 +274,11 @@ four = '4'() assertNodesEqual(expected.getChildAt(i), actual.getChildAt(i)) } } + + private static void checkRowMappings(@NotNull Map expected, @NotNull TIntIntHashMap actual) { + assertEquals(expected.size(), actual.size()) + expected.each {key, value -> assertEquals(value, actual.get(key)) } + } } public class TreeNodeBuilder extends BuilderSupport { diff --git a/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImplTest.java b/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImplTest.java index 4ba822e30d59..d1dbdfa8a935 100644 --- a/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImplTest.java +++ b/java/java-tests/testSrc/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImplTest.java @@ -15,13 +15,9 @@ */ package com.intellij.application.options.codeStyle.arrangement; -import com.intellij.openapi.util.Ref; import com.intellij.psi.codeStyle.arrangement.model.ArrangementSettingsNode; -import gnu.trove.TObjectProcedure; import org.junit.Test; -import javax.swing.tree.DefaultMutableTreeNode; - import static com.intellij.psi.codeStyle.arrangement.ArrangementUtil.and; import static com.intellij.psi.codeStyle.arrangement.match.ArrangementEntryType.FIELD; import static com.intellij.psi.codeStyle.arrangement.match.ArrangementModifier.PUBLIC; @@ -51,14 +47,13 @@ public class ArrangementRuleEditingModelImplTest extends AbstractArrangementRule @Test public void buildNewSingleLevel() { configure(atom(PUBLIC)); - ArrangementRuleEditingModel model = myRowMappings.get(1); + ArrangementRuleEditingModelImpl model = myRowMappings.get(1); assertNotNull(model); assertEquals(1, myRowMappings.size()); model.addAndCondition(atom(FIELD)); - assertEquals(1, myRowMappings.size()); - assertSame(model, myRowMappings.get(2)); + assertEquals(2, model.getRow()); assertEquals(and(atom(FIELD), atom(PUBLIC)), model.getSettingsNode()); ArrangementTreeNode fieldNode = myRoot.getFirstChild(); @@ -117,14 +112,13 @@ public class ArrangementRuleEditingModelImplTest extends AbstractArrangementRule @Test public void removeLastRowCondition() { configure(and(atom(FIELD), atom(PUBLIC))); - ArrangementRuleEditingModel model = myRowMappings.get(2); + ArrangementRuleEditingModelImpl model = myRowMappings.get(2); assertNotNull(model); assertEquals(1, myRowMappings.size()); model.removeAndCondition(atom(PUBLIC)); - assertEquals(1, myRowMappings.size()); - assertSame(model, myRowMappings.get(1)); + assertEquals(1, model.getRow()); assertEquals(atom(FIELD), model.getSettingsNode()); ArrangementTreeNode fieldNode = myRoot.getFirstChild(); @@ -134,26 +128,24 @@ public class ArrangementRuleEditingModelImplTest extends AbstractArrangementRule assertEquals(0, fieldNode.getChildCount()); } - // TODO den uncomment - //@Test + @Test public void removeFirstRowConditionFromMultiChildrenParent() { configure(and(atom(FIELD), atom(PUBLIC))); configure(and(atom(FIELD), atom(STATIC))); - ArrangementRuleEditingModel modelToChange = myRowMappings.get(2); + ArrangementRuleEditingModelImpl modelToChange = myRowMappings.get(2); assertNotNull(modelToChange); - ArrangementRuleEditingModel siblingModel = myRowMappings.get(3); + ArrangementRuleEditingModelImpl siblingModel = myRowMappings.get(3); assertNotNull(siblingModel); assertEquals(2, myRowMappings.size()); modelToChange.removeAndCondition(atom(PUBLIC)); - assertEquals(2, myRowMappings.size()); - assertSame(modelToChange, myRowMappings.get(1)); + assertSame(1, modelToChange.getRow()); assertEquals(atom(FIELD), modelToChange.getSettingsNode()); - assertSame(siblingModel, myRowMappings.get(3)); + assertSame(3, siblingModel.getRow()); assertEquals(and(atom(FIELD), atom(STATIC)), siblingModel.getSettingsNode()); ArrangementTreeNode atomFieldNode = myRoot.getFirstChild(); @@ -167,8 +159,6 @@ public class ArrangementRuleEditingModelImplTest extends AbstractArrangementRule ArrangementTreeNode staticNode = layeredFieldNode.getFirstChild(); assertNotNull(staticNode); assertEquals(atom(STATIC), staticNode.getBackingSetting()); - - checkTreeNodesConsistency(); } @Test @@ -203,26 +193,5 @@ public class ArrangementRuleEditingModelImplTest extends AbstractArrangementRule ArrangementTreeNode atomFieldNode = compositeFieldNode.getNextSibling(); assertNotNull(atomFieldNode); assertEquals(atom(FIELD), atomFieldNode.getBackingSetting()); - - checkTreeNodesConsistency(); - } - - private void checkTreeNodesConsistency() { - final Ref rootRef = new Ref(); - myRowMappings.forEachValue(new TObjectProcedure() { - @Override - public boolean execute(ArrangementRuleEditingModelImpl model) { - ArrangementTreeNode root = ArrangementConfigUtil.getRoot(model.getTopMost()); - assertSame(root, ArrangementConfigUtil.getRoot(model.getBottomMost())); - - if (rootRef.get() == null) { - rootRef.set(root); - } - else { - assertSame(rootRef.get(), root); - } - return true; - } - }); } } diff --git a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtil.java b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtil.java index f6d2d9a6943e..750bb3149e6d 100644 --- a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtil.java +++ b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementConfigUtil.java @@ -218,8 +218,8 @@ public class ArrangementConfigUtil { * Utility method which helps to replace node sub-hierarchy identified by the given start and end nodes (inclusive) by * a sub-hierarchy which is denoted by the given root. * - * @param from indicates start of the node sub-hierarchy to be replaced (inclusive) - * @param to indicates end of the node sub-hierarchy to be replaced (inclusive) + * @param from indicates start of the node sub-hierarchy (top-most node) to be replaced (inclusive) + * @param to indicates end of the node sub-hierarchy (bottom-most node) to be replaced (inclusive) * @param replacement root of the node sub-hierarchy which should replace the one identified by the given 'start' and 'end' nodes * @param treeModel model which should hold ui nodes * @param rootVisible determines if the root should be count during rows calculations @@ -323,8 +323,8 @@ public class ArrangementConfigUtil { cutHierarchy = parentCopy; } //endregion - - int insertionIndex = root.getIndex(from) + 1; + + int childCountBefore = root.getChildCount(); //region Remove target sub-hierarchy for (ArrangementTreeNode current = to; current != root;) { @@ -338,6 +338,7 @@ public class ArrangementConfigUtil { //endregion //region Insert nodes. + int insertionIndex = root.getChildCount() < childCountBefore ? childCountBefore - 1 : childCountBefore; boolean merged = insert(root, insertionIndex, replacement, treeModel); if (cutHierarchy != null) { insert(root, insertionIndex + (merged ? 0 : 1), cutHierarchy, treeModel); @@ -483,13 +484,14 @@ public class ArrangementConfigUtil { if (child.getChildCount() <= 0) { // Don't merge the last child. treeModel.insertNodeInto(child, parent, index); + return false; } boolean anchorAbove = false; ArrangementTreeNode mergeCandidate = null; if (index > 0) { mergeCandidate = parent.getChildAt(index - 1); - if (!hasEqualSetting(mergeCandidate, child)) { + if (mergeCandidate.getChildCount() <= 0 /* don't merge into leaf node*/ || !hasEqualSetting(mergeCandidate, child)) { mergeCandidate = null; } } diff --git a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImpl.java b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImpl.java index 664c39b8b8e0..82c8d8c917f8 100644 --- a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImpl.java +++ b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleEditingModelImpl.java @@ -99,6 +99,10 @@ public class ArrangementRuleEditingModelImpl implements ArrangementRuleEditingMo return mySettingsNode; } + public int getRow() { + return myRow; + } + @NotNull public ArrangementTreeNode getTopMost() { return myTopMost; diff --git a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleTree.java b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleTree.java index d53fe9d8d7cf..29ed01a786ad 100644 --- a/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleTree.java +++ b/platform/lang-impl/src/com/intellij/application/options/codeStyle/arrangement/ArrangementRuleTree.java @@ -51,6 +51,7 @@ public class ArrangementRuleTree { @NotNull private final List myListeners = new ArrayList(); @NotNull private final TreeSelectionModel mySelectionModel = new MySelectionModel(); @NotNull private final MyModelChangeListener myModelChangeListener = new MyModelChangeListener(); + @NotNull private final MyModelNodesRefresher myModelNodesRefresher = new MyModelNodesRefresher(); @NotNull private final TIntObjectHashMap myRenderers = new TIntObjectHashMap(); @@ -398,8 +399,12 @@ public class ArrangementRuleTree { } private void onModelChange(@NotNull ArrangementRuleEditingModelImpl model, @NotNull final TIntIntHashMap rowChanges) { + // Refresh models. + myModels.forEachValue(myModelNodesRefresher); + // Shift row-based caches. - final TIntObjectHashMap changedModelMappings = new TIntObjectHashMap(); + final TIntObjectHashMap changedModelMappings = + new TIntObjectHashMap(); final TIntObjectHashMap changedRendererMappings = new TIntObjectHashMap(); rowChanges.forEachEntry(new TIntIntProcedure() { @Override @@ -526,4 +531,12 @@ public class ArrangementRuleTree { onModelChange(model, rowChanges); } } + + private class MyModelNodesRefresher implements TObjectProcedure { + @Override + public boolean execute(ArrangementRuleEditingModelImpl model) { + model.refreshTreeNodes(); + return true; + } + } }