You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/05/27 20:08:00 UTC

[GitHub] [netbeans] mbien opened a new pull request, #4165: safer auto completion chaining

mbien opened a new pull request, #4165:
URL: https://github.com/apache/netbeans/pull/4165

    - attempting to open the next completion can cause problems if the
      completion itself is not implemented correctly (infinite completion loops)
    - to mitigate this we can check if the template had a removed
      "completionInvoke" group (defined by the template param with the same name)
      so that for example constructor completions still work as expected
   
   
   ```java
   // completion for:
   new HashM
   // would have the template:
   "HashMap<>${cursor completionInvoke}"
   // and would insert
   new HashMap<>
   // and would continue with the next completion for constructors due to the removed "completionInvoke" group
   new HashMap<>()
   ```
   
   followup of #2519, #3290
   
   example of a completion which never finishes: #3805
   
   lets see if all tests are green.


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1142018137

   everything looks good so far. I did some testing with editor templates but would have to do more to be confident that this does not cause regressions.


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1152680636

   @lkishalmi Is groovy using the same API for code completion (ide/editor.codetemplates)?
   
   We tested this PR for php and java and it looks promising.


-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1159448827

   Agree.


-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1143599935

   @mbien I don't say about it. If we can implement your issue in the Java editor, we shouldn't change the editor.
   
   e.g. call the method like the following (`scheduleShowingCompletion()`) in the Java editor
   ```java
       private static ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
   
       private static void scheduleShowingCompletion() {
           service.schedule(() -> {
               Completion.get().showCompletion();
           }, 750, TimeUnit.MILLISECONDS);
       }
   ````


-- 
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

[GitHub] [netbeans] mbien commented on a diff in pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#discussion_r890274449


##########
ide/editor.codetemplates/src/org/netbeans/lib/editor/codetemplates/CodeTemplateInsertHandler.java:
##########
@@ -109,7 +109,7 @@ public final class CodeTemplateInsertHandler implements TextRegionManagerListene
     private TextSyncGroup textSyncGroup;
     
     private boolean completionInvoked;
-    
+

Review Comment:
   usually I am keeping the cleanup separate and arrange it like in this PR:
   https://github.com/apache/netbeans/pull/4142/commits
   but this is extra work since every time i modify the bugfix commit I have to resolve merge conflicts in the cleanup commit
   
   So I do that only if I think the PR complexity justifies that.
   
   This PR was fairly small and changes like the one you marked were not even intended. You can filter them out in the github UI btw. Settings cog -> hide whitespace
   
   At least I know that someone is inspecting commit by commit and not just via the "Files Changed" tab so that it is worth the effort :) Thanks for the feedback.



-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1153192450

   if anyone knows someone who might want to review this, please add to PR.


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1142056903

   > The thing is that it take sometime (some ms) to add the () after I hit enter to see the ctor but this is a general problem I think. I just wanted to mentioned it here, because I think this is of this completion chaining stuff what you mentioned. I mean I really wanted the ctor and it still completes to new HashMap<>...~200ms new HashMap<>(). Why not just immediately?
   
   I don't think i can reproduce this issue. The text substitution happens as fast as i can press return twice in a row. Might be influenced by the complexity of the document/project, this PR doesn't do any heavy lifting, it only tweaks the conditions when followup/parametrized completions are triggered.


-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1143102078

   I think we should implement it in the Java editor after we revert all changes for the editor.
   I guess that we can handle it in the Java editor (Maybe, we should be able to run CC again from the Java editor if needed.) because the Java editor knows the context.
   Did you try that?
   
   I think fixing it in the editor(invoking CC infinitely) is risky.
   
   @sdedic @jlahoda Do you have any ideas or advice?


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1143540236

   @junichi11 this isn't java specific. `${cursor completionInvoke}`is part of regular completion templates.


-- 
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

[GitHub] [netbeans] tmysik commented on a diff in pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
tmysik commented on code in PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#discussion_r891589954


##########
ide/editor.codetemplates/src/org/netbeans/lib/editor/codetemplates/CodeTemplateInsertHandler.java:
##########
@@ -109,7 +109,7 @@ public final class CodeTemplateInsertHandler implements TextRegionManagerListene
     private TextSyncGroup textSyncGroup;
     
     private boolean completionInvoked;
-    
+

Review Comment:
   I simply marked the first unrelated change in the PR and added the comment, that is why a whitespace change is selected :)
   
   Personally, I prefer more commits (the cleanup one can be the first one in PR so no need to rebase/resolve conflicts :) because even in this simple one, I kind-of had to search what changed exactly :)
   
   Just my nitpicks, nothing serious, of course.



-- 
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

[GitHub] [netbeans] mbien merged pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien merged PR #4165:
URL: https://github.com/apache/netbeans/pull/4165


-- 
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

[GitHub] [netbeans] Chris2011 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
Chris2011 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1143622649

   What is the delay for and why is it that high?


-- 
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

[GitHub] [netbeans] Chris2011 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
Chris2011 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1142042729

   The thing is that it take sometime (some ms) to add the () after I hit enter to see the ctor but this is a general problem I think. I just wanted to mentioned it here, because I think this is of this completion chaining stuff what you mentioned. I mean I really wanted the ctor and it still completes to new HashMap<>...~200msnew HashMap<>(). Why not just now?


-- 
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

[GitHub] [netbeans] lkishalmi commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1153118925

   @mbien I'm sorry, but I do not know the Groovy implementation details.


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1159378809

   i would like to merge this one soon too so that it gets good testing in during NB 15 dev


-- 
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

[GitHub] [netbeans] mbien commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1143622882

   > @mbien I don't say about it. If we can implement your issue in the Java editor, we shouldn't change the editor.
   
   again. This is **not** a java issue. A completion template (for any language) with a `completionInvoke` parameter, should open the completion if the parameter is active. This was always the case, there was just a regression in 12.3 caused by #2519 (please read the PR text). So lets try to fix this properly in the editor lib. This PR is attempting exactly that, restore <12.3 functionality without causing regressions.


-- 
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

[GitHub] [netbeans] junichi11 commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
junichi11 commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1145621592

   @mbien I've verified #3805 doesn't occur. Thank you for your work!
   
   ![netbeans-gh-4165](https://user-images.githubusercontent.com/738383/171797332-8490375f-03e7-4f49-ac06-901fa20cb9c9.gif)
   


-- 
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

[GitHub] [netbeans] tmysik commented on a diff in pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
tmysik commented on code in PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#discussion_r890236366


##########
ide/editor.codetemplates/src/org/netbeans/lib/editor/codetemplates/CodeTemplateInsertHandler.java:
##########
@@ -109,7 +109,7 @@ public final class CodeTemplateInsertHandler implements TextRegionManagerListene
     private TextSyncGroup textSyncGroup;
     
     private boolean completionInvoked;
-    
+

Review Comment:
   Nitpick: I totally understand all the formatting and code-style changes :) However, please consider doing them at least in a separate commit, so the patch is more clear. 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

[GitHub] [netbeans] matthiasblaesing commented on pull request #4165: safer auto completion chaining

Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on PR #4165:
URL: https://github.com/apache/netbeans/pull/4165#issuecomment-1153154648

   I don't feel really qualified to review this. However I verified, that the issue is gone and the behavior seems to be fixed. The changes look ok to me.


-- 
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