Several improvements to Code Completion plugin

[Huki]

Hello,
I've been using CB for a few months now and love it. As I started noticing some parsing bugs and areas of improvement in CC features, I decided to take a look at it and have so far managed to fix most of the things I had in mind. I've attached a patch file and commented all my changes, let me know if something is not clear.

To summarize the important changes:
1) Full file parsing (i.e., expand each #include file and recurse), no need to keep priority headers. Previously when the parser finds an #include token, the file was simply added to the end of the parsing queue, so although all the include files are getting parsed anyway, it is done in the wrong order. Now the #include file parsing is done immediately instead of getting queued. It also means there is no need for 3-phase parsing (priority headers, non-priority headers, sources) anymore, simply parsing the source files is enough.

2) The "Grey out inactive code" feature now uses CC plugin support. I had to study carefully the CB SDK vs Scintilla communication to get this done in a clean way. I'm happy with my current implementation but let me know if it needs improvement. Despite the limitations of CC plugin, this feature is working nicely for most practical purposes. In case a token is #defined and used within the same file, their order is checked too. This ensures header files (with header guards) do not get totally greyed out.

3) CC support for DirectX (and related typedefs) and full WinAPI (including macro and function calltips). Parses function pointers and variables defined with constructor (eg., wxString myString(_T("text")); ), recursive union inside union, etc. and several other fixes. Improved tooltip behavior and speed, for both CC and Debugger.

With my 2008 PC (Core 2 Duo 2.6 GHz) and all parsing options enabled (parse global / local headers, preprocessor, complex macros) it takes about 20sec to parse the full CodeBlocks project, not bad at all.
All changes are checked with rev 9271.


[attachment deleted by admin]

[oBFusCATed]

Can you provide separate patches for all different fixes, so they can be reviewed on a per fix basis?
Reviewing one massive patch is a night mare and minimizes the chance of a fix getting in.

[BlueHazzard]

also look at this thread: http://forums.codeblocks.org/index.php/topic,18114.45.html
i think alpha has made some changes, maybe you both can sync your patches (i don't have revised any of the patches, but i thought i will inform you that there is ongoing work on the CC plugin)

With my 2008 PC (Core 2 Duo 2.6 GHz) and all parsing options enabled (parse global / local headers, preprocessor, complex macros) it takes about 20sec to parse the full CodeBlocks project, not bad at all.
All changes are checked with rev 9271.

these sounds great!

greetings

[Huki]

Ok, I'll try to split the patch file for each fix / improvement. Several fixes belong to the same file, but I'll see what I can do.

@BlueHazzard: Thanks for the link, I'm not sure if they are working on the current CC plugin or something new entirely (seems like the latter). Will check it in detail soon.

[oBFusCATed]

Ok, I'll try to split the patch file for each fix / improvement. Several fixes belong to the same file, but I'll see what I can do.

You can do it, pretty easily with git and git gui.
You can use the repo at https://github.com/obfuscated/codeblocks_sf

@BlueHazzard: Thanks for the link, I'm not sure if they are working on the current CC plugin or something new entirely (seems like the latter). Will check it in detail soon.

In fact Alpha is extracting some API for writing CC plugins and he is modifying the current CC plugin to use the API.
I've looked at your changes and you're adding something similar, but you're doing it in a pretty unacceptable way (I'm talking about the wxdynamiclib stuff and all the modifications to wxscintilla). But I'm waiting for the separate patches to post some more thorough comments.

[ollydbg]

@Huki, very good work, thanks.
I will apply the patch, and test it. It is better have a patch serials not a big one. As OBF said, git can splite a big patch and generate such patch serials.

About the parsing strategy, my concern is that recursive expanding include files in a cpp file will parse a lot of same header files for many times.

[Alpha]

I have not had a chance to look through your work (my priority is currently on CC API), but the features sound useful.  The pure parsing related parts, *should* have no conflicts with my modifications, I believe.

[Huki]

I decided to split the patch file manually with a text editor this time, I think it should work. I've attached the patches set below. I'll try out git hopefully tomorrow or next week, and sync my changes with the github repo.

About the parsing strategy, my concern is that recursive expanding include files in a cpp file will parse a lot of same header files for many times.

Till now I had not checked this part in detail (i.e., whether already parsed files are parsed again and again, or not), but now I can confirm: already parsed headers actually get skipped. In fact it's not going to make a difference even if we change this behavior, because tokens are not managed per-file but project wide. In this case it only matters for us to ensure tokens are collected in the correct order. If they are already collected, we don't achieve anything by reparsing the file. So the accuracy is good enough to avoid the use of priority headers, while having almost no impact on performance (because we are still parsing the same number of files - in fact lesser as we don't have a separate headers parsing phase).

[oBFusCATed]

Hm...

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

I'm particularly interested in the two debugger patches and the thread search one.

[Huki]

I'll quickly explain the patches you asked for, rest I'll try to do later (please let me know if there is anything else specifically).

debugger_tooltip:
- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.
- Placed the tooltip below the mouse pointer, rather than directly on it. This makes it possible to select text (in the editor) even if the tooltip was triggered. In addition, the user must be able to move the mouse pointer into the tooltip (mainly to expand items in the tooltip), so we should not dismiss the tooltip as soon as the mouse is moved (give a safe area outside the tooltip where the mouse can move without accidentally dismissing the tooltip).
- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.
- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).

debugger_fixes:
- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread.
- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.

thread_search:
- Do not automatically prepend the tilde sign if found. Users can still select the text including the ~ if they wish to include it in the search text. I have seen several cases where the tilde is used as the bitwise NOT operator, so shouldn't always assume for C++ destructor. I've also done similar fixes in Code Completion plugin.

[oBFusCATed]

- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.

Can you provide the steps, where the old one failed?

- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.

This is more of a black magic, not science, but I'll see if it makes a difference.

- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).

I still don't understand...Steps to reproduce?

- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread.

I think this is fixed already. Are you able to reproduce the problem without this patch using latest trunk?

- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.

Hm, I'm not sure if this is not going to break something else...

thread_search:
- Do not automatically prepend the tilde sign if found. Users can still select the text including the ~ if they wish to include it in the search text. I have seen several cases where the tilde is used as the bitwise NOT operator, so shouldn't always assume for C++ destructor. I've also done similar fixes in Code Completion plugin.

It sounds like we need to move the cbDebuggerPlugin::GetEditorWordAtCaret to the EditorManager class, so all plugins can reuse it or in cbPlugin...

[Huki]

- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.

Can you provide the steps, where the old one failed?

When a line is not fully selected, the old test would detect the unselected parts of the line as selected and display tooltip on it (we only want to display debugger tooltip on the selected text).

- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.

This is more of a black magic, not science, but I'll see if it makes a difference.

Hmm no, we need to make sure the "get rid of other calltips" is the first test to be done. If there are any tests done before it, and they already fail and return, the existing tooltips never gets canceled, hence the bug.

- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).

I still don't understand...Steps to reproduce?

This one not a bugfix, I've added a small feature: press and hold Ctrl key to temporarily disable the debugger tooltip.

- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread.

I think this is fixed already. Are you able to reproduce the problem without this patch using latest trunk?

When I updated to rev 9271 I noticed there was an attempt to fix this. I haven't tested if it works. I still found my modification relevant (I commented that part). RunQueue() is already called at the end of the procedure, so we'd better flag the m_ProgramIsStopped, and let RunQueue() reset it if required.

- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.

Hm, I'm not sure if this is not going to break something else...

I can confirm the path is only used with wxSetWorkingDirectory(), never sent to GDB. So at least for now it should be safe to remove ConvertToGDBDirectory();

[Huki]

- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.

Can you provide the steps, where the old one failed?

When a line is not fully selected, the old test would detect the unselected parts of the line as selected and display tooltip on it (we only want to display debugger tooltip on the selected text).

Additional info: the old test simply takes a rect from selection start to selection end, so it gives incorrect results when selecting multiple lines. I've attached some screenshots below - the red rectangle shows the incorrectly detected selection block. It's something that bothered me when I was selecting text (to copy, etc) and noticed the tooltip appearing behavior was not consistent.


I also checked if any of my other patches were in need of explanation:
scintilla_fixes: In LexerCPP::Fold(), fixed folding glitch in case of hanging braces in a preprocessor block. Eg,

Code

#ifdef _DEFINE
    if (condition1) {
#else
    if (condition2) {
#endif
        ...
    }

jump_tracker: More consistent behavior for Jump Tracker (cursor history navigation) feature (View -> Jump -> Jump Back / Frwd - I set Ctrl - Left / Right shortcuts for it as it's a very useful feature for me ).

incremental_search: Fixed search history flooding. Eg, when typing "codecomp" in the search box, it used to record in history:

Code

c
co
cod
code
...

The rest of patches are CC related...
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)

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() ...

Code: this is not a macro

#define MYDEFINE (A + B)

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

[oBFusCATed]

One note:
Scintilla fixes should go to the Scintilla project first.
Here we do use a modified Scintilla, but we prefer not to diverge too much from upstream Scintilla.
It eases the upgrade of the component a lot.

Also I think your wxDynamicLib modification to (wx)Scintilla are too much and we cannot accept them (this is my opinion and others should speak, too, especially Morten as he is the maintainer of this component in C::B).
You'll have to find another way to do it. Probably consulting with Scintilla devs for a way to do it or to ask them to add some API that will suit you.

[oBFusCATed]

Additional info: the old test simply takes a rect from selection start to selection end, so it gives incorrect results when selecting multiple lines. I've attached some screenshots below - the red rectangle shows the incorrectly detected selection block. It's something that bothered me when I was selecting text (to copy, etc) and noticed the tooltip appearing behavior was not consistent.

Understood and I was able to reproduce it. Also I was able to implement it in a simpler way:

Code

            int startPos = stc->GetSelectionStart();
            int endPos = stc->GetSelectionEnd();
            int mousePos = stc->PositionFromPointClose(mousePosition->x, mousePosition->y);
            if (mousePos == wxSCI_INVALID_POSITION)
                return wxEmptyString;
            else if (startPos <= mousePos && mousePos <= endPos)
                return selected_text;
            else
                return wxEmptyString;

Can you try if this snippet fulfils all your requirements?

BTW: Thank you for you contribution, it is appreciated.