Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Several improvements to Code Completion plugin

<< < (16/29) > >>

Huki:
Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:

--- Code: ---int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

--- End code ---

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.


--- Code: ---Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -249,8 +249,9 @@
                 if (!word.IsEmpty())
                 {
                     NameUnderCursor.Clear();
-                    if (GetLastNonWhitespaceChar(control, start) == _T('~'))
-                        NameUnderCursor << _T('~');
+                    //FIX(huki), don't prepend '~' to NameUnderCursor (only used for displaying in right-click popup menu)
+                    //if (GetLastNonWhitespaceChar(control, start) == _T('~'))
+                    //    NameUnderCursor << _T('~');
                     NameUnderCursor << word;
                     ReturnValue = true;
                     IsInclude = false;
@@ -2182,8 +2233,9 @@
     const int startPos = editor->GetControl()->WordStartPosition(pos, true);
     const int endPos   = editor->GetControl()->WordEndPosition(pos, true);
     wxString target;
+    bool isDestructor = false;  //NOTE(huki), don't prepend '~' to target, use isDestructor flag instead
     if (CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T('~'))
-        target << _T('~');
+        isDestructor = true; //target << _T('~');
     target << editor->GetControl()->GetTextRange(startPos, endPos);
     if (target.IsEmpty())
         return;
@@ -2201,7 +2253,7 @@
     CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
 
     // special handle destructor function
-    if (target[0] == _T('~'))
+    if (isDestructor)   //if (target[0] == _T('~'))
     {
         TokenIdxSet tmp = result;
         result.clear();
@@ -2211,11 +2263,15 @@
             const Token* token = tree->at(*it);
             if (token && token->m_TokenKind == tkClass)
             {
-                token = tree->at(tree->TokenExists(target, token->m_Index, tkDestructor));
+                token = tree->at(tree->TokenExists(_T("~") + target, token->m_Index, tkDestructor));
                 if (token)
                     result.insert(token->m_Index);
             }
         }
+
+        // no destructor found, but could be a variable.
+        if (result.empty())
+            result = tmp;
     }
     // special handle constructor function
     else

--- End code ---

Let me know if it's ok for commit.

Huki:
@ollydbg: Also, I think you can go ahead and commit your patch for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.

ollydbg:

--- Quote from: Huki on April 30, 2014, 06:27:30 pm ---@ollydbg: Also, I think you can go ahead and commit your patch for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.


--- End quote ---
Hi, Huki, thanks, the mentioned patch is already in trunk, see: Re: Find Declaration of constructor doesn't work


--- Quote from: Huki on April 30, 2014, 06:14:04 pm ---Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:

--- Code: ---int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

--- End code ---

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.
...
...
...
Let me know if it's ok for commit.


--- End quote ---
I will check your patches in the following days. Thanks.

Huki:
Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:


--- Code: ---Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 

--- End code ---

ollydbg:

--- Quote from: Huki on June 16, 2014, 12:56:18 am ---Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:


--- Code: ---Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 

--- End code ---


--- End quote ---
Hi, Huki, thanks, tested and applied in trunk now (revision 9802).

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version