Fixed: [Issue 174] Content assistance too eager to provide field id.
Fixed: [Issue 177] Typing semicolon too fast does not generate tag
number.
diff --git a/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateIndexOf_Test.java b/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateNewIndexOf_Test.java
similarity index 86%
rename from com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateIndexOf_Test.java
rename to com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateNewIndexOf_Test.java
index 8fcf9ad..2ca6260 100644
--- a/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateIndexOf_Test.java
+++ b/com.google.eclipse.protobuf.ui.test/src/com/google/eclipse/protobuf/ui/util/Literals_calculateNewIndexOf_Test.java
@@ -19,11 +19,11 @@
import com.google.eclipse.protobuf.protobuf.Literal;
/**
- * Tests for <code>{@link Literals#calculateIndexOf(Literal)}</code>.
+ * Tests for <code>{@link Literals#calculateNewIndexOf(Literal)}</code>.
*
* @author alruiz@google.com (Alex Ruiz)
*/
-public class Literals_calculateIndexOf_Test {
+public class Literals_calculateNewIndexOf_Test {
@Rule public XtextRule xtext = overrideRuntimeModuleWith(unitTestModule());
private Literals literals;
@@ -39,7 +39,7 @@
// }
@Test public void should_return_zero_for_first_and_only_literal() {
Literal mobile = xtext.find("MOBILE", Literal.class);
- long index = literals.calculateIndexOf(mobile);
+ long index = literals.calculateNewIndexOf(mobile);
assertThat(index, equalTo(0L));
}
@@ -52,7 +52,7 @@
// }
@Test public void should_return_max_index_value_plus_one_for_new_literal() {
Literal work = xtext.find("WORK", Literal.class);
- long index = literals.calculateIndexOf(work);
+ long index = literals.calculateNewIndexOf(work);
assertThat(index, equalTo(6L));
}
}
diff --git a/com.google.eclipse.protobuf.ui/plugin.xml b/com.google.eclipse.protobuf.ui/plugin.xml
index 8498bdc..54a8999 100644
--- a/com.google.eclipse.protobuf.ui/plugin.xml
+++ b/com.google.eclipse.protobuf.ui/plugin.xml
@@ -30,7 +30,7 @@
</activeWhen>
</handler>
<handler
- class="com.google.eclipse.protobuf.ui.ProtobufExecutableExtensionFactory:com.google.eclipse.protobuf.ui.commands.SmartSemicolonHandler"
+ class="com.google.eclipse.protobuf.ui.ProtobufExecutableExtensionFactory:com.google.eclipse.protobuf.ui.commands.semicolon.SmartSemicolonHandler"
commandId="com.google.eclipse.protobuf.ui.smartSemicolon">
<activeWhen>
<reference definitionId="com.google.eclipse.protobuf.Protobuf.Editor.opened">
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/CommentNodesFinder.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/CommentNodesFinder.java
new file mode 100644
index 0000000..059a2c6
--- /dev/null
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/CommentNodesFinder.java
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2011 Google Inc.
+ *
+ * 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
+ */
+package com.google.eclipse.protobuf.ui.commands.semicolon;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static com.google.eclipse.protobuf.util.SystemProperties.lineSeparator;
+import static java.util.regex.Pattern.CASE_INSENSITIVE;
+import static org.eclipse.xtext.nodemodel.util.NodeModelUtils.getNode;
+import static org.eclipse.xtext.util.Strings.isEmpty;
+import static org.eclipse.xtext.util.Tuples.pair;
+
+import java.util.List;
+import java.util.regex.*;
+
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.xtext.nodemodel.*;
+import org.eclipse.xtext.util.*;
+
+import com.google.eclipse.protobuf.model.util.INodes;
+import com.google.inject.*;
+
+/**
+ * @author alruiz@google.com (Alex Ruiz)
+ */
+@Singleton class CommentNodesFinder {
+ private static final String MATCH_ANYTHING = ".*";
+
+ @Inject private INodes nodes;
+ @Inject private final IResourceScopeCache cache = IResourceScopeCache.NullImpl.INSTANCE;
+
+ Pair<INode, Matcher> matchingCommentNode(EObject target, String... patternsToMatch) {
+ ICompositeNode node = getNode(target);
+ for (INode currentNode : node.getAsTreeIterable()) {
+ if (!nodes.isHiddenLeafNode(currentNode) || !nodes.isComment(currentNode)) {
+ continue;
+ }
+ String rawComment = ((ILeafNode) currentNode).getText();
+ if (isEmpty(rawComment)) {
+ continue;
+ }
+ String[] comment = rawComment.split(lineSeparator());
+ for (String line : comment) {
+ for (Pattern pattern : compile(patternsToMatch, target)) {
+ Matcher matcher = pattern.matcher(line);
+ if (matcher.matches()) { return pair(currentNode, matcher); }
+ }
+ }
+ }
+ return null;
+ }
+
+ private List<Pattern> compile(String[] patterns, EObject target) {
+ List<Pattern> compiled = newArrayList();
+ for (final String s : patterns) {
+ Pattern p = cache.get(s, target.eResource(), new Provider<Pattern>() {
+ @Override public Pattern get() {
+ return Pattern.compile(MATCH_ANYTHING + s + MATCH_ANYTHING, CASE_INSENSITIVE);
+ }
+ });
+ compiled.add(p);
+ }
+ return compiled;
+ }
+}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/SmartSemicolonHandler.java
similarity index 60%
rename from com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java
rename to com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/SmartSemicolonHandler.java
index 455003e..20a69a4 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/SmartSemicolonHandler.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/SmartSemicolonHandler.java
@@ -6,16 +6,17 @@
*
* http://www.eclipse.org/legal/epl-v10.html
*/
-package com.google.eclipse.protobuf.ui.commands;
+package com.google.eclipse.protobuf.ui.commands.semicolon;
import static com.google.eclipse.protobuf.protobuf.ProtobufPackage.Literals.*;
import static java.util.regex.Pattern.compile;
import static org.eclipse.xtext.util.Strings.isEmpty;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.*;
import org.apache.log4j.Logger;
-import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.*;
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.swt.custom.StyledText;
import org.eclipse.xtext.nodemodel.INode;
@@ -31,6 +32,7 @@
import com.google.eclipse.protobuf.grammar.CommonKeyword;
import com.google.eclipse.protobuf.model.util.*;
import com.google.eclipse.protobuf.protobuf.*;
+import com.google.eclipse.protobuf.ui.commands.SmartInsertHandler;
import com.google.eclipse.protobuf.ui.preferences.editor.numerictag.core.NumericTagPreferences;
import com.google.eclipse.protobuf.ui.util.Literals;
import com.google.inject.Inject;
@@ -45,6 +47,10 @@
public class SmartSemicolonHandler extends SmartInsertHandler {
private static final Pattern NUMBERS_PATTERN = compile("[\\d]+");
+ private static final IUnitOfWork.Void<XtextResource> NULL_UNIT_OF_WORK = new IUnitOfWork.Void<XtextResource>() {
+ @Override public void process(XtextResource resource) {}
+ };
+
private static Logger logger = Logger.getLogger(SmartSemicolonHandler.class);
@Inject private CommentNodesFinder commentNodesFinder;
@@ -56,36 +62,29 @@
private static final String SEMICOLON = CommonKeyword.SEMICOLON.toString();
- private static final ContentToInsert INSERT_SEMICOLON_AT_CURRENT_LOCATION = new ContentToInsert(SEMICOLON, Location.CURRENT);
-
- /** {@inheritDoc} */
@Override protected void insertContent(XtextEditor editor, StyledText styledText) {
- int offset = styledText.getCaretOffset();
- int lineAtOffset = styledText.getLineAtOffset(offset);
- int offsetAtLine = styledText.getOffsetAtLine(lineAtOffset);
- String line = styledText.getLine(lineAtOffset);
- ContentToInsert newContent = newContent(editor, styledText, line);
- if (newContent.equals(ContentToInsert.TAG_NUMBER_INSERTED)) {
- refreshHighlighting(editor);
+ StyledTextAccess styledTextAccess = new StyledTextAccess(styledText);
+ String line = styledTextAccess.lineAtCaretOffset();
+ if (line.endsWith(SEMICOLON)) {
+ styledTextAccess.insert(SEMICOLON);
return;
}
- if (newContent.location.equals(Location.END)) {
- offset = offsetAtLine + line.length();
- styledText.setCaretOffset(offset);
- }
- styledText.insert(newContent.value);
- styledText.setCaretOffset(offset + newContent.value.length());
+ waitForReconcilerToFinishWork(editor);
+ insertContent(editor, styledTextAccess);
+ refreshHighlighting(editor);
}
- private ContentToInsert newContent(final XtextEditor editor, final StyledText styledText, final String line) {
- if (line.endsWith(SEMICOLON)) {
- return INSERT_SEMICOLON_AT_CURRENT_LOCATION;
- }
+ private void waitForReconcilerToFinishWork(XtextEditor editor) {
+ editor.getDocument().readOnly(NULL_UNIT_OF_WORK);
+ }
+
+ private void insertContent(final XtextEditor editor, final StyledTextAccess styledTextAccess) {
+ final AtomicBoolean shouldInsertSemicolon = new AtomicBoolean(true);
final IXtextDocument document = editor.getDocument();
try {
- return document.modify(new IUnitOfWork<ContentToInsert, XtextResource>() {
- @Override public ContentToInsert exec(XtextResource resource) {
- int offset = styledText.getCaretOffset();
+ document.modify(new IUnitOfWork.Void<XtextResource>() {
+ @Override public void process(XtextResource resource) {
+ int offset = styledTextAccess.caretOffset();
ContentAssistContext[] context = contextFactory.create(editor.getInternalSourceViewer(), offset, resource);
for (ContentAssistContext c : context) {
if (nodes.isCommentOrString(c.getCurrentNode())) {
@@ -98,63 +97,46 @@
}
if (model instanceof Literal) {
Literal literal = (Literal) model;
- ContentToInsert content = newContent(literal);
- if (content.equals(ContentToInsert.TAG_NUMBER_INSERTED)) {
- long index = literals.calculateIndexOf(literal);
+ if (shouldCalculateIndex(literal, LITERAL__INDEX)) {
+ long index = literals.calculateNewIndexOf(literal);
literal.setIndex(index);
updateIndexInCommentOfParent(literal, index, document);
+ shouldInsertSemicolon.set(false);
}
- return content;
}
if (model instanceof MessageField) {
MessageField field = (MessageField) model;
- ContentToInsert content = newContent(field);
- if (content.equals(ContentToInsert.TAG_NUMBER_INSERTED)) {
+ if (shouldCalculateIndex(field, MESSAGE_FIELD__INDEX)) {
long index = indexedElements.calculateNewIndexFor(field);
field.setIndex(index);
updateIndexInCommentOfParent(field, index, document);
+ shouldInsertSemicolon.set(false);
}
- return content;
}
}
- return INSERT_SEMICOLON_AT_CURRENT_LOCATION;
}
});
- } catch (Throwable e) {
- logger.error("Unable to generate tag number", e);
- return INSERT_SEMICOLON_AT_CURRENT_LOCATION;
+ } catch (Throwable t) {
+ logger.error("Unable to generate tag number", t);
}
+ if (shouldInsertSemicolon.get()) {
+ styledTextAccess.insert(SEMICOLON);
+ }
+ }
+
+ private boolean shouldCalculateIndex(EObject target, EAttribute indexAttribute) {
+ INode node = nodes.firstNodeForFeature(target, indexAttribute);
+ return node == null || isEmpty(node.getText());
}
private EObject modelFrom(ContentAssistContext c) {
EObject current = c.getCurrentModel();
- if (isIndexed(current)) {
- return current;
- }
- return c.getPreviousModel();
+ boolean isIndexed = current instanceof MessageField || current instanceof Literal;
+ return (isIndexed) ? current : c.getPreviousModel();
}
- private boolean isIndexed(EObject e) {
- return e instanceof MessageField || e instanceof Literal;
- }
-
- private ContentToInsert newContent(Literal literal) {
- INode indexNode = nodes.firstNodeForFeature(literal, LITERAL__INDEX);
- return newContent(indexNode);
- }
-
- private ContentToInsert newContent(MessageField field) {
- INode indexNode = nodes.firstNodeForFeature(field, MESSAGE_FIELD__INDEX);
- return newContent(indexNode);
- }
-
- private ContentToInsert newContent(INode indexNode) {
- boolean hasIndex = indexNode != null && !isEmpty(indexNode.getText());
- return hasIndex ? new ContentToInsert(SEMICOLON, Location.END) : ContentToInsert.TAG_NUMBER_INSERTED;
- }
-
- private void updateIndexInCommentOfParent(EObject o, long index, IXtextDocument document) {
- EObject parent = o.eContainer();
+ private void updateIndexInCommentOfParent(EObject target, long index, IXtextDocument document) {
+ EObject parent = target.eContainer();
if (parent == null) {
return;
}
@@ -184,28 +166,4 @@
}
});
}
-
- private static class ContentToInsert {
- final String value;
- final Location location;
-
- static final ContentToInsert TAG_NUMBER_INSERTED = new ContentToInsert();
-
- ContentToInsert() {
- this("", Location.NONE);
- }
-
- ContentToInsert(String value, Location location) {
- this.value = value;
- this.location = location;
- }
-
- @Override public String toString() {
- return String.format("ContentToInsert [value=%s, location=%s]", value, location);
- }
- }
-
- private static enum Location {
- NONE, CURRENT, END;
- }
}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/StyledTextAccess.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/StyledTextAccess.java
new file mode 100644
index 0000000..2f6bfda
--- /dev/null
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/commands/semicolon/StyledTextAccess.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2012 Google Inc.
+ *
+ * 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
+ */
+package com.google.eclipse.protobuf.ui.commands.semicolon;
+
+import org.eclipse.swt.custom.StyledText;
+
+/**
+ * @author alruiz@google.com (Alex Ruiz)
+ */
+class StyledTextAccess {
+ private final StyledText styledText;
+
+ StyledTextAccess(StyledText styledText) {
+ this.styledText = styledText;
+ }
+
+ String lineAtCaretOffset() {
+ int offset = caretOffset();
+ int lineAtOffset = styledText.getLineAtOffset(offset);
+ return styledText.getLine(lineAtOffset);
+ }
+
+ void setCaretOffsetToEndOfLine() {
+ int offset = caretOffset();
+ int lineAtOffset = styledText.getLineAtOffset(offset);
+ String line = styledText.getLine(lineAtOffset);
+ int offsetAtLine = styledText.getOffsetAtLine(lineAtOffset);
+ offset = offsetAtLine + line.length();
+ styledText.setCaretOffset(offset);
+ }
+
+ void insert(String text) {
+ styledText.insert(text);
+ styledText.setCaretOffset(caretOffset() + text.length());
+ }
+
+ int caretOffset() {
+ return styledText.getCaretOffset();
+ }
+}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
index 7e6dadb..6da38e0 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/contentassist/ProtobufProposalProvider.java
@@ -282,7 +282,7 @@
@Override public void completeLiteral_Index(EObject model, Assignment assignment, ContentAssistContext context,
ICompletionProposalAcceptor acceptor) {
- long index = literals.calculateIndexOf((Literal) model);
+ long index = literals.calculateNewIndexOf((Literal) model);
proposeIndex(index, context, acceptor);
}
diff --git a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Literals.java b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Literals.java
index 0dcf2ea..d962ed8 100644
--- a/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Literals.java
+++ b/com.google.eclipse.protobuf.ui/src/com/google/eclipse/protobuf/ui/util/Literals.java
@@ -43,7 +43,7 @@
* the given literal.
* @return the calculated index value.
*/
- public long calculateIndexOf(Literal l) {
+ public long calculateNewIndexOf(Literal l) {
long index = -1;
List<Literal> allLiterals = getAllContentsOfType(l.eContainer(), Literal.class);
for (Literal c : allLiterals) {
