Simplify default implementation of ICompletionProposal (XTextICompletionProposal) and fix some bugs

4 files changed, 93 insertions, 200 deletions

diff --git a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/AbstractProposalProvider.java b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/AbstractProposalProvider.java
index 9a065e8..63dbd56 100644
--- a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/AbstractProposalProvider.java
+++ b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/AbstractProposalProvider.java
@@ -69,8 +69,7 @@ public abstract class AbstractProposalProvider implements IProposalProvider {
 			logger.debug("completeKeyword '" + keyword.getValue() + "' for model '" + contentAssistContext.getModel() + "' and prefix '"
 					+ contentAssistContext.getMatchString().trim() + "'");
 		}
-		String text = keyword.getValue().length() == 1 ? keyword.getValue() : keyword.getValue() + " ";
-		return Collections.singletonList(createCompletionProposal(keyword, text, contentAssistContext));
+		return Collections.singletonList(createCompletionProposal(keyword, keyword.getValue(), contentAssistContext));
 	}
 
 	/*
diff --git a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistCalculator.java b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistCalculator.java
index e8e83a3..faa1d09 100644
--- a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistCalculator.java
+++ b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistCalculator.java
@@ -25,6 +25,7 @@ import org.eclipse.xtext.CrossReference;
 import org.eclipse.xtext.EcoreUtil2;
 import org.eclipse.xtext.GrammarUtil;
 import org.eclipse.xtext.Group;
+import org.eclipse.xtext.Keyword;
 import org.eclipse.xtext.ParserRule;
 import org.eclipse.xtext.RuleCall;
 import org.eclipse.xtext.crossref.ILinkingService;
@@ -95,7 +96,7 @@ public class DefaultContentAssistCalculator extends XtextSwitch<List<AbstractEle
 		}
 
 		for (Iterator<AbstractElement> iterator = nextValidElementSet.iterator(); iterator.hasNext();) {
-			AbstractElement abstractElement = (AbstractElement) iterator.next();
+			AbstractElement abstractElement = iterator.next();
 			computedElementList.addAll(doSwitch(abstractElement));
 		}
 		
@@ -130,10 +131,11 @@ public class DefaultContentAssistCalculator extends XtextSwitch<List<AbstractEle
 		List<AbstractElement> elementList = new ArrayList<AbstractElement>();
 		if (assignment.getTerminal() instanceof RuleCall) {
 			addWithNullCheck(elementList, doSwitch(assignment.getTerminal()));
-		}
-		else if (assignment.getTerminal() instanceof Alternatives) {
-			addWithNullCheck(elementList, doSwitch(assignment.getTerminal()));
-		}
+		} else if (assignment.getTerminal() instanceof Alternatives) {
+            addWithNullCheck(elementList, doSwitch(assignment.getTerminal()));
+        } else if (assignment.getTerminal() instanceof Keyword) {
+            addWithNullCheck(elementList, doSwitch(assignment.getTerminal()));
+        }
 		elementList.add(assignment);
 		return elementList;
 	}
diff --git a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/XtextCompletionProposal.java b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/XtextCompletionProposal.java
index 0e87712..b72b2ba 100644
--- a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/XtextCompletionProposal.java
+++ b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/XtextCompletionProposal.java
@@ -2,28 +2,18 @@ package org.eclipse.xtext.ui.common.editor.contentassist.impl;
 
 import org.apache.log4j.Logger;
 import org.eclipse.emf.ecore.EObject;
-import org.eclipse.jface.resource.JFaceResources;
 import org.eclipse.jface.text.BadLocationException;
-import org.eclipse.jface.text.DocumentEvent;
 import org.eclipse.jface.text.IDocument;
-import org.eclipse.jface.text.ITextViewer;
-import org.eclipse.jface.text.contentassist.ContextInformation;
 import org.eclipse.jface.text.contentassist.ICompletionProposal;
-import org.eclipse.jface.text.contentassist.ICompletionProposalExtension2;
-import org.eclipse.jface.text.contentassist.ICompletionProposalExtension6;
 import org.eclipse.jface.text.contentassist.IContextInformation;
-import org.eclipse.jface.viewers.StyledString;
 import org.eclipse.swt.graphics.Image;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.xtext.AbstractElement;
-import org.eclipse.xtext.Assignment;
 import org.eclipse.xtext.CrossReference;
 import org.eclipse.xtext.GrammarUtil;
 import org.eclipse.xtext.Keyword;
 import org.eclipse.xtext.ParserRule;
-import org.eclipse.xtext.RuleCall;
 import org.eclipse.xtext.parsetree.AbstractNode;
-import org.eclipse.xtext.parsetree.LeafNode;
 import org.eclipse.xtext.ui.common.editor.contentassist.IContentAssistContext;
 
 /**
@@ -32,165 +22,86 @@ import org.eclipse.xtext.ui.common.editor.contentassist.IContentAssistContext;
  * @author Dennis Hübner - Initial contribution and API
  * @author Michael Clay
  * @see org.eclipse.jface.text.contentassist.ICompletionProposal
- * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension2
- * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension6
  */
