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

Several improvements to Code Completion plugin

<< < (6/29) > >>

Huki:

--- Quote from: oBFusCATed on September 11, 2013, 10:31:17 pm ---1. Using PluginManager and GetDynamicLibrary is still bad. Please checkout the work done here: https://github.com/alpha0010/codeblocks_sf/tree/cc_interface
   This branch tries to implement a CCManager and similar stuff. There is a topic in the forum and I think Ollydbg posted a link to it in a previous comment.
--- End quote ---
My main concern was to safely pass the CC function pointer for Scintilla's use. Using PluginManager we ensure that each cbEditor can get hold of the CC library during its lifetime and prevents the plugin from being accidentally unloaded (eg, upon user uninstall). The good thing is that the CodeCompletion object along with the parsers and everything will get freed as the user expects, but as the library still remains loaded, it is safe for Scintilla to keep calling our eval function (which will simply report that CC isn't enabled). Once all the open cbEditors are closed the library will go away too.

I see about the CCManager work. It would definitely make it easier for the CB SDK to communicate with CC for features like these, still don't know about Scintilla. Perhaps working with PluginManager through CCManager would be cleaner rather than doing it directly from cbEditor like now.


--- Quote ---2. You still modify the lexer for C++, why don't you just find the defines for the file after parsing the includes and pass them to Scintilla, I think it is possible.
3. Using char* to store strings is highly discouraged.
--- End quote ---
There is not only #ifdef style defines, we also need to handle defines with value, or defined to other defines, function-like macros. The Scintilla lexer's expression parser is also bare-bones... on the other hand CC's expression solver will give us consistent results. Letting Scintilla call a CC function to get the result seems like the best option before me.
What I could do is limit all Scintilla modifications to LexerCPP.cxx. Using Scintilla lexer's PrivateCall() support this should be possible. It will also eliminate the need for char*.


--- Quote ---4. I think this will be one of the harder to apply patches, because it does weird things in many subsystems. Probably you should concentrate on the simpler things getting in and then this one.

--- End quote ---
I was going to suggest the same actually. I'll put this one on hold till we've dealt with the other patches.

Huki:

--- Quote from: ollydbg ---I got an error when apply one patch:
--- End quote ---
Oh, sorry about that. I took care to keep extra lines, must have missed this one.


--- Quote from: ollydbg on September 13, 2013, 05:20:25 pm ---About cc_enum_values.patch
I think it is OK. I see you want to use the expression solver to calculate the enum value, which means if you are going to parse the code:


--- Code: ---enum A
{
x = 1+2;
}
--- End code ---

Then, you will get the correct enum value of x. I think a simple test code is necessary, you can simply follow the file under: \src\plugins\codecompletion\testing, maybe the enumerator.cpp, thanks.

--- End quote ---
I'm sending a patch file for \src\plugins\codecompletion\testing\enumerator.cpp to show some supported cases.
Note that my current implementation doesn't check for correct parent, it just accepts any parent (I'm talking about enums defined inside a particular class or namespace). Maybe I'll try to finish this part soon.

Huki:

--- Quote from: ollydbg on September 13, 2013, 05:37:20 pm ---
--- Quote from: oBFusCATed on September 08, 2013, 01:25:57 am ---Hm...

Can you post thorough descriptions for the problems you try to solve with every patch?
Steps to reproduce will be useful, too.

--- End quote ---
About the CC part: I'm interested on the patch: cc_fix_tokenizer.patch, cc_fix_ctor_dtor.patch, thanks. Maybe other cc patches, but I'm not viewing them yet. ;)

--- End quote ---
For cc_fix_tokenizer.patch I already explained in a previous post. I'm quoting it again for convenience:

cc_fix_tokenizer: In Tokenizer::ReadToEOL(), which is used by HandleDefines(),

--- Code: ---    //FIX(huki) if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
    if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)

--- End code ---
i.e., do not remove the leading space. This is to differentiate defines containing brackets, from legitimate macros. Eg,

--- Code: (This is a macro) ---#define MYMACRO() ...
--- End code ---

--- Code: (This is NOT a macro) ---#define MYDEFINE (A + B)
--- End code ---

The fix in Tokenizer::ReadParantheses() support '==' (previously it was taken as two assignments, i.e., ' = = '.
Rest should be commented already in the patch.


I'll discuss the others later tonight.

ollydbg:

--- Quote from: Huki on September 14, 2013, 08:05:20 am ---For cc_fix_tokenizer.patch I already explained in a previous post. I'm quoting it again for convenience:

--- End quote ---
Thanks.


--- Quote ---cc_fix_tokenizer: In Tokenizer::ReadToEOL(), which is used by HandleDefines(),

--- Code: ---    //FIX(huki) if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
    if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)

--- End code ---
i.e., do not remove the leading space. This is to differentiate defines containing brackets, from legitimate macros. Eg,

--- Code: (This is a macro) ---#define MYMACRO() ...
--- End code ---

--- Code: (This is NOT a macro) ---#define MYDEFINE (A + B)
--- End code ---

--- End quote ---
I understand your change of the ReadToEOL(), I see that the only usage/call of the ReadToEOL function is in HandleDefines(), so here we should read the first token after the "#define", after that, we should check if there is a "(" directly after the token, if yes, then this is a function like macro, if not, this is a variable like macro.
The parameter stripUnneeded means: remove comments and compression spaces(two or more spaces should become one space), I think you change is OK. The string returned from the ReadToEOL have two cases:
If the string is beginning of the space -> this is a variable like macro definition, otherwise, it is a function like macro definition.
I think I need to update the comments in either HandleDefines and ReadToEOL function now. :)

Huki:
I have updated the cc_enum_value patch:
- In case of enum assignment to previous enum, it now looks under the correct parent instead of doing a global search.
- If the expression cannot be evaluated, leave it blank instead of resetting to zero (further enums will also be left blank till the next successful assignment).

In case you've already applied my previous patch, note that I have removed my changes to tokentree.cpp / tokentree.h (in my previous patch I had added a new TokenTree::TokenExists() function).

I'm trying to get each patch committed one by one, let me know if this one is ok.

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version