[GitHub] [netbeans] asbachb commented on a change in pull request #3074: [NETBEANS-189] Updates for Sql autocomplete

[GitBox <gi...@apache.org>]

asbachb commented on a change in pull request #3074:
URL: https://github.com/apache/netbeans/pull/3074#discussion_r684686016



##########
File path: ide/db.sql.editor/src/org/netbeans/modules/db/sql/editor/completion/SQLCompletionProvider.java
##########
@@ -41,39 +40,61 @@
  * @author Andrei Badea
  */
 public class SQLCompletionProvider implements CompletionProvider {
-
+    @Override
     public CompletionTask createTask(int queryType, JTextComponent component) {
-        if (queryType == CompletionProvider.COMPLETION_QUERY_TYPE || queryType == CompletionProvider.COMPLETION_ALL_QUERY_TYPE) {
+        if (queryType == CompletionProvider.COMPLETION_QUERY_TYPE || 
+                queryType == CompletionProvider.COMPLETION_ALL_QUERY_TYPE) {
+            
+            /* to support DB related completion tasks (i.e. auto populating table 
+            names or db columns for given schema) check for connection */
             DatabaseConnection dbconn = findDBConn(component);
-            if (dbconn == null) {
-                // XXX perhaps should have an item in the completion instead?
-                Completion.get().hideAll();
-                SQLExecutionBaseAction.notifyNoDatabaseConnection();
-                return null;
-            }
+
             return new AsyncCompletionTask(new SQLCompletionQuery(dbconn), component);
         }
+        
+        // not a completion query type so return nothing
         return null;
     }
 
+    /**
+     * getAutoQueryTypes is invoked to check whether a popup with suggestions
+     * should be shown without the user explicitly asking for it.
+     * 
+     * If either #getAutoQueryTypes return a non-zero value or the user
+     * explicitly asks for completion, #createTask is invoked with the
+     * requested type. In case of SQL see
+     * org.netbeans.modules.db.sql.editor.completion.SQLCompletionQuery.
+     * 
+     * @param component
+     * @param typedText
+     * @return 
+     */
     public int getAutoQueryTypes(JTextComponent component, String typedText) {
+        // XXX: Check if "enable/disable" autocomplete is setting.  See NETBEANS-188
+        // If "." has not been typed then acceptable to start checking for options.
         if (!".".equals(typedText)) { // NOI18N
             return 0;
         }
+        // check typed text if dot is present at the selected offset
         if (!isDotAtOffset(component, component.getSelectionStart() - 1)) {
             return 0;
         }
+        
+        // check if there is a DB connection
         DatabaseConnection dbconn = findDBConn(component);
         if (dbconn == null) {
             String message = NbBundle.getMessage(SQLCompletionProvider.class, "MSG_NoDatabaseConnection");
             StatusDisplayer.getDefault().setStatusText(message);
-            return 0;
+            SQLExecutionBaseAction.notifyNoDatabaseConnection();
         }
-        if (dbconn.getJDBCConnection() == null) {
+
+        // check if DB connection is active
+        if (dbconn != null && dbconn.getJDBCConnection() == null) {

Review comment:
       From my understanding a comment describes what happening so I was puzzled to see the comment mentioning to check for an active connection when the if statement checks for an inactive db connection.
   
   So I'd name that comment `check if DB connection is inactive` or `check if DB connection is not active` or otherwise extract that check into a separate method with an appropriate naming.




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