From 981015db8f627a5dd32ea8b5010cb11ea381f831 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Mon, 16 Jan 2017 15:29:17 +0300 Subject: [PATCH] Highlight tokens in console using regular range highlighters instead of artificial range markers-based EditorHighlighter (because the latter doesn't react properly on attributes changes) to fix Console IDEA-166441 Unexpected text coloring in console --- .../execution/impl/ConsoleViewImpl.java | 166 +++++------------- .../execution/impl/ConsoleViewImplTest.java | 8 +- .../process/AnsiEscapeDecoderTest.java | 13 +- 3 files changed, 54 insertions(+), 133 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/execution/impl/ConsoleViewImpl.java b/platform/lang-impl/src/com/intellij/execution/impl/ConsoleViewImpl.java index 3299548d1bce..9405125d3c3b 100644 --- a/platform/lang-impl/src/com/intellij/execution/impl/ConsoleViewImpl.java +++ b/platform/lang-impl/src/com/intellij/execution/impl/ConsoleViewImpl.java @@ -43,19 +43,16 @@ import com.intellij.openapi.editor.*; import com.intellij.openapi.editor.actionSystem.*; import com.intellij.openapi.editor.actions.ScrollToTheEndToolbarAction; import com.intellij.openapi.editor.actions.ToggleUseSoftWrapsToolbarAction; -import com.intellij.openapi.editor.colors.EditorColorsScheme; -import com.intellij.openapi.editor.event.DocumentAdapter; import com.intellij.openapi.editor.event.EditorMouseEvent; import com.intellij.openapi.editor.ex.DocumentEx; import com.intellij.openapi.editor.ex.EditorEx; +import com.intellij.openapi.editor.ex.MarkupModelEx; import com.intellij.openapi.editor.ex.util.EditorUtil; -import com.intellij.openapi.editor.highlighter.EditorHighlighter; -import com.intellij.openapi.editor.highlighter.HighlighterClient; -import com.intellij.openapi.editor.highlighter.HighlighterIterator; import com.intellij.openapi.editor.impl.DocumentImpl; +import com.intellij.openapi.editor.impl.DocumentMarkupModel; import com.intellij.openapi.editor.impl.RangeMarkerImpl; import com.intellij.openapi.editor.impl.softwrap.SoftWrapAppliancePlaces; -import com.intellij.openapi.editor.markup.TextAttributes; +import com.intellij.openapi.editor.markup.*; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.fileEditor.OpenFileDescriptor; import com.intellij.openapi.ide.CopyPasteManager; @@ -68,11 +65,9 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.*; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.search.GlobalSearchScope; -import com.intellij.psi.tree.IElementType; import com.intellij.ui.awt.RelativePoint; import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; -import com.intellij.util.containers.Queue; import com.intellij.util.text.CharArrayUtil; import com.intellij.util.ui.UIUtil; import gnu.trove.THashSet; @@ -91,7 +86,7 @@ import java.awt.event.MouseWheelEvent; import java.io.IOException; import java.util.*; import java.util.List; -import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.*; public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableConsoleView, DataProvider, OccurenceNavigator { @NonNls private static final String CONSOLE_VIEW_POPUP_MENU = "ConsoleView.PopupMenu"; @@ -132,10 +127,6 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo /** the text from {@link #print(String, ConsoleViewContentType)} goes there and stays there until {@link #flushDeferredText()} is called */ private final TokenBuffer myDeferredBuffer = new TokenBuffer(ConsoleBuffer.useCycleBuffer() ? ConsoleBuffer.getCycleBufferSize() : Integer.MAX_VALUE); - // Range markers created for tokens returned from myDeferredBuffer. - // Each range marker contains ConsoleViewContentType in its user data CONTENT_TYPE - // Accessed in EDT only - private final Queue tokenRangeMarkers = new Queue<>(1000); // strong referenced range markers which store ConsoleViewContentType in their user data private boolean myUpdateFoldingsEnabled = true; private EditorHyperlinkSupport myHyperlinks; @@ -518,12 +509,34 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo @TestOnly void waitAllRequests() { - myFlushAlarm.flush(); - myFlushUserInputAlarm.flush(); - myFlushAlarm.flush(); - myFlushUserInputAlarm.flush(); + ApplicationManager.getApplication().assertIsDispatchThread(); + Future future = ApplicationManager.getApplication().executeOnPooledThread(() -> { + try { + myFlushAlarm.waitForAllExecuted(10, TimeUnit.SECONDS); + myFlushUserInputAlarm.waitForAllExecuted(10, TimeUnit.SECONDS); + myFlushAlarm.waitForAllExecuted(10, TimeUnit.SECONDS); + myFlushUserInputAlarm.waitForAllExecuted(10, TimeUnit.SECONDS); + } + catch (InterruptedException | ExecutionException | TimeoutException e) { + throw new RuntimeException(e); + } + }); + try { + while (true) { + try { + future.get(10, TimeUnit.MILLISECONDS); + break; + } + catch (TimeoutException ignored) { + } + UIUtil.dispatchAllInvocationEvents(); + } + } + catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } } - + protected void disposeEditor() { UIUtil.invokeAndWaitIfNeeded((Runnable)() -> { if (!myEditor.isDisposed()) { @@ -567,7 +580,7 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo if (contentType == ConsoleViewContentType.USER_INPUT) { requestFlushImmediately(); } - if (myEditor != null) { + else if (myEditor != null) { final boolean shouldFlushNow = myDeferredBuffer.length() >= myDeferredBuffer.getCycleBufferSize(); addFlushRequest(FLUSH, shouldFlushNow ? 0 : DEFAULT_FLUSH_DELAY); } @@ -575,6 +588,7 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo } private void sendUserInput(@NotNull CharSequence typedText) { + ApplicationManager.getApplication().assertIsDispatchThread(); if (myState.isRunning() && NEW_LINE_MATCHER.indexIn(typedText) >= 0) { StringBuilder textToSend = new StringBuilder(); // compute text input from the console contents: @@ -689,21 +703,9 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo if (info != null) { myHyperlinks.createHyperlink(start, offset, null, info); } - createTokenRangeMarker(document, token.contentType, start, offset); + createTokenRangeHighlighter(token.contentType, start, offset); offset = start; } - // remove invalid markers from tokenRangeMarkers - while (true) { - RangeMarker marker = tokenRangeMarkers.peekFirst(); - if (marker == null) break; - if (!marker.isValid() || marker.getStartOffset() == marker.getEndOffset()) { - marker.dispose(); - tokenRangeMarkers.pullFirst(); - } - else { - break; - } - } } finally { if (!shouldStickToEnd) { @@ -728,14 +730,15 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo sendUserInput(addedText); } - private void createTokenRangeMarker(@NotNull Document document, - @NotNull ConsoleViewContentType contentType, - int startOffset, - int endOffset) { + private void createTokenRangeHighlighter(@NotNull ConsoleViewContentType contentType, + int startOffset, + int endOffset) { ApplicationManager.getApplication().assertIsDispatchThread(); - RangeMarker tokenMarker = document.createRangeMarker(startOffset, endOffset); + TextAttributes attributes = contentType.getAttributes(); + MarkupModel model = DocumentMarkupModel.forDocument(myEditor.getDocument(), getProject(), true); + RangeHighlighter tokenMarker = model.addRangeHighlighter(startOffset, endOffset, HighlighterLayer.CONSOLE_FILTER, + attributes, HighlighterTargetArea.EXACT_RANGE); tokenMarker.putUserData(CONTENT_TYPE, contentType); - tokenRangeMarkers.addLast(tokenMarker); } boolean isDisposed() { @@ -853,7 +856,6 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo editor.putUserData(CONSOLE_VIEW_IN_EDITOR_VIEW, this); editor.getSettings().setAllowSingleLogicalLineFolding(true); // We want to fold long soft-wrapped command lines - editor.setHighlighter(createHighlighter()); return editor; }); @@ -864,11 +866,6 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo return ConsoleViewUtil.setupConsoleEditor(myProject, true, false); } - @NotNull - private TokenHighlighter createHighlighter() { - return new TokenHighlighter(); - } - private void registerConsoleEditorActions() { Shortcut[] shortcuts = KeymapManager.getInstance().getActiveKeymap().getShortcuts(IdeActions.ACTION_GOTO_DECLARATION); CustomShortcutSet shortcutSet = new CustomShortcutSet(ArrayUtil.mergeArrays(shortcuts, CommonShortcuts.ENTER.getShortcuts())); @@ -1113,9 +1110,9 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo } private RangeMarker findTokenMarker(int offset) { - DocumentEx document = myEditor.getDocument(); RangeMarker[] marker = new RangeMarker[1]; - document.processRangeMarkersOverlappingWith(offset, offset, m->{ + MarkupModelEx model = (MarkupModelEx)DocumentMarkupModel.forDocument(myEditor.getDocument(), getProject(), true); + model.processRangeHighlightersOverlappingWith(offset, offset, m->{ if (getTokenType(m) == null) return true; marker[0] = m; return false; @@ -1128,81 +1125,6 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo return m == null ? null : m.getUserData(CONTENT_TYPE); } - private class TokenHighlighter extends DocumentAdapter implements EditorHighlighter { - private HighlighterClient myEditor; - - @NotNull - @Override - public HighlighterIterator createIterator(final int startOffset) { - // iterate all range markers searching for ones with CONTENT_TYPE user data - RangeMarker marker = findTokenMarker(startOffset); - - return new HighlighterIterator() { - private RangeMarker myCursor = marker; - - @Override - public TextAttributes getTextAttributes() { - return atEnd() ? null : ConsoleViewImpl.getTokenType(myCursor).getAttributes(); - } - - @Override - public int getStart() { - return atEnd() ? 0 : myCursor.getStartOffset(); - } - - @Override - public int getEnd() { - return atEnd() ? 0 : myCursor.getEndOffset(); - } - - @Override - public IElementType getTokenType() { - return null; - } - - @Override - public void advance() { - while (true) { - myCursor = ((RangeMarkerImpl)myCursor).findRangeMarkerAfter(); - if (myCursor == null || ConsoleViewImpl.getTokenType(myCursor) != null) break; - } - } - - @Override - public void retreat() { - while (true) { - myCursor = ((RangeMarkerImpl)myCursor).findRangeMarkerBefore(); - if (myCursor == null || ConsoleViewImpl.getTokenType(myCursor) != null) break; - } - } - - @Override - public boolean atEnd() { - return myCursor == null; - } - - @Override - public Document getDocument() { - return myEditor.getDocument(); - } - }; - } - - @Override - public void setText(@NotNull final CharSequence text) { - } - - @Override - public void setEditor(@NotNull final HighlighterClient editor) { - LOG.assertTrue(myEditor == null, "Highlighters cannot be reused with different editors"); - myEditor = editor; - } - - @Override - public void setColorScheme(@NotNull EditorColorsScheme scheme) { - } - } - private static class MyTypedHandler extends TypedActionHandlerBase { private MyTypedHandler(final TypedActionHandler originalAction) { super(originalAction); @@ -1507,7 +1429,7 @@ public class ConsoleViewImpl extends JPanel implements ConsoleView, ObservableCo int newEndOffset = document.getTextLength() - oldDocLength + offset; // take care of trim document if (findTokenMarker(newEndOffset) == null) { - createTokenRangeMarker(document, ConsoleViewContentType.USER_INPUT, newStartOffset, newEndOffset); + createTokenRangeHighlighter(ConsoleViewContentType.USER_INPUT, newStartOffset, newEndOffset); } moveScrollRemoveSelection(editor, newEndOffset); diff --git a/platform/platform-tests/testSrc/com/intellij/execution/impl/ConsoleViewImplTest.java b/platform/platform-tests/testSrc/com/intellij/execution/impl/ConsoleViewImplTest.java index f38c741bae28..9cb34506861c 100644 --- a/platform/platform-tests/testSrc/com/intellij/execution/impl/ConsoleViewImplTest.java +++ b/platform/platform-tests/testSrc/com/intellij/execution/impl/ConsoleViewImplTest.java @@ -30,7 +30,6 @@ import com.intellij.openapi.editor.actionSystem.TypedAction; import com.intellij.openapi.editor.ex.EditorEx; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Disposer; -import com.intellij.openapi.util.io.BufferExposingByteArrayOutputStream; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.testFramework.LightPlatformCodeInsightTestCase; import com.intellij.testFramework.LightPlatformTestCase; @@ -42,6 +41,7 @@ import com.intellij.util.TimeoutUtil; import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; +import java.io.ByteArrayOutputStream; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -224,6 +224,7 @@ public class ConsoleViewImplTest extends LightPlatformTestCase { UIUtil.dispatchAllInvocationEvents(); } LightPlatformCodeInsightTestCase.type('\n', console.getEditor(), getProject()); + console.waitAllRequests(); }).cpuBound().assertTiming()); } @@ -269,7 +270,7 @@ public class ConsoleViewImplTest extends LightPlatformTestCase { public void testUserInputIsSentToProcessAfterNewLinePressed() { Process testProcess = AnsiEscapeDecoderTest.createTestProcess(); - BufferExposingByteArrayOutputStream outputStream = (BufferExposingByteArrayOutputStream)testProcess.getOutputStream(); + ByteArrayOutputStream outputStream = (ByteArrayOutputStream)testProcess.getOutputStream(); AnsiEscapeDecoderTest.withProcessHandlerFrom(testProcess, handler -> withCycleConsole(100, console -> { @@ -283,14 +284,13 @@ public class ConsoleViewImplTest extends LightPlatformTestCase { assertEquals(0, outputStream.size()); console.print("\n", ConsoleViewContentType.USER_INPUT); console.waitAllRequests(); - assertEquals(3, outputStream.size()); assertEquals("IK\n", outputStream.toString()); })); } public void testUserTypingIsSentToProcessAfterNewLinePressed() { Process testProcess = AnsiEscapeDecoderTest.createTestProcess(); - BufferExposingByteArrayOutputStream outputStream = (BufferExposingByteArrayOutputStream)testProcess.getOutputStream(); + ByteArrayOutputStream outputStream = (ByteArrayOutputStream)testProcess.getOutputStream(); AnsiEscapeDecoderTest.withProcessHandlerFrom(testProcess, handler -> withCycleConsole(100, console -> { diff --git a/platform/platform-tests/testSrc/com/intellij/execution/process/AnsiEscapeDecoderTest.java b/platform/platform-tests/testSrc/com/intellij/execution/process/AnsiEscapeDecoderTest.java index f8ee56345ae0..9a1a90030577 100644 --- a/platform/platform-tests/testSrc/com/intellij/execution/process/AnsiEscapeDecoderTest.java +++ b/platform/platform-tests/testSrc/com/intellij/execution/process/AnsiEscapeDecoderTest.java @@ -3,7 +3,6 @@ package com.intellij.execution.process; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.io.BufferExposingByteArrayInputStream; -import com.intellij.openapi.util.io.BufferExposingByteArrayOutputStream; import com.intellij.testFramework.PlatformTestCase; import com.intellij.testFramework.PlatformTestUtil; import com.intellij.util.Consumer; @@ -11,6 +10,7 @@ import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.junit.Assert; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -102,9 +102,9 @@ public class AnsiEscapeDecoderTest extends PlatformTestCase { @NotNull public static Process createTestProcess() { - byte[] buffer = new byte[1000]; - BufferExposingByteArrayOutputStream outputStream = new BufferExposingByteArrayOutputStream(buffer); - BufferExposingByteArrayInputStream inputStream = new BufferExposingByteArrayInputStream(buffer); + // have to be synchronised because used from pooled thread + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(10000); + BufferExposingByteArrayInputStream inputStream = new BufferExposingByteArrayInputStream(new byte[0]); AtomicBoolean finished = new AtomicBoolean(); return new Process() { @Override @@ -156,6 +156,7 @@ public class AnsiEscapeDecoderTest extends PlatformTestCase { public static void withProcessHandlerFrom(@NotNull Process testProcess, @NotNull Consumer actionToTest) { KillableColoredProcessHandler handler = new KillableColoredProcessHandler(testProcess, "testProcess"); handler.setShouldDestroyProcessRecursively(false); + handler.setShouldKillProcessSoftly(false); handler.startNotify(); handler.notifyTextAvailable("Running stuff...\n", ProcessOutputTypes.STDOUT); @@ -163,10 +164,8 @@ public class AnsiEscapeDecoderTest extends PlatformTestCase { actionToTest.consume(handler); } finally { - handler.doDestroyProcess(); - handler.notifyProcessTerminated(0); + handler.destroyProcess(); handler.waitFor(); } - } }