Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
Change-Id: I7c6eec0871e7f2b3a2a71c401b19e52a687f126e
Signed-off-by: Stephan Wahlbrink <sw@wahlbrink.eu>
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
index 8d648d9..af6d754 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContentAssistSubjectControlAdapter.java
@@ -8,6 +8,7 @@
* Contributors:
* IBM Corporation - initial API and implementation
* Mickael Istria (Red Hat Inc.) - [251156] Allow multiple contentAssitProviders internally & inheritance
+ * Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
*******************************************************************************/
package org.eclipse.jface.text.contentassist;
@@ -120,6 +121,14 @@
return fViewer.getTextWidget().getLineDelimiter();
}
+ public boolean isValidWidgetOffset(int widgetOffset) {
+ if (fContentAssistSubjectControl != null) {
+ IDocument document= fContentAssistSubjectControl.getDocument();
+ return (widgetOffset >= 0 && widgetOffset <= document.getLength());
+ }
+ return (widgetOffset >= 0 && widgetOffset <= fViewer.getTextWidget().getCharCount());
+ }
+
@Override
public void addKeyListener(KeyListener keyListener) {
if (fContentAssistSubjectControl != null)
diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
index f733d6d..8315265 100644
--- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
+++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/ContextInformationPopup.java
@@ -7,6 +7,7 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
+ * Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
*******************************************************************************/
package org.eclipse.jface.text.contentassist;
@@ -274,7 +275,7 @@
* @since 3.0
*/
private void internalShowContextInfo(ContextFrame frame) {
- if (frame != null) {
+ if (frame != null && canShowFrame(frame)) {
fContextFrameStack.push(frame);
if (fContextFrameStack.size() == 1)
fLastContext= null;
@@ -336,6 +337,19 @@
}
/**
+ * Pre-checks if the given context frame can be (re)shown.
+ *
+ * The function checks if the frame has valid position data. It does not call the context
+ * information validator.
+ *
+ * @param frame the frame to check
+ * @return <code>true</code> if the context frame OK to use, otherwise <code>false</code>
+ */
+ private boolean canShowFrame(ContextFrame frame) {
+ return fContentAssistSubjectControlAdapter.isValidWidgetOffset(frame.fVisibleOffset);
+ }
+
+ /**
* Shows the given context frame.
*
* @param frame the frame to display
@@ -457,16 +471,23 @@
if (Helper.okToUse(fContextInfoPopup)) {
int size= fContextFrameStack.size();
- if (size > 0) {
+ while (size > 0) {
fLastContext= fContextFrameStack.pop();
-- size;
+
+ if (size > 0) {
+ ContextFrame current= fContextFrameStack.peek();
+ if (canShowFrame(current)) {
+ internalShowContextFrame(current, false);
+ return;
+ }
+ // continue - try next
+ }
+ else {
+ break;
+ }
}
-
- if (size > 0) {
- ContextFrame current= fContextFrameStack.peek();
- internalShowContextFrame(current, false);
- } else {
-
+ {
fContentAssistant.removeContentAssistListener(this, ContentAssistant.CONTEXT_INFO_POPUP);
if (fContentAssistSubjectControlAdapter.getControl() != null)
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
index 2d31f0a..ef1e3b0 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/AbstratGenericEditorTest.java
@@ -23,11 +23,17 @@
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.text.tests.Accessor;
+
+import org.eclipse.jface.text.source.SourceViewer;
+
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.internal.genericeditor.ExtensionBasedTextEditor;
import org.eclipse.ui.intro.IIntroPart;
import org.eclipse.ui.part.FileEditorInput;
+import org.eclipse.ui.texteditor.AbstractTextEditor;
+
/**
* Closes intro, create {@link #project}, create {@link #file} and open {@link #editor}; and clean up.
* Also contains additional utility methods
@@ -81,7 +87,12 @@
file = null;
}
}
-
+
+ protected SourceViewer getSourceViewer() {
+ SourceViewer sourceViewer= (SourceViewer) new Accessor(editor, AbstractTextEditor.class).invoke("getSourceViewer", new Object[0]);
+ return sourceViewer;
+ }
+
@After
public void tearDown() throws Exception {
cleanFileAndEditor();
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java
new file mode 100644
index 0000000..e92016c
--- /dev/null
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/ContextInfoTest.java
@@ -0,0 +1,126 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Stephan Wahlbrink and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Stephan Wahlbrink <sw@wahlbrink.eu>
+ *******************************************************************************/
+package org.eclipse.ui.genericeditor.tests;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.junit.After;
+import org.junit.Test;
+
+import org.eclipse.swt.custom.StyledText;
+import org.eclipse.swt.widgets.Control;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Text;
+
+import org.eclipse.text.tests.Accessor;
+
+import org.eclipse.jface.text.contentassist.ContentAssistant;
+import org.eclipse.jface.text.source.SourceViewer;
+
+import org.eclipse.ui.genericeditor.tests.contributions.BarContentAssistProcessor;
+
+import org.eclipse.ui.texteditor.ITextEditorActionConstants;
+import org.eclipse.ui.texteditor.TextOperationAction;
+
+
+public class ContextInfoTest extends AbstratGenericEditorTest {
+
+
+ private Shell completionShell;
+
+
+ @Test
+ public void testContextInfo() throws Exception {
+ cleanFileAndEditor();
+ createAndOpenFile("bar.txt", BarContentAssistProcessor.PROPOSAL);
+
+ final Set<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+ TextOperationAction action = (TextOperationAction) editor.getAction(ITextEditorActionConstants.CONTENT_ASSIST_CONTEXT_INFORMATION);
+
+ editor.selectAndReveal(4, 0);
+ action.update();
+ action.run();
+ waitAndDispatch(100);
+ this.completionShell= findNewShell(beforeShells);
+ assertEquals("idx= 0", getInfoText(this.completionShell));
+
+ editor.selectAndReveal(8, 0);
+ action.update();
+ action.run();
+ waitAndDispatch(100);
+ this.completionShell= findNewShell(beforeShells);
+ assertEquals("idx= 1", getInfoText(this.completionShell));
+ }
+
+ @Test
+ public void testContextInfo_hide_Bug512251() throws Exception {
+ cleanFileAndEditor();
+ createAndOpenFile("bar.txt", BarContentAssistProcessor.PROPOSAL);
+
+ final Set<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+ TextOperationAction action = (TextOperationAction) editor.getAction(ITextEditorActionConstants.CONTENT_ASSIST_CONTEXT_INFORMATION);
+
+ editor.selectAndReveal(4, 0);
+ action.update();
+ action.run();
+ waitAndDispatch(100);
+ this.completionShell= findNewShell(beforeShells);
+
+ editor.selectAndReveal(8, 0);
+ action.update();
+ action.run();
+ waitAndDispatch(100);
+ this.completionShell= findNewShell(beforeShells);
+
+ editor.getAction(ITextEditorActionConstants.DELETE_LINE).run();
+
+ SourceViewer sourceViewer= getSourceViewer();
+ ContentAssistant assist= (ContentAssistant) new Accessor(sourceViewer, SourceViewer.class).get("fContentAssistant");
+ new Accessor(assist, ContentAssistant.class).invoke("hide", new Object[0]);
+ }
+
+
+ private Shell findNewShell(Set<Shell> beforeShells) {
+ Shell[] afterShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells())
+ .filter(Shell::isVisible)
+ .filter(shell -> !beforeShells.contains(shell))
+ .toArray(Shell[]::new);
+ assertEquals("No new shell found", 1, afterShells.length);
+ return afterShells[0];
+ }
+
+ private String getInfoText(final Shell shell) {
+ assertTrue(shell.isVisible());
+ Control[] children= shell.getChildren();
+ for (Control child : children) {
+ if (child instanceof Text) {
+ return ((Text) child).getText();
+ }
+ if (child instanceof StyledText) {
+ return ((StyledText) child).getText();
+ }
+ }
+ return null;
+ }
+
+ @After
+ public void closeShell() {
+ if (this.completionShell != null && !completionShell.isDisposed()) {
+ completionShell.close();
+ }
+ }
+
+}
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
index bf987dc..76c9c1c 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/GenericEditorTestSuite.java
@@ -17,6 +17,7 @@
@RunWith(Suite.class)
@SuiteClasses({
CompletionTest.class,
+ ContextInfoTest.class,
StylingTest.class,
HoverTest.class,
EditorTest.class,
diff --git a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
index f53da97..9c22f88 100644
--- a/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
+++ b/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/BarContentAssistProcessor.java
@@ -7,12 +7,18 @@
*
* Contributors:
* - Mickael Istria (Red Hat Inc.)
+ * Stephan Wahlbrink <sw@wahlbrink.eu> - Bug 512251 - Fix IllegalArgumentException in ContextInformationPopup
*******************************************************************************/
package org.eclipse.ui.genericeditor.tests.contributions;
+import java.util.Arrays;
+
import org.eclipse.jface.text.BadLocationException;
+import org.eclipse.jface.text.IDocument;
+import org.eclipse.jface.text.IRegion;
import org.eclipse.jface.text.ITextViewer;
import org.eclipse.jface.text.contentassist.CompletionProposal;
+import org.eclipse.jface.text.contentassist.ContextInformation;
import org.eclipse.jface.text.contentassist.ICompletionProposal;
import org.eclipse.jface.text.contentassist.IContentAssistProcessor;
import org.eclipse.jface.text.contentassist.IContextInformation;
@@ -49,11 +55,6 @@
}
@Override
- public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
- return null;
- }
-
- @Override
public char[] getCompletionProposalAutoActivationCharacters() {
return null;
}
@@ -63,13 +64,58 @@
return null;
}
+ /**
+ * Creates context info "idx= <word index in #PROPOSAL>" at the end of a word.
+ **/
@Override
- public String getErrorMessage() {
+ public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
+ try {
+ IDocument document= viewer.getDocument();
+ int begin= offset;
+ while (begin > 0 && Character.isLetterOrDigit(document.getChar(begin - 1))) {
+ begin--;
+ }
+ if (begin < offset) {
+ String word= document.get(begin, offset - begin);
+ int idx= Arrays.asList(completeString.split("\\W")).indexOf(word);
+ if (idx >= 0) {
+ return new IContextInformation[] {
+ new ContextInformation(word, "idx= " + idx)
+ };
+ }
+ }
+ } catch (BadLocationException e) {
+ e.printStackTrace();
+ }
return null;
}
@Override
public IContextInformationValidator getContextInformationValidator() {
+ return new IContextInformationValidator() {
+ ITextViewer viewer;
+ int offset;
+ @Override
+ public void install(IContextInformation info, ITextViewer viewer, int offset) {
+ this.viewer= viewer;
+ this.offset= offset;
+ }
+ @Override
+ public boolean isContextInformationValid(int offset) {
+ try {
+ IDocument document= viewer.getDocument();
+ IRegion line= document.getLineInformationOfOffset(this.offset);
+ int end= line.getOffset() + line.getLength();
+ return (offset >= this.offset && offset < end);
+ } catch (BadLocationException e) {
+ return false;
+ }
+ }
+ };
+ }
+
+ @Override
+ public String getErrorMessage() {
return null;
}