-public class XtextCompletionProposal implements ICompletionProposal,
-		ICompletionProposalExtension2, ICompletionProposalExtension6 {
-	
+public class XtextCompletionProposal implements ICompletionProposal {
+
+	// logger available to subclasses
 	protected final Logger logger = Logger.getLogger(XtextCompletionProposal.class);
 
-	
-	private AbstractElement abstractElement;
-	private String text;
-	private String description;
-	private Image image;
-	private final StyledString label;
-	private String pluginIdentifier;
-	private IContentAssistContext contentAssistContext;
+	private final AbstractElement abstractElement;
+	private final String text;
+	private final IContentAssistContext contentAssistContext;
+	private int selectionOffset;
 
-	
 	public XtextCompletionProposal(AbstractElement abstractElement, String displayString,
 			IContentAssistContext contentAssistContext) {
 		this.abstractElement = abstractElement;
 		this.text = displayString;
-		this.description = displayString;
-		this.label = new StyledString(displayString);
 		this.contentAssistContext = contentAssistContext;
 	}
 
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#apply(org.eclipse.jface.text.IDocument)
 	 */
 	public void apply(IDocument document) {
 
-	}
+		try {
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getAdditionalProposalInfo()
-	 */
-	public String getAdditionalProposalInfo() {
-		return this.description == null ? null : this.description
-				+ " Additional";
-	}
+			AbstractNode abstractNode = this.contentAssistContext.getNode();
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getContextInformation()
-	 */
-	public IContextInformation getContextInformation() {
-		if (this.description != null)
-			return new ContextInformation(getImage(), this.text,
-					this.description);
-		return null;
-	}
+			int replacementOffset = this.contentAssistContext.getOffSet();
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getDisplayString()
-	 */
-	public String getDisplayString() {
-		return this.label.getString();
-	}
+			if (!"".equals(this.contentAssistContext.getMatchString())
+					&& getDisplayString().toUpperCase().startsWith(
+							this.contentAssistContext.getMatchString().toUpperCase())) {
 
-	/*
-	 * (non-Javadoc)
-	 * 
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getImage()
-	 */
-	public Image getImage() {
-		return this.image;
-	}
+				document.replace(abstractNode.getTotalOffset(), abstractNode.getTotalLength(), "");
 
-	/*
-	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension2#apply(org.eclipse.jface.text.ITextViewer, char, int, int)
-	 */
-	public void apply(ITextViewer viewer, char trigger, int stateMask,
-			int offset) {
+				replacementOffset = abstractNode.getTotalOffset();
 
-		try {
-			
-			IDocument document = viewer.getDocument();
-			
-			if (this.contentAssistContext.getModel() != null) {
-				
-				
-				AbstractNode abstractNode = this.contentAssistContext.getNode();
-				
-				if (abstractNode instanceof LeafNode) {
-					
-					LeafNode currentLeafNode = (LeafNode) abstractNode;
-					
-					if (getDisplayString().toUpperCase().startsWith(currentLeafNode.getText().toUpperCase())) {
-						this.text = this.text.substring(this.contentAssistContext.getOffSet() - currentLeafNode.getTotalOffset());
-					} else if (isCusorAtEndOfLastCompleteNode()) {
-
-						if (currentLeafNode.getGrammarElement() instanceof CrossReference
-								&& abstractElement instanceof CrossReference) {
-							this.text =  " " + this.text;
-						}
-						else if (currentLeafNode.getGrammarElement() instanceof RuleCall
-								&& currentLeafNode.getGrammarElement().eContainer() instanceof Assignment
-								&& abstractElement instanceof Assignment) {
-							this.text = " " + this.text;
-						}
-						else if (!GrammarUtil.containingParserRule(abstractElement).equals(
-								  GrammarUtil.containingParserRule(currentLeafNode.getGrammarElement()))) {
-							this.text = " " + this.text;
-						}
-					}
-				} 
 			}
-			
-			document.replace(this.contentAssistContext.getOffSet(), 0, this.text);
 
-		} catch (BadLocationException e) {
+			document.replace(replacementOffset, 0, this.text.trim());
+
+			this.selectionOffset = replacementOffset + this.text.trim().length();
+
+		}
+		catch (BadLocationException e) {
 			logger.error(e);
 		}
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension2#selected(org.eclipse.jface.text.ITextViewer, boolean)
+	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getAdditionalProposalInfo()
 	 */
-	public void selected(ITextViewer viewer, boolean smartToggle) {
+	public String getAdditionalProposalInfo() {
+		return null;
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension2#unselected(org.eclipse.jface.text.ITextViewer)
+	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getContextInformation()
 	 */
-	public void unselected(ITextViewer viewer) {
+	public IContextInformation getContextInformation() {
+		return null;
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.eclipse.jface.text.contentassist.ICompletionProposalExtension2#validate(org.eclipse.jface.text.IDocument, int, org.eclipse.jface.text.DocumentEvent)
+	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getDisplayString()
 	 */
-	public boolean validate(IDocument document, int offset, DocumentEvent event) {
-		boolean startsWith = false;
-		try {
-			String prefix = document.get(this.contentAssistContext.getOffSet(), offset - this.contentAssistContext.getOffSet());
-			startsWith = getDisplayString().toLowerCase().startsWith(
-					prefix.toLowerCase());
-		} catch (BadLocationException e) {
-			logger.error(e);
-		}
-		return startsWith;
+	public String getDisplayString() {
+		return this.text;
 	}
 
 	/*
 	 * (non-Javadoc)
-	 * 
-	 * @seeorg.eclipse.jface.text.contentassist.ICompletionProposalExtension6#
-	 * getStyledDisplayString()
+	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getImage()
 	 */
-	public StyledString getStyledDisplayString() {
-		return this.label;
+	public Image getImage() {
+		return null;
 	}
 
 	/*
@@ -198,77 +109,57 @@ public class XtextCompletionProposal implements ICompletionProposal,
 	 * @see org.eclipse.jface.text.contentassist.ICompletionProposal#getSelection(org.eclipse.jface.text.IDocument)
 	 */
 	public Point getSelection(IDocument document) {
-		return new Point(this.contentAssistContext.getOffSet() + this.text.length(), 0);
+		return new Point(this.selectionOffset, 0);
 	}
-	
+
 	/**
 	 * 
 	 * @return true or false whether the given prefix matches the text of this completion proposal
 	 */
 	public boolean matches() {
-		
+
 		boolean matches = true;
-		
-		if (this.contentAssistContext.getModel() != null) {
-			
-			AbstractElement abstractElement = null;
-
-			if (this.abstractElement instanceof Keyword ||
-					this.abstractElement instanceof CrossReference) {
-				abstractElement = GrammarUtil.containingAssignment(this.abstractElement);
-			} 
-			
-			if (null==abstractElement) {
-				abstractElement = this.abstractElement;
-			}
-			
-			
-			boolean candidateToCompare 	= false;
-			
-			// means if we are at the end of a complete token we want to filter only equal grammarelements (not the 'next' ones)
-			if (isCusorAtEndOfLastCompleteNode() && 
-					abstractElement.equals(getCurrentGrammarElement())) {
-				candidateToCompare = true;
-			} else if (!isCusorAtEndOfLastCompleteNode() ) {
-				candidateToCompare = true;
-			}
-			
-			if ( candidateToCompare && (!"".equals(this.contentAssistContext.getMatchString().trim()) && 
-					!getDisplayString().toUpperCase().trim().startsWith(this.contentAssistContext.getMatchString().toUpperCase().trim()))) {
-				matches = false;
-			}	
+
+		AbstractElement abstractElement = null;
+
+		if (this.abstractElement instanceof Keyword || this.abstractElement instanceof CrossReference) {
+			abstractElement = GrammarUtil.containingAssignment(this.abstractElement);
+		}
+
+		if (null == abstractElement) {
+			abstractElement = this.abstractElement;
 		}
-		
-		
+
+		boolean candidateToCompare = false;
+
+		// means if we are at the end of a complete token we want to filter only
+		// equal grammarelements (not the 'next' ones)
+		if (isCusorAtEndOfLastCompleteNode() && abstractElement.equals(getCurrentGrammarElement())) {
+			candidateToCompare = true;
+		}
+		else if (!isCusorAtEndOfLastCompleteNode()) {
+			candidateToCompare = true;
+		}
+
+		if (candidateToCompare
+				&& (!"".equals(this.contentAssistContext.getMatchString().trim()) && !getDisplayString().toUpperCase()
+						.trim().startsWith(this.contentAssistContext.getMatchString().toUpperCase().trim()))) {
+			matches = false;
+		}
+
 		return matches;
 	}
 
 	@Override
 	public String toString() {
-		return "XtextCompletionPoposal[text='"+this.text+"']";
+		return "XtextCompletionPoposal[text='" + this.text + "']";
 	}
 
-	private void initializeImage(String imageName) {
-		Image newImage = JFaceResources.getImage(imageName);
-		//TODO: needs plugin to resolve image path
-//		if (newImage == null) {
-//			ImageDescriptor imageDescriptorFromPlugin = Activator
-//					.imageDescriptorFromPlugin(this.pluginIdentifier, imageName);
-//			if (imageDescriptorFromPlugin != null) {
-//				JFaceResources.getImageRegistry().put(imageName,
-//						imageDescriptorFromPlugin);
-//				newImage = JFaceResources.getImage(imageName);
-//			}
-//		}
-		this.image = newImage;
-	}
-	
-	
 	/**
 	 * @return true or false wheter the cursor is at the the last complete node
 	 */
 	public boolean isCusorAtEndOfLastCompleteNode() {
-		return  this.contentAssistContext.getNode() == this.contentAssistContext.getReferenceNode();
+		return this.contentAssistContext.getNode() == this.contentAssistContext.getReferenceNode();
 	}
 
 	/**
@@ -276,11 +167,13 @@ public class XtextCompletionProposal implements ICompletionProposal,
 	 */
 	public EObject getCurrentGrammarElement() {
 
-		EObject grammarElement = GrammarUtil.containingAssignment(this.contentAssistContext.getNode().getGrammarElement());
+		EObject grammarElement = GrammarUtil.containingAssignment(this.contentAssistContext.getNode()
+				.getGrammarElement());
 
 		if (null == grammarElement) {
-			grammarElement = (this.contentAssistContext.getNode().getGrammarElement() instanceof ParserRule ? ((ParserRule) this.contentAssistContext.getNode()
-					.getGrammarElement()).getAlternatives() : this.contentAssistContext.getNode().getGrammarElement());
+			grammarElement = (this.contentAssistContext.getNode().getGrammarElement() instanceof ParserRule ? ((ParserRule) this.contentAssistContext
+					.getNode().getGrammarElement()).getAlternatives()
+					: this.contentAssistContext.getNode().getGrammarElement());
 		}
 
 		return grammarElement;
diff --git a/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistProcessorTest.java b/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistProcessorTest.java
index 5545286..0d93d0b 100644
--- a/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistProcessorTest.java
+++ b/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DefaultContentAssistProcessorTest.java
@@ -20,7 +20,6 @@ import org.eclipse.xtext.testlanguages.ContentAssistTestLanguageUiConfig;
 import org.eclipse.xtext.testlanguages.ReferenceGrammarTestLanguageStandaloneSetup;

 import org.eclipse.xtext.testlanguages.ReferenceGrammarTestLanguageUiConfig;

 import org.eclipse.xtext.ui.common.AbstractUiTest;

-import org.eclipse.xtext.ui.common.editor.contentassist.impl.DefaultContentAssistProcessor;

 

 

 /**

@@ -56,11 +55,11 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 	}

 	

 	public void testComputeCompletionProposalsText() throws Exception {

-		contentAssistProcessorTestBuilder.assertText("spielplatz ")

+		contentAssistProcessorTestBuilder.assertText("spielplatz")

 			.applyText().assertText("0","1")

 			.applyText().assertText("\"SpielplatzBeschreibungSTRING\"","\"SpielplatzBeschreibung\"","{")

 			.applyText().assertText("{")

-			.applyText().assertText("erwachsener ","familie ","spielzeug ","kind ","}")

+			.applyText().assertText("erwachsener", "familie", "spielzeug", "kind", "}")

 			.append("erwachsener ").assertText("(")

 			.applyText().assertText("ErwachsenerNameID","ErwachsenerName")

 			.append("e1 ").assertText("0","1")

@@ -70,7 +69,7 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 			.append("k1 ").assertText("0","1")

 			.applyText().assertText(")")

 			.applyText().append("kind (k2 0) familie ").assertText("(")

-			.applyText().assertText("keyword ","\"FamilieNameSTRING\"","FamilieNameID")

+			.applyText().assertText("keyword","\"FamilieNameSTRING\"","FamilieNameID")

 			.append("keyword ").assertText("e1","e2")

 			.applyText().assertText("e1","e2")

 			.append("e2 ").assertText("k1","k2")

@@ -84,13 +83,13 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 	

 	public void testComputeCompletionProposalsIgnoreCase() throws Exception {

 		contentAssistProcessorTestBuilder.set("spielplatz 1 \"SpielplatzBeschreibung\" { kind(k1 0) kind(k2 0) erwachsener(e1 0) erwachsener(e2 0) ")

-			.append(" KI").assertText("kind ").delete(3)

-			.append(" ER").assertText("erwachsener ").delete(3)

-			.append(" SP").assertText("spielzeug ").delete(3)

-			.append(" FA").assertText("familie ").delete(3)

-			.append(" familie ( KEY").assertText("keyword ").delete(13)

+			.append(" KI").assertText("kind").delete(3)

+			.append(" ER").assertText("erwachsener").delete(3)

+			.append(" SP").assertText("spielzeug").delete(3)

+			.append(" FA").assertText("familie").delete(3)

+			.append(" familie ( KEY").assertText("keyword").delete(13)

 			.append(" familie ( \"").assertText("\"FamilieNameSTRING\"").delete(12)

-			.append(" familie ( K").assertText("keyword ").delete(11)

+			.append(" familie ( K").assertText("keyword").delete(11)

 			.append(" familie ( keyword E").assertText("e1","e2").delete(19)

 			.append(" familie ( keyword e1 E").assertText("e1","e2").delete(22)

 			.append(" familie ( keyword e1 e2 K").assertText("k1","k2",",",")").delete(25)

@@ -99,11 +98,6 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 		;

 	}

 	

-	private ContentAssistProcessorTestBuilder newBuilder(Class<?> standAloneSetup, Class<?> uiConfig) throws Exception {

-		withUi(standAloneSetup, uiConfig);

-		return new ContentAssistProcessorTestBuilder(getCurrentServiceScope(), new DefaultContentAssistProcessor());

-	}

-	

 	public void testCompleteRuleCall() throws Exception {

 		newBuilder(XtextGrammarTestLanguageStandaloneSetup.class, XtextGrammarTestLanguageUiConfig.class)

 			.appendNl("language foo")

@@ -118,7 +112,7 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 					"\"KeywordValue\"", 

 					"(", 

 					"[", 

-					"+= " // TODO: Why does this proposal come up?

+					"+=" // TODO: Why does this proposal come up?

 			);

 	}

 	

@@ -133,4 +127,9 @@ public class DefaultContentAssistProcessorTest extends AbstractUiTest
 			);

 	}

 	

+	private ContentAssistProcessorTestBuilder newBuilder(Class<?> standAloneSetup, Class<?> uiConfig) throws Exception {

+		withUi(standAloneSetup, uiConfig);

+		return new ContentAssistProcessorTestBuilder(getCurrentServiceScope(), new DefaultContentAssistProcessor());

+	}

+	

 }
\ No newline at end of file