From 890d7f74fe0620270bfbca44ef90823ecac92918 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Thu, 23 Sep 2010 13:18:57 +0400 Subject: [PATCH] IDEA-59034 Editor: Modify default wrap strategy in order to prefer wrap on comma if possible 1. Generic customizable line wrap strategy is introduced; 2. Existing default line wrap strategy is adapted to the new generic strategy API; 3. Added test for preferring comma during wrapping; --- .../DefaultLineWrapPositionStrategy.java | 123 +--------- .../GenericLineWrapPositionStrategy.java | 222 ++++++++++++++++++ .../DefaultLineWrapPositionStrategyTest.java | 54 +++-- 3 files changed, 269 insertions(+), 130 deletions(-) create mode 100644 platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java diff --git a/platform/platform-api/src/com/intellij/openapi/editor/DefaultLineWrapPositionStrategy.java b/platform/platform-api/src/com/intellij/openapi/editor/DefaultLineWrapPositionStrategy.java index 65d2ece59df7..e187fc961133 100644 --- a/platform/platform-api/src/com/intellij/openapi/editor/DefaultLineWrapPositionStrategy.java +++ b/platform/platform-api/src/com/intellij/openapi/editor/DefaultLineWrapPositionStrategy.java @@ -15,9 +15,6 @@ */ package com.intellij.openapi.editor; -import gnu.trove.TIntHashSet; -import org.jetbrains.annotations.NotNull; - /** * Default {@link LineWrapPositionStrategy} implementation. Is assumed to provide language-agnostic algorithm that may * be used with almost any kind of text. @@ -25,116 +22,22 @@ import org.jetbrains.annotations.NotNull; * @author Denis Zhdanov * @since Aug 25, 2010 11:33:00 AM */ -public class DefaultLineWrapPositionStrategy implements LineWrapPositionStrategy { +public class DefaultLineWrapPositionStrategy extends GenericLineWrapPositionStrategy { - /** Contains white space characters (has special treatment during soft wrap position calculation). */ - private static final TIntHashSet WHITE_SPACES = new TIntHashSet(); - static { - WHITE_SPACES.add(' '); - WHITE_SPACES.add('\t'); - } + public DefaultLineWrapPositionStrategy() { + // Commas. + addRule(new Rule(',', WrapCondition.AFTER, Rule.DEFAULT_WEIGHT * 2)); - /** - * Contains symbols that are special in that soft wrap is allowed to be performed only - * after them (not before). - */ - private static final TIntHashSet SPECIAL_SYMBOLS_TO_WRAP_AFTER = new TIntHashSet(); - static { - SPECIAL_SYMBOLS_TO_WRAP_AFTER.add(','); - SPECIAL_SYMBOLS_TO_WRAP_AFTER.add(';'); - SPECIAL_SYMBOLS_TO_WRAP_AFTER.add(')'); - } + // Symbols to wrap either before or after. + addRule(new Rule(' ')); + addRule(new Rule('\t')); - /** - * Contains symbols that are special in that soft wrap is allowed to be performed only - * before them (not after). - */ - private static final TIntHashSet SPECIAL_SYMBOLS_TO_WRAP_BEFORE = new TIntHashSet(); - static { - SPECIAL_SYMBOLS_TO_WRAP_BEFORE.add('('); - SPECIAL_SYMBOLS_TO_WRAP_BEFORE.add('.'); - } + // Symbols to wrap after. + addRule(new Rule(';', WrapCondition.AFTER)); + addRule(new Rule(')', WrapCondition.AFTER)); - @Override - public int calculateWrapPosition(@NotNull CharSequence text, - int startOffset, - int endOffset, - final int maxPreferredOffset, - boolean allowToBeyondMaxPreferredOffset) - { - if (endOffset <= startOffset) { - return endOffset; - } - - // Normalization. - int maxPreferredOffsetToUse = maxPreferredOffset >= endOffset ? endOffset - 1 : maxPreferredOffset; - maxPreferredOffsetToUse = maxPreferredOffsetToUse < startOffset ? startOffset : maxPreferredOffsetToUse; - - // Try to find target offset that is not greater than preferred position. - for (int i = maxPreferredOffsetToUse; i > startOffset; i--) { - char c = text.charAt(i); - if (c == '\n') { - return i + 1; - } - - if (WHITE_SPACES.contains(c)) { - return i < maxPreferredOffsetToUse ? i + 1 : i; - } - - // Don't wrap on the non-id symbol preceded by another non-id symbol. E.g. consider that we have a statement - // like 'foo(int... args)'. We don't want to wrap on the second or third dots then. - if (i > startOffset + 1 && !isIdSymbol(c) && !isIdSymbol(text.charAt(i - 1))) { - continue; - } - if (SPECIAL_SYMBOLS_TO_WRAP_AFTER.contains(c)) { - if (i < maxPreferredOffsetToUse) { - return i + 1; - } - continue; - } - if (SPECIAL_SYMBOLS_TO_WRAP_BEFORE.contains(c) || WHITE_SPACES.contains(c)) { - return i; - } - - // Don't wrap on a non-id symbol followed by non-id symbol, e.g. don't wrap between two pluses at i++. - // Also don't wrap before non-id symbol preceded by a space - wrap on space instead; - if (!isIdSymbol(c) && (i < startOffset + 2 || (isIdSymbol(text.charAt(i - 1)) && !WHITE_SPACES.contains(text.charAt(i - 1))))) { - return i; - } - } - - // Try to find target offset that is greater than preferred position. - for (int i = maxPreferredOffsetToUse + 1; i < endOffset; i++) { - char c = text.charAt(i); - if (c == '\n') { - return i + 1; - } - - if (WHITE_SPACES.contains(c)) { - return i; - } - // Don't wrap on the non-id symbol preceded by another non-id symbol. E.g. consider that we have a statement - // like 'foo(int... args)'. We don't want to wrap on the second or third dots then. - if (i < endOffset - 1 && !isIdSymbol(c) && !isIdSymbol(text.charAt(i + 1)) && !isIdSymbol(text.charAt(i - 1))) { - continue; - } - if (SPECIAL_SYMBOLS_TO_WRAP_BEFORE.contains(c)) { - return i; - } - if (SPECIAL_SYMBOLS_TO_WRAP_AFTER.contains(c) && i < endOffset - 1) { - return i + 1; - } - - // Don't wrap on a non-id symbol followed by non-id symbol, e.g. don't wrap between two pluses at i++; - if (!isIdSymbol(c) && (i >= endOffset - 1 || isIdSymbol(text.charAt(i + 1)))) { - return i; - } - } - - return allowToBeyondMaxPreferredOffset ? endOffset : maxPreferredOffset; - } - - private static boolean isIdSymbol(char c) { - return c == '_' || c == '$' || (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); + // Symbols to wrap before + addRule(new Rule('(', WrapCondition.BEFORE)); + addRule(new Rule('.', WrapCondition.BEFORE)); } } diff --git a/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java b/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java new file mode 100644 index 000000000000..6ff4c11aab7e --- /dev/null +++ b/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java @@ -0,0 +1,222 @@ +/* + * Copyright 2000-2010 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.openapi.editor; + +import gnu.trove.TIntIntHashMap; +import gnu.trove.TIntIntProcedure; +import gnu.trove.TIntObjectHashMap; +import org.jetbrains.annotations.NotNull; + +/** + * Highly customizable {@link LineWrapPositionStrategy} implementation. + *

+ * Not thread-safe. + * + * @author Denis Zhdanov + * @since Sep 23, 2010 12:04:52 PM + */ +public class GenericLineWrapPositionStrategy implements LineWrapPositionStrategy { + + private static final TIntIntProcedure GT_COMPARATOR = new TIntIntProcedure() { + @Override + public boolean execute(int a, int b) { + return a > b; + } + }; + + //private static final TIntIntProcedure LT_COMPARATOR = new TIntIntProcedure() { + // @Override + // public boolean execute(int a, int b) { + // return a < b; + // } + //}; + + /** + * We consider that it's possible to wrap line on non-id symbol. However, weight of such position is expected to be less + * than weight of wrap position bound to explicitly configured symbol. + */ + private static final int NON_ID_WEIGHT = (Rule.DEFAULT_WEIGHT - 1) / 2; + + /** Holds symbols wrap rules by symbol. */ + private final TIntObjectHashMap myRules = new TIntObjectHashMap(); + + @Override + public int calculateWrapPosition(@NotNull CharSequence text, + int startOffset, + int endOffset, + int maxPreferredOffset, + boolean allowToBeyondMaxPreferredOffset) + { + if (endOffset <= startOffset) { + return endOffset; + } + + // Normalization. + int maxPreferredOffsetToUse = maxPreferredOffset >= endOffset ? endOffset - 1 : maxPreferredOffset; + maxPreferredOffsetToUse = maxPreferredOffsetToUse < startOffset ? startOffset : maxPreferredOffsetToUse; + + TIntIntHashMap offset2Weight = new TIntIntHashMap(); + + // Try to find out wrap position before preferred offset. + for (int i = maxPreferredOffsetToUse; i > startOffset; i--) { + char c = text.charAt(i); + if (c == '\n') { + return i + 1; + } + + Rule rule = myRules.get(c); + if (rule != null) { + switch (rule.condition) { + case BOTH: + case BEFORE: offset2Weight.put(i, rule.weight); break; + case AFTER: if (i < maxPreferredOffsetToUse) offset2Weight.put(i + 1, rule.weight); + } + continue; + } + + // Don't wrap on a non-id symbol followed by non-id symbol, e.g. don't wrap between two pluses at i++. + // Also don't wrap before non-id symbol preceded by a space - wrap on space instead; + if (!isIdSymbol(c) && (i < startOffset + 2 || (isIdSymbol(text.charAt(i - 1)) && !myRules.contains(text.charAt(i - 1))))) { + offset2Weight.put(i, NON_ID_WEIGHT); + } + } + + int result = chooseOffset(offset2Weight, GT_COMPARATOR); + if (result > 0) { + return result; + } + + // Try to find target offset that is beyond preferred offset. + // Note that we don't consider symbol weights here and just break on the first appropriate position. + if (!allowToBeyondMaxPreferredOffset) { + return maxPreferredOffset; + } + for (int i = maxPreferredOffsetToUse + 1; i < endOffset; i++) { + char c = text.charAt(i); + if (c == '\n') { + return i + 1; + } + + Rule rule = myRules.get(c); + if (rule != null) { + switch (rule.condition) { + case BOTH: + case BEFORE: return i; + case AFTER: if (i < endOffset - 1) return i + 1; + } + } + + // Don't wrap on a non-id symbol followed by non-id symbol, e.g. don't wrap between two pluses at i++; + if (!isIdSymbol(c) && (i >= endOffset - 1 || isIdSymbol(text.charAt(i + 1)))) { + return i; + } + } + + return maxPreferredOffsetToUse; + } + + /** + * Registers given rule with the current strategy. + * + * @param rule rule to register + * @throws IllegalArgumentException if another rule for the same symbol is already registered within the current strategy + */ + public void addRule(@NotNull Rule rule) throws IllegalArgumentException { + Rule existing = myRules.get(rule.symbol); + if (existing != null) { + throw new IllegalArgumentException(String.format( + "Can't register given wrap rule (%s) within the current line wrap position strategy. Reason: another rule is already " + + "registered for it - '%s'", rule, existing + )); + } + existing = myRules.put(rule.symbol, rule); + assert existing == null; + } + + private static boolean isIdSymbol(char c) { + return c == '_' || c == '$' || (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); + } + + /** + * Tries to derive offset to use at the given map assuming that it contains mappings like '{@code offset -> weight}'. + * + * @param offset2Weight map that holds '{@code offset -> weight}' entries (is allows to be empty) + * @param comparator strategy interface that is expected to return 'true' if the first parameter + * given to it is more preferred than the second + * @return one of the keys of the given map to use; negative value if no appropriate key is found or the map is empty + */ + private static int chooseOffset(@NotNull TIntIntHashMap offset2Weight, @NotNull final TIntIntProcedure comparator) { + if (offset2Weight.isEmpty()) { + return -1; + } + + final int[] resultingWeight = new int[1]; + final int[] resultingOffset = new int[1]; + offset2Weight.forEachEntry(new TIntIntProcedure() { + @Override + public boolean execute(int offset, int weight) { + if (weight > resultingWeight[0]) { + resultingWeight[0] = weight; + resultingOffset[0] = offset; + } + else if (weight == resultingWeight[0] && comparator.execute(offset, resultingOffset[0])) { + resultingOffset[0] = offset; + } + return true; + } + }); + return resultingOffset[0]; + } + + /** + * Defines how wrapping may be performed for particular symbol. + * + * @see Rule + */ + public enum WrapCondition { + AFTER, BEFORE, BOTH + } + + /** + * Encapsulates information about rule to use during line wrapping. + */ + public static class Rule { + + public static final int DEFAULT_WEIGHT = 10; + + public final char symbol; + public final WrapCondition condition; + public final int weight; + + public Rule(char symbol) { + this(symbol, WrapCondition.BOTH, DEFAULT_WEIGHT); + } + + public Rule(char symbol, WrapCondition condition) { + this(symbol, condition, DEFAULT_WEIGHT); + } + + public Rule(char symbol, int weight) { + this(symbol, WrapCondition.BOTH, weight); + } + + public Rule(char symbol, WrapCondition condition, int weight) { + this.symbol = symbol; + this.condition = condition; + this.weight = weight; + } + } +} diff --git a/platform/platform-api/testSrc/com/intellij/openapi/editor/DefaultLineWrapPositionStrategyTest.java b/platform/platform-api/testSrc/com/intellij/openapi/editor/DefaultLineWrapPositionStrategyTest.java index be8fcf62cebf..a09fd08dc452 100644 --- a/platform/platform-api/testSrc/com/intellij/openapi/editor/DefaultLineWrapPositionStrategyTest.java +++ b/platform/platform-api/testSrc/com/intellij/openapi/editor/DefaultLineWrapPositionStrategyTest.java @@ -18,13 +18,15 @@ package com.intellij.openapi.editor; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertSame; + /** * @author Denis Zhdanov * @since Aug 25, 2010 3:20:41 PM */ public class DefaultLineWrapPositionStrategyTest { - private static final String MAX_PREFERRED_MARKER = ""; + private static final String EDGE_MARKER = ""; private static final String WRAP_MARKER = ""; private DefaultLineWrapPositionStrategy myStrategy; @@ -37,14 +39,21 @@ public class DefaultLineWrapPositionStrategyTest { @Test public void commaNotSeparated() { String document = - "void method(String p1, String p2) {}"; - doTest(document); + "void method(String p1, String p2) {}"; + doTest(document, false); } @Test public void wrapOnExceedingWhiteSpace() { String document = - "void method(String p1, String p2) {}"; + "void method(String p1, String p2) {}"; + doTest(document); + } + + @Test + public void preferWrapOnComma() { + String document = + "int variable = testMethod(var1 + var2, var3 + var4);"; doTest(document); } @@ -55,9 +64,10 @@ public class DefaultLineWrapPositionStrategyTest { private void doTest(final String document, boolean allowToBeyondMaxPreferredOffset) { final Context context = new Context(document); context.init(); - myStrategy.calculateWrapPosition( - context.document, 0, context.document.length(), context.preferredIndex, allowToBeyondMaxPreferredOffset + int actual = myStrategy.calculateWrapPosition( + context.document, 0, context.document.length(), context.edgeIndex, allowToBeyondMaxPreferredOffset ); + assertSame(context.wrapIndex, actual); } /** @@ -73,7 +83,9 @@ public class DefaultLineWrapPositionStrategyTest { private String document; private int index; private int wrapIndex; - private int preferredIndex; + private int tmpWrapIndex; + private int edgeIndex; + private int tmpEdgeIndex; Context(String rawDocument) { if (rawDocument.contains("\n")) { @@ -85,10 +97,10 @@ public class DefaultLineWrapPositionStrategyTest { } public void init() { - wrapIndex = rawDocument.indexOf(WRAP_MARKER); - preferredIndex = rawDocument.indexOf(MAX_PREFERRED_MARKER); - if (wrapIndex >= 0 && preferredIndex >= 0) { - if (wrapIndex < preferredIndex) { + tmpWrapIndex = rawDocument.indexOf(WRAP_MARKER); + tmpEdgeIndex = rawDocument.indexOf(EDGE_MARKER); + if (tmpWrapIndex >= 0 && tmpEdgeIndex >= 0) { + if (tmpWrapIndex < tmpEdgeIndex) { processWrap(); processMaxPreferredIndex(); } @@ -98,33 +110,35 @@ public class DefaultLineWrapPositionStrategyTest { } } else { - if (wrapIndex >= 0) { + if (tmpWrapIndex >= 0) { processWrap(); } - if (preferredIndex >= 0) { + if (tmpEdgeIndex >= 0) { processMaxPreferredIndex(); } } buffer.append(rawDocument.substring(index)); document = buffer.toString(); - if (preferredIndex <= 0) { - preferredIndex = document.length(); + if (edgeIndex <= 0) { + edgeIndex = document.length(); } } private void processWrap() { - buffer.append(rawDocument.substring(index, wrapIndex)); - index = wrapIndex + WRAP_MARKER.length(); + buffer.append(rawDocument.substring(index, tmpWrapIndex)); + index = tmpWrapIndex + WRAP_MARKER.length(); + wrapIndex = buffer.length(); if (rawDocument.indexOf(WRAP_MARKER, index) >= 0) { throw new IllegalArgumentException(String.format("More than one wrap indicator is found at the document '%s'", rawDocument)); } } private void processMaxPreferredIndex() { - buffer.append(rawDocument.substring(index, preferredIndex)); - index = preferredIndex + MAX_PREFERRED_MARKER.length(); - if (rawDocument.indexOf(MAX_PREFERRED_MARKER, index) >= 0) { + buffer.append(rawDocument.substring(index, tmpEdgeIndex)); + index = tmpEdgeIndex + EDGE_MARKER.length(); + edgeIndex = buffer.length(); + if (rawDocument.indexOf(EDGE_MARKER, index) >= 0) { throw new IllegalArgumentException(String.format("More than one max preferred offset is found at the document '%s'", rawDocument)); } }