sdedic commented on code in PR #4091:
URL: https://github.com/apache/netbeans/pull/4091#discussion_r883397225


##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/api/GroovyIndex.java:
##########
@@ -348,6 +359,25 @@ public Set<IndexedMethod> getMethods(final String name, 
final String clz, QueryS
         return methods;
     }
 
+    private boolean matchInsensitiveCamelCase(String text, String where) {
+        if (text.length() > where.length()) {
+            return false;
+        }
+        String textP = text.toLowerCase();
+        String whereP = where.toLowerCase();
+        int index = -1;
+        for (int i = 0; i < textP.length(); i++) {
+            index = whereP.indexOf(textP.charAt(i), index + 1);
+            if (index == -1) {
+                return false;
+            }
+            if (whereP.length() <= (index + 1)) {

Review Comment:
   I don't understand this condition; at line 370, `index` is computed as 
`whereP.indexOf`, which can be either `< whereP.length()` or `-1`. This 
condition seems always `false` for me.



##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/api/GroovyIndex.java:
##########
@@ -593,18 +630,28 @@ private IndexedMethod createMethod(String signature, 
IndexResult map) {
 
         // Extract attributes
         int attributeIndex = signature.indexOf(';', typeIndex + 1);
+        int offsetRangeIndex = signature.indexOf(";[", attributeIndex == -1 ? 
typeIndex + 1 : attributeIndex + 1);

Review Comment:
   Hm, since `signature.indexOf(';', typeIndex + 1)` did not find even the `;` 
(-> attributeIndex == -1), it won't find `;[` either. Or in another way, if 
`attribute` part would be missing, then the `attributeIndex` will be the first 
to catch up on the `;[` string since it only looks for `;`. Take as an example 
`field;int;[100-130]` (missing attribute part).
   
   Consider ordering all mandatory parts at the start and optionals at the end. 



##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/api/GroovyIndexer.java:
##########
@@ -321,6 +323,9 @@ private void indexClass(ASTClass element, IndexDocument 
document) {
             document.addPair(FQN_NAME, element.getFqn(), true, true);
             document.addPair(CLASS_NAME, name, true, true);
             document.addPair(CASE_INSENSITIVE_CLASS_NAME, name.toLowerCase(), 
true, true);
+            StringBuilder sb = new StringBuilder();

Review Comment:
   Q: `addPair(CLASS_NAME,...)` is called multiple times (toplevel classes, 
inner classes), but [it is read as a single value 
here](https://github.com/apache/netbeans/pull/4091/files#diff-d59a38cf1daed90d439de7eb0be4bcea950ef015a4f2321d2cfdd9a50b90a104R607)
 -- will this work OK with fields of inner classes ? If not I'd suggest to file 
a bug for Groovy & fix it in a later PR.



##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/api/GroovyIndex.java:
##########
@@ -348,6 +359,25 @@ public Set<IndexedMethod> getMethods(final String name, 
final String clz, QueryS
         return methods;
     }
 
+    private boolean matchInsensitiveCamelCase(String text, String where) {
+        if (text.length() > where.length()) {
+            return false;
+        }
+        String textP = text.toLowerCase();
+        String whereP = where.toLowerCase();
+        int index = -1;
+        for (int i = 0; i < textP.length(); i++) {
+            index = whereP.indexOf(textP.charAt(i), index + 1);
+            if (index == -1) {
+                return false;
+            }
+            if (whereP.length() <= (index + 1)) {

Review Comment:
   BTW do I understand right that this check basically returns `true`, if the 
characters present in `text` are found (case-insensitive) in `where` in the 
same order, so e.g. `matchInsensitiveCamelCase("bfu", "beloved favourite user") 
== true` (see no CamelCase in either search pattern or the matched text) ?
   



##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/api/GroovyIndex.java:
##########
@@ -252,10 +253,18 @@ public Set<IndexedMethod> getConstructors(final String 
className) {
                     }
                 }
                 int flags = 0;
-                if (parts.length > 2) {
+                if (parts.length > 3) {

Review Comment:
   Please document what the `signature` syntax is; from the code it seems that 
e.g. `type` and `attribute` part may be optional (there are checks for that, 
but still better to have it documented), the `range` part is (?) mandatory.
   
   Depending on that various checks may be insufficient; e.g. there are (?) 2 
optional parts (type, attributes) separted by the same separator. So from 
notation `name;a` it's not obvious what 'a' is: a type or an attribute ?
   
   But from the Indexer code, it seems that `type` is always present, so maybe 
the checks in index reading code can be simplified ?



##########
groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/language/GroovyTypeSearcher.java:
##########
@@ -59,10 +62,38 @@ public class GroovyTypeSearcher implements IndexSearcher {
 
     @Override
     public Set<? extends Descriptor> getSymbols(Project project, String 
textForQuery, Kind kind, Helper helper) {
-        // TODO - search for methods too!!
+        GroovyIndex index = GroovyIndex.get(QuerySupport.findRoots(project, 
Collections.singleton(ClassPath.SOURCE), Collections.<String>emptySet(), 
Collections.<String>emptySet()));
 
-        // For now, just at a minimum do the types
-        return getTypes(project, textForQuery, kind, helper);
+        kind = adjustKind(kind, textForQuery);
+        
+        if (kind == QuerySupport.Kind.CASE_INSENSITIVE_PREFIX /*|| kind == 
QuerySupport.Kind.CASE_INSENSITIVE_REGEXP*/) {
+            textForQuery = textForQuery.toLowerCase(Locale.ENGLISH);
+        }
+        
+        Set<GroovyTypeDescriptor> result = new HashSet<GroovyTypeDescriptor>();
+        
+        
+        if (textForQuery.length() > 0) {
+            Set<IndexedClass> classes = null;
+            classes = index.getClasses(textForQuery, kind);
+            for (IndexedClass cls : classes) {
+                result.add(new GroovyTypeDescriptor(cls, helper));
+            }
+            
+            Set<IndexedField> fields = index.getFields(textForQuery, null, 
kind);
+            for (IndexedField field : fields) {
+                result.add(new GroovyTypeDescriptor(field, helper));
+            }
+            
+            Set<IndexedMethod> methods = index.getMethods(textForQuery, null, 
kind);
+            for (IndexedMethod method : methods) {
+                if (method.getOffsetRange(null)  != OffsetRange.NONE) {

Review Comment:
   Why is this check done for methods and not for fields + classes ? Synthetic 
methods ? Shouldn't we rather flag them with an attribute ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to