You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "sid-srini (via GitHub)" <gi...@apache.org> on 2024/09/12 09:27:24 UTC

[PR] Java completion enhanced to support no insertion of method parameters [netbeans]

sid-srini opened a new pull request, #7747:
URL: https://github.com/apache/netbeans/pull/7747

   This configurable enhancement is based on end-user feedback received in oracle/javavscode#197
   - This seems to be a matter of personal preference
   - It is not a reflection on the correctness of the default functionality.
   
   1. Added a user preference for insertion of method parameters in code completions.
       - `"completion-insert-text-parameters"`
       - default value = `true`
       - Also available via:
           - ide/editor.lib2: `EditorPreferencesKeys.COMPLETION_INSERT_TEXT_PARAMETERS`
           - java/java.completion: `Utilities.isCompletionInsertTextParameters()`
   
   2. Added disabling of the above preference from the LSP client via the client config key `PREFIX + "java.completion.disable.insertMethodParameters"`
       - Updation occurs in `TextDocumentServiceImpl.completion()`
   
   3. Added an extra arg `insertTextParams` in the following interface methods with the default value `true`:
       - `JavaCompletionTask.ItemFactory`:
           - `createExecutableItem`
           - `createThisOrSuperConstructorItem`
       - `JavaCompletionTask.TypeCastableItemFactory`:
           - `createTypeCastableExecutableItem`
   
   4. Invoked the above methods in `JavaCompletionTask` with this parameter value set by `Utilities.isCompletionInsertTextParameters()`.
   
   5. Enhanced the implementations of the above methods in `JavaCompletionCollector` and `JavaCompletionItem` to insert text for method arguments when `insertTextParams` (or `!memberRef`).
       - The cursor would be placed inside the parentheses if it is _not_ a no-args method completion.
   
   6. Added a follow-up LSP command to the completion item **"editor.action.triggerParameterHints"** for showing the signature help when not inserting text parameters. 
       - This is a necessary guide for the user. 
       - It would not be triggered without this addition, since the trigger character '(' is not typed by the user.
   
   7. Unrelated minor fixes in `JavaCompletionCollector.ItemFactoryImpl`: 
       - Added the default prefix (**"nbls"**) to the usages of the follow-up command **"java.complete.abstract.methods"**. 
       - Without this, the command would not get launched by the client. 
       - Added to the builder in `createExecutableItem()`, the instantiated follow-up command "nbls.java.complete.abstract.methods".
   


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4487862144

   Well, thanks @ebarboni more than me on the sig file updates!
   
   Have triggered the tests for you.  Will leave @lahodaj to review.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4449072181

   I have rebased the PR commits to current master and added some unit tests as requested in previous reviews.
   
   @lahodaj - Please re-run the CI checks and re-review this PR. Thank you so much.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757018763


##########
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:
   Yes. I'll add a comment to explain it. Thanks.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-2346657085

   > Is there an IDE options UI for this?!
   > 
   Thanks. I will try to add this. 
   - Currently, this is implemented only for Java.
   - So, shall I add this only in the specific UI for _Java_ within the _Editor_ > _Code Completions_ preferences? 
   - Perhaps later when someone else needs this implemented for other languages, it could be exposed there.
   
   > I'm curious at the reasoning for adding this where it is? It seems similar to the guess filled method arguments option? Between them they feel like they cover 3 states? (no fill, parameter names, guess values)
   > 
   Yes, that progression in the complexity for insertions makes sense to me.
   1. boolean "completion-insert-text-parameters":
       - on/off inserting parameter values
       - inserted values maybe parameter names or guessed local variables or something else.
       - possibly more widely available in different completion contexts
   2. boolean "guessMethodArguments": 
        - on/off guessing a value from locally available variables
        - inserted value takes effect only when the flag above is on.
        - possibly unavailable in different completion contexts
   
   Given that the latter is already a defined preference key, it seems easier to maintain backward compatibility and a minimal state increase with the added boolean flag.
   
   When implementing the UI above, I could try to introduce joint states for these 2. However, it may become difficult to move the first setting to other languages.
   
   Let me know if this analysis makes sense.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757038086


##########
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:
   Thanks. I'll add the comment explaining the hard-coding to `false`.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4350033653

   I have squashed the commits also as requested earlier.
   Please re-review. Thank you.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4487885272

   > Well, thanks @ebarboni more than me on the sig file updates!
   > 
   
   Thanks @ebarboni for the sig file updates. :)
   


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4342728732

   I have rebased the PR commits to current master and updated the java.completion module sig file for the sigtest CI check. I'm sorry for the delay in doing this.
   
   @lahodaj or @dbalek - Please re-run the CI checks and re-review this PR. Thank you so much.
   


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj merged PR #7747:
URL: https://github.com/apache/netbeans/pull/7747


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-2346534267

   > I went through the patch - overall seems sensible. Added a few comments here and there. But, I think @dbalek should review - he might have some different views.
   > 
   > My probably biggest complaint would be that there are not tests checking the new behavior.
   
   Thanks a lot @lahodaj. 
   I'll try to add some tests for the existing and new behaviour.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757160826


##########
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:
   I have removed the unrelated grammatical change in this file. Thanks.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
lahodaj commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-3264796296

   @sid-srini
   Please note you need to fix the sigtest, and squash. To fix the sigtest:
   ```
   cd java/java.completion
   ant gen-sigtest
   ```
   


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4477897756

   Sig files for NB30 now merged.  This will require rebasing and fixing up before merging, but removing the blocking label.  Thanks for waiting.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757018763


##########
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:
   Yes



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "lahodaj (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757038086


##########
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:
   Thanks. I'll add the change.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-3265286647

   Thank you @dbalek and @lahodaj. I will re-push after making the changes.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-3212106859

   I've rebased the PR to the current master and resolved the merge conflicts.
   
   >  Let's let @dbalek review that.
   Requesting reviews from @dbalek.
   cc: @lahodaj @neilcsmith-net 
   
   Thank you
   
   


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on code in PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#discussion_r1757033650


##########
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:
   I didn't quite understand this suggestion. I refrained from adding to `SimpleValueNames` because the comment in `EditorPreferenceKeys` mentioned that the latter is meant for *private* keys, while the former for *public*, *devel* or *friend* keys.
   I can move the key here if I misunderstood. Thanks.



-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-2346772913

   Sure, I agree with keeping as two boolean flags.  The UI could handle valid combinations / or just visually disable one checkbox if desired.  I was just curious as to the different reasons / pros and cons of where the keys are registered.  Obviously the guess arguments key is in the options panel UI code itself - https://github.com/apache/netbeans/blob/master/java/java.editor/src/org/netbeans/modules/java/editor/options/CodeCompletionPanel.java#L40  Let's let @dbalek review that.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-2346592234

   Is there an IDE options UI for this?!
   
   I'm curious at the reasoning for adding this where it is?  It seems similar to the guess filled method arguments option?  Between them they feel like they cover 3 states? (no fill, parameter names, guess values)


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "sid-srini (via GitHub)" <gi...@apache.org>.
sid-srini commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4487744460

   Thanks @neilcsmith-net for the updates on the sig files. I've rebased the commit to the current master, with conflict resolution as required.
   @lahodaj - Please re-trigger the CI checks and review the PR. Thanks a lot.


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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


Re: [PR] Java completion enhanced to support no insertion of method parameters [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7747:
URL: https://github.com/apache/netbeans/pull/7747#issuecomment-4449178959

   Triggered tests.
   
   Added `do-not-merge` for now as this changes a sig file, which is a problem to merge during delivery phases.  Should wait until release is complete, NB30 sig files merged, and rebased again if necessary.
   
   cc/ @ebarboni 


-- 
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: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

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