From 53b9222ced97950c981ff4a76bc38dc44d722b3b Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 16 May 2014 14:18:58 +0400 Subject: [PATCH] processOverlapping should iterate in order --- .../injected/editor/MarkupModelWindow.java | 7 ---- .../openapi/editor/ex/MarkupModelEx.java | 2 - .../openapi/editor/impl/EmptyMarkupModel.java | 6 --- .../openapi/editor/impl/MarkupModelImpl.java | 40 +++++++------------ .../openapi/editor/impl/RangeMarkerTest.java | 22 ++++++---- .../util/containers/ContainerUtil.java | 37 ++++++++++++----- 6 files changed, 56 insertions(+), 58 deletions(-) diff --git a/platform/editor-ui-ex/src/com/intellij/injected/editor/MarkupModelWindow.java b/platform/editor-ui-ex/src/com/intellij/injected/editor/MarkupModelWindow.java index d60f12839e75..2f9d331f8a13 100644 --- a/platform/editor-ui-ex/src/com/intellij/injected/editor/MarkupModelWindow.java +++ b/platform/editor-ui-ex/src/com/intellij/injected/editor/MarkupModelWindow.java @@ -21,7 +21,6 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.ex.DisposableIterator; import com.intellij.openapi.editor.ex.MarkupModelEx; import com.intellij.openapi.editor.ex.RangeHighlighterEx; -import com.intellij.openapi.editor.ex.SweepProcessor; import com.intellij.openapi.editor.impl.event.MarkupModelListener; import com.intellij.openapi.editor.markup.HighlighterTargetArea; import com.intellij.openapi.editor.markup.RangeHighlighter; @@ -151,12 +150,6 @@ public class MarkupModelWindow extends UserDataHolderBase implements MarkupModel return myHostModel.overlappingIterator(startOffset, endOffset); } - @Override - public boolean sweep(int start, int end, @NotNull SweepProcessor sweepProcessor) { - // todo convert - return myHostModel.sweep(start, end, sweepProcessor); - } - @Override public void fireAttributesChanged(@NotNull RangeHighlighterEx segmentHighlighter, boolean renderersChanged) { diff --git a/platform/editor-ui-ex/src/com/intellij/openapi/editor/ex/MarkupModelEx.java b/platform/editor-ui-ex/src/com/intellij/openapi/editor/ex/MarkupModelEx.java index 2d5a437bcf2d..e80652347ca6 100644 --- a/platform/editor-ui-ex/src/com/intellij/openapi/editor/ex/MarkupModelEx.java +++ b/platform/editor-ui-ex/src/com/intellij/openapi/editor/ex/MarkupModelEx.java @@ -72,6 +72,4 @@ public interface MarkupModelEx extends MarkupModel { // runs change attributes action and fires highlighterChanged event if there were changes void changeAttributesInBatch(@NotNull RangeHighlighterEx highlighter, @NotNull Consumer changeAttributesAction); - - boolean sweep(int start, int end, @NotNull final SweepProcessor sweepProcessor); } diff --git a/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/EmptyMarkupModel.java b/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/EmptyMarkupModel.java index 52fdbf1030a9..a87dc9aeaa93 100644 --- a/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/EmptyMarkupModel.java +++ b/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/EmptyMarkupModel.java @@ -20,7 +20,6 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.ex.DisposableIterator; import com.intellij.openapi.editor.ex.MarkupModelEx; import com.intellij.openapi.editor.ex.RangeHighlighterEx; -import com.intellij.openapi.editor.ex.SweepProcessor; import com.intellij.openapi.editor.impl.event.MarkupModelListener; import com.intellij.openapi.editor.markup.HighlighterTargetArea; import com.intellij.openapi.editor.markup.RangeHighlighter; @@ -144,11 +143,6 @@ public class EmptyMarkupModel implements MarkupModelEx { return DisposableIterator.EMPTY; } - @Override - public boolean sweep(int start, int end, @NotNull SweepProcessor sweepProcessor) { - return false; - } - @Override public void fireAttributesChanged(@NotNull RangeHighlighterEx segmentHighlighter, boolean renderersChanged) { diff --git a/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/MarkupModelImpl.java b/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/MarkupModelImpl.java index d2c5c96141f0..bf30bae53eca 100644 --- a/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/MarkupModelImpl.java +++ b/platform/editor-ui-ex/src/com/intellij/openapi/editor/impl/MarkupModelImpl.java @@ -44,7 +44,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.NoSuchElementException; @@ -258,9 +257,18 @@ public class MarkupModelImpl extends UserDataHolderBase implements MarkupModelEx @Override public boolean processRangeHighlightersOverlappingWith(int start, int end, @NotNull Processor processor) { - if (!myHighlighterTree.processOverlappingWith(start, end, processor)) return false; - TextRangeInterval lines = roundToLineBoundaries(start, end); - return myHighlighterTreeForLines.processOverlappingWith(lines.getStartOffset(), lines.getEndOffset(), processor); + DisposableIterator iterator = overlappingIterator(start, end); + try { + while (iterator.hasNext()) { + if (!processor.process(iterator.next())) { + return false; + } + } + return true; + } + finally { + iterator.dispose(); + } } @Override @@ -272,7 +280,8 @@ public class MarkupModelImpl extends UserDataHolderBase implements MarkupModelEx @Override @NotNull public DisposableIterator overlappingIterator(int startOffset, int endOffset) { - IntervalTreeImpl.PeekableIterator exact = myHighlighterTree.overlappingIterator(new TextRangeInterval(startOffset, endOffset)); + startOffset = Math.max(0,startOffset); + IntervalTreeImpl.PeekableIterator exact = myHighlighterTree.overlappingIterator(new TextRangeInterval(startOffset, Math.max(startOffset, endOffset))); IntervalTreeImpl.PeekableIterator lines = myHighlighterTreeForLines.overlappingIterator(roundToLineBoundaries(startOffset, endOffset)); return merge(exact, lines); } @@ -319,25 +328,4 @@ public class MarkupModelImpl extends UserDataHolderBase implements MarkupModelEx int lineEndOffset = endOffset <= 0 ? 0 : endOffset >= document.getTextLength() ? document.getTextLength() : document.getLineEndOffset(document.getLineNumber(endOffset)); return new TextRangeInterval(lineStartOffset, lineEndOffset); } - - @Override - public boolean sweep(int start, int end, @NotNull SweepProcessor sweepProcessor) { - TextRangeInterval lines = roundToLineBoundaries(start, end); - List linesInRange = new ArrayList(); - myHighlighterTreeForLines.processOverlappingWith(lines.getStartOffset(), lines.getEndOffset(), new CommonProcessors.CollectProcessor(linesInRange)); - if (linesInRange.isEmpty()) { - return myHighlighterTree.sweep(start, end, sweepProcessor); - } - final List highlighters = new ArrayList(); - myHighlighterTree.processOverlappingWith(start, end, new CommonProcessors.CollectProcessor(highlighters)); - highlighters.addAll(linesInRange); - Collections.sort(highlighters, RangeHighlighterEx.BY_AFFECTED_START_OFFSET); - - return RangeMarkerTree.sweep(new RangeMarkerTree.Generator() { - @Override - public boolean generateInStartOffsetOrder(@NotNull Processor processor) { - return ContainerUtil.process(highlighters, processor); - } - }, sweepProcessor); - } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java index 925d92d93283..67f516811687 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java @@ -22,12 +22,10 @@ import com.intellij.openapi.command.undo.UndoManager; import com.intellij.openapi.editor.*; import com.intellij.openapi.editor.event.DocumentAdapter; import com.intellij.openapi.editor.event.DocumentEvent; -import com.intellij.openapi.editor.ex.DocumentEx; -import com.intellij.openapi.editor.ex.MarkupModelEx; -import com.intellij.openapi.editor.ex.RangeHighlighterEx; -import com.intellij.openapi.editor.ex.RangeMarkerEx; +import com.intellij.openapi.editor.ex.*; import com.intellij.openapi.editor.markup.HighlighterTargetArea; import com.intellij.openapi.editor.markup.MarkupModel; +import com.intellij.openapi.editor.markup.RangeHighlighter; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.ThrowableComputable; @@ -45,10 +43,7 @@ import com.intellij.util.ThrowableRunnable; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; -import java.util.ArrayList; -import java.util.List; -import java.util.Random; -import java.util.Set; +import java.util.*; /** * @author mike @@ -1090,4 +1085,15 @@ public class RangeMarkerTest extends LightPlatformTestCase { } }).assertTiming(); } + + public void testRangeHighlighterIteratorOrder() throws Exception { + Document document = EditorFactory.getInstance().createDocument("1234567890"); + + final MarkupModelEx markupModel = (MarkupModelEx)DocumentMarkupModel.forDocument(document, ourProject, true); + RangeHighlighter exact = markupModel.addRangeHighlighter(3, 6, 0, null, HighlighterTargetArea.EXACT_RANGE); + RangeHighlighter line = markupModel.addRangeHighlighter(4, 5, 0, null, HighlighterTargetArea.LINES_IN_RANGE); + List list = new ArrayList(); + markupModel.processRangeHighlightersOverlappingWith(2, 9, new CommonProcessors.CollectProcessor(list)); + assertEquals(Arrays.asList(line,exact), list); + } } diff --git a/platform/util/src/com/intellij/util/containers/ContainerUtil.java b/platform/util/src/com/intellij/util/containers/ContainerUtil.java index a01652d05dca..f08c405d576f 100644 --- a/platform/util/src/com/intellij/util/containers/ContainerUtil.java +++ b/platform/util/src/com/intellij/util/containers/ContainerUtil.java @@ -313,7 +313,7 @@ public class ContainerUtil extends ContainerUtilRt { return CHM_FACTORY.createMap(); } - public static ConcurrentMap newConcurrentMap(TObjectHashingStrategy hashStrategy) { + public static ConcurrentMap newConcurrentMap(@NotNull TObjectHashingStrategy hashStrategy) { return CHM_FACTORY.createMap(hashStrategy); } @@ -321,7 +321,7 @@ public class ContainerUtil extends ContainerUtilRt { return CHM_FACTORY.createMap(initialCapacity); } - public static ConcurrentMap newConcurrentMap(int initialCapacity, float loadFactor, int concurrencyLevel, TObjectHashingStrategy hashStrategy) { + public static ConcurrentMap newConcurrentMap(int initialCapacity, float loadFactor, int concurrencyLevel, @NotNull TObjectHashingStrategy hashStrategy) { return CHM_FACTORY.createMap(initialCapacity, loadFactor, concurrencyLevel, hashStrategy); } @@ -704,6 +704,15 @@ public class ContainerUtil extends ContainerUtilRt { return true; } + public static boolean process(@NotNull Iterator iterator, @NotNull Processor processor) { + while (iterator.hasNext()) { + if (!processor.process(iterator.next())) { + return false; + } + } + return true; + } + @Nullable public static V find(@NotNull Iterable iterable, @NotNull Condition condition) { return find(iterable.iterator(), condition); @@ -2057,45 +2066,53 @@ public class ContainerUtil extends ContainerUtilRt { } private interface ConcurrentMapFactory { - ConcurrentMap createMap(); - ConcurrentMap createMap(int initialCapacity); - ConcurrentMap createMap(TObjectHashingStrategy hashStrategy); - ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel); - ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel, TObjectHashingStrategy hashStrategy); + @NotNull ConcurrentMap createMap(); + @NotNull ConcurrentMap createMap(int initialCapacity); + @NotNull ConcurrentMap createMap(@NotNull TObjectHashingStrategy hashStrategy); + @NotNull ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel); + @NotNull ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel, @NotNull TObjectHashingStrategy hashStrategy); } private static final ConcurrentMapFactory V8_MAP_FACTORY = new ConcurrentMapFactory() { + @NotNull public ConcurrentMap createMap() { return new ConcurrentHashMap(); } + @NotNull public ConcurrentMap createMap(int initialCapacity) { return new ConcurrentHashMap(initialCapacity); } - public ConcurrentMap createMap(TObjectHashingStrategy hashStrategy) { + @NotNull + public ConcurrentMap createMap(@NotNull TObjectHashingStrategy hashStrategy) { return new ConcurrentHashMap(hashStrategy); } + @NotNull public ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel) { return new ConcurrentHashMap(initialCapacity, loadFactor, concurrencyLevel); } + @NotNull public ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel, @NotNull TObjectHashingStrategy hashingStrategy) { return new ConcurrentHashMap(initialCapacity, loadFactor, concurrencyLevel, hashingStrategy); } }; private static final ConcurrentMapFactory PLATFORM_MAP_FACTORY = new ConcurrentMapFactory() { + @NotNull public ConcurrentMap createMap() { return createMap(16, 0.75f, DEFAULT_CONCURRENCY_LEVEL); } + @NotNull public ConcurrentMap createMap(int initialCapacity) { return new java.util.concurrent.ConcurrentHashMap(initialCapacity); } - public ConcurrentMap createMap(TObjectHashingStrategy hashingStrategy) { + @NotNull + public ConcurrentMap createMap(@NotNull TObjectHashingStrategy hashingStrategy) { if (hashingStrategy != canonicalStrategy()) { throw new UnsupportedOperationException("Custom hashStrategy is not supported in java.util.concurrent.ConcurrentHashMap"); } @@ -2103,10 +2120,12 @@ public class ContainerUtil extends ContainerUtilRt { return createMap(); } + @NotNull public ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel) { return new java.util.concurrent.ConcurrentHashMap(initialCapacity, loadFactor, concurrencyLevel); } + @NotNull public ConcurrentMap createMap(int initialCapacity, float loadFactor, int concurrencyLevel, @NotNull TObjectHashingStrategy hashingStrategy) { if (hashingStrategy != canonicalStrategy()) { throw new UnsupportedOperationException("Custom hashStrategy is not supported in java.util.concurrent.ConcurrentHashMap");