lahodaj commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1756981981
##########
java/java.completion/src/org/netbeans/modules/java/completion/JavaCompletionTask.java:
##########
@@ -3926,7 +3926,7 @@ public boolean accept(Element e, TypeMirror t) {
switch (e.getKind()) {
case METHOD:
ExecutableType et = (ExecutableType) asMemberOf(e, type,
types);
-
results.add(itemFactory.createExecutableItem(env.getController(),
(ExecutableElement) e, et, anchorOffset, null, typeElem !=
e.getEnclosingElement(), elements.isDeprecated(e), false, false,
isOfSmartType(env, et, smartTypes), env.assignToVarPos(), true));
+
results.add(itemFactory.createExecutableItem(env.getController(),
(ExecutableElement) e, et, anchorOffset, null, typeElem !=
e.getEnclosingElement(), elements.isDeprecated(e), false, false,
isOfSmartType(env, et, smartTypes), env.assignToVarPos(), true, false));
Review Comment:
FWIW, this use is in `addMethodReferences`, and the value of
`insertTextParams` is irrelevant, so can be hardcoded to `false`.
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaCompletionCollector.java:
##########
@@ -1082,23 +1082,24 @@ private Completion createExecutableItem(CompilationInfo
info, ExecutableElement
sortParams.append(paramTypeName);
if (!inImport && !memberRef) {
VariableElement inst = instanceOf(tm, paramName);
-
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ if (insertTextParams)
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ else if (cnt == 1) insertText.append("$1");
asTemplate = true;
}
if (tIt.hasNext()) {
labelDetail.append(", ");
sortParams.append(',');
if (!inImport && !memberRef) {
Review Comment:
The `insertTextParams` test can be hoisted to this if, correct?
```suggestion
if (!inImport && !memberRef && insertTextParams) {
```
##########
ide/editor.settings/src/org/netbeans/api/editor/settings/SimpleValueNames.java:
##########
@@ -295,7 +295,7 @@ public final class SimpleValueNames {
public static final String COMPLETION_AUTO_POPUP =
"completion-auto-popup"; // NOI18N
/**
- * Whether the code completion query search will be case sensitive
+ * Whether the code completion query search will be case-sensitive
* Values: java.lang.Boolean
*/
public static final String COMPLETION_CASE_SENSITIVE =
"completion-case-sensitive"; // NOI18N
Review Comment:
It might make sense to put the option name to the API, as is done for other
options.
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaCompletionCollector.java:
##########
@@ -1073,7 +1073,7 @@ private Completion createExecutableItem(CompilationInfo
info, ExecutableElement
break;
}
if (!inImport && !memberRef && cnt == 0 &&
cs.spaceWithinMethodCallParens()) {
Review Comment:
I think I would merge the `insertTextParams` tests to existing `if`, where
possible:
```suggestion
if (!inImport && !memberRef && cnt == 0 &&
cs.spaceWithinMethodCallParens() && insertTextParams) {
```
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaCompletionCollector.java:
##########
@@ -1082,23 +1082,24 @@ private Completion createExecutableItem(CompilationInfo
info, ExecutableElement
sortParams.append(paramTypeName);
if (!inImport && !memberRef) {
VariableElement inst = instanceOf(tm, paramName);
-
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ if (insertTextParams)
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ else if (cnt == 1) insertText.append("$1");
Review Comment:
I assume this is to ensure the cursor is put inbetween `()`.
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaCompletionCollector.java:
##########
@@ -1082,23 +1082,24 @@ private Completion createExecutableItem(CompilationInfo
info, ExecutableElement
sortParams.append(paramTypeName);
if (!inImport && !memberRef) {
VariableElement inst = instanceOf(tm, paramName);
-
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ if (insertTextParams)
insertText.append("${").append(cnt).append(":").append(inst != null ?
inst.getSimpleName() : paramName).append("}");
+ else if (cnt == 1) insertText.append("$1");
asTemplate = true;
}
if (tIt.hasNext()) {
labelDetail.append(", ");
sortParams.append(',');
if (!inImport && !memberRef) {
if (cs.spaceBeforeComma()) {
- insertText.append(' ');
+ if (insertTextParams) insertText.append(' ');
}
- insertText.append(',');
+ if (insertTextParams) insertText.append(',');
if (cs.spaceAfterComma()) {
- insertText.append(' ');
+ if (insertTextParams) insertText.append(' ');
}
}
} else if (!inImport && !memberRef &&
cs.spaceWithinMethodCallParens()) {
Review Comment:
```suggestion
} else if (!inImport && !memberRef &&
cs.spaceWithinMethodCallParens() && insertTextParams) {
```
--
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
