Include completion improved

[oBFusCATed]

Hello,
Here is a patch for the codecompletion plugin,
that tries to improve the include file completion.

Improvements:
1. Parse correctly:

Code

#    include "ala-bala"
    #   include "ala-bala"

2. Use the project/active target include dirs to simplify the entries in the auto-complete list.
In my projects, I always put the source (*.h/*.cpp) files in subdirectory "src" and add it as an include path to the projects.
This let me use the headers without the "src/" prefix.
In the patch, the file path is modified, so it doesn't start with the include dir (the dir is stripped from the path).
So I can type the file path relative to the src directory and the completion list has the correct entries.
I've tested it only on linux and I need some help to handle paths in platform independent way (handle paths with \ instead of /)
Also I'm not sure how to handle ./ or ../ added to include dir. Any help here will be appreciated

Code

#include "file1.h"
#include "dir1/file2.h"

3. Fixed a bug in the usage of the "caseSens" option

Code

-        ed->GetControl()->AutoCompSetIgnoreCase(caseSens);
+        ed->GetControl()->AutoCompSetIgnoreCase(!caseSens);

KNOWN PROBLEM:
The closing '"' or '>' is not added automatically and I couldn't find a way to implement it.
If anyone knows (and shares this info) if there is a message, that I can handle to add the one of the two characters, I'll be very thankful.

Best regards

p.s. the patch is attached
p.s.s. the patch is made against the 5617 wxaui branch


[attachment deleted by admin]

[ollydbg]

Your message has much information, so, it will take me a lot of time to absorb.

Lets take the path related aspect, I found it was there: In Globals.cpp

Code

bool NormalizePath(wxFileName& f,const wxString& base)
{
    bool result = true;
//    if(!f.IsAbsolute())
    {
        f.Normalize(wxPATH_NORM_ALL & ~wxPATH_NORM_CASE, base);
        result = f.IsOk();
    }
    return result;
}

In Parser.cpp

wxArrayString Parser::FindFileInIncludeDirs(const wxString& file,bool firstonly)
{
    wxArrayString FoundSet;
    for(size_t idxSearch = 0; idxSearch < m_IncludeDirs.GetCount(); ++idxSearch)
    {
        wxString base = m_IncludeDirs[idxSearch];
        wxFileName tmp = file;
        NormalizePath(tmp,base);
        wxString fullname = tmp.GetFullPath();
        if(wxFileExists(fullname))
        {
            FoundSet.Add(fullname);
            if(firstonly)
                break;
        }
    } // end for : idx : idxSearch
//    Manager::Get()->GetLogManager()->DebugLog(_T("Searching %s"), file.c_str());
//    Manager::Get()->GetLogManager()->DebugLog(_T("Found %d"), FoundSet.GetCount());
    return FoundSet;
} // end of FindFileInIncludeDirs

Also, I searched on Google, and this page will give a more details:

http://lists.wxwidgets.org/pipermail/wx-users/2007-January/096389.html

> *bool* *Normalize*(*int */flags = wxPATH_NORM_ALL/, *const wxString&
> */cwd = wxEmptyString/, *wxPathFormat */format = wxPATH_NATIVE/)
>
> Normalize the path: with the default flags value, the path will be made
> absolute, without any ".." and "." and all environment variables will be
> expanded in it this may be done using another (than current) value of cwd

Maybe, it's helpful or not, I'm not sure.

[killerbot]

In my projects, I always put the source (*.h/*.cpp) files in subdirectory "src" and add it as an include path to the projects.
This let me use the headers without the "src/" prefix.
In the patch, the file path is modified, so it doesn't start with the include dir (the dir is stripped from the path).
So I can type the file path relative to the src directory and the completion list has the correct entries.

Is not already the case that CB offers both, like :
 "./src/MyHeader.h
 "MyHeader.h"  ??

Should recheck.

While you are at it, could you look at the following issue : I am trying out another patch that tries to put closing quotes/double quotes/braces/brackets/... when you type the opening one.
This has however a negative effect on the include completion since what happens is :

I type :

Code

#include "

and I get :

Code

#include ""

with the cursor between the 2 ". But that has killed the include completion.

[oBFusCATed]

ollydbg, thanks I'll take a look at it tonight.

Is not already the case that CB offers both, like :
 "./src/MyHeader.h
 "MyHeader.h"  ??

Should recheck.

You talk about the case when you have both files, and the ./src is added to the include paths?

While you are at it, could you look at the following issue : I am trying out another patch that tries to put closing quotes/double quotes/braces/brackets/... when you type the opening one.
This has however a negative effect on the include completion since what happens is :

I type :

Code

#include "

and I get :

Code

#include ""

with the cursor between the 2 ". But that has killed the include completion.

I suppose you talk about: http://forums.codeblocks.org/index.php/topic,10361.0.html
I've not tested it, because I don't like the feature. I'll give it a try.
I suppose the problem is something like that:
1. The user types a " or <
2. The list opens
3. The brace completion kicks in and places a character
4. The list closes because of the placed character

[killerbot]

I guess something like that, or the character inserted event is no longer past on .. to investigate ;-)

[oBFusCATed]

Here is a little update:

1. Used NormalizePath when obtaining the include dirs
2. Replace '\' with '/' everywhere, because I'm not sure what character is used on windows for the paths and also that is the standard (portable) way to write include paths.

[attachment deleted by admin]

[killerbot]

ok, I have done a very quick look at the (original/official) code. And there are a few things that could be improved.

Step 1 : get rid of unused stuff :

Code

    // find opening quote (" or <)
    int idx = pos - lineStartPos;
    int found = -1;
    wxString filename;
    while (idx > 0)
    {
        wxChar c = line.GetChar(idx);
        if (c == _T('>'))
            return; // the quote is closed already...
        else if (c == _T('"') || c == _T('<'))
        {
            if (found != -1)
                return; // the already found quote was the closing one...
            found = idx + 1;
        }
        else if (c != _T(' ') && c != _T('\t') && !found)
            filename << c;
        --idx;
    }

The filename is NOT used, so in my private copy I will first make things simpler, and remove it.


Secondly :

Code

    // fill a list of matching project files
    wxArrayString files;
    for (int i = 0; i < pPrj->GetFilesCount(); ++i)

So only file of the current project seems to be added. So no files in that nice include directory that I added to my project ;-)

And then we see :

Code

            wxFileName fname(pf->relativeFilename);
            if (files.Index(pf->relativeFilename) == wxNOT_FOUND)
                files.Add(pf->relativeFilename);
            if (files.Index(fname.GetFullName()) == wxNOT_FOUND)
                files.Add(fname.GetFullName());

So that means it should be twice in the list : full path and relative.
EDIT : well actually something like this; Say I have a header in my project whose relative path to the cbp file is ../../inc/MyHeader.h, then there will be 2 entries in the list :
1)../../inc/MyHeader.h
2)MyHeader.h

Note : this is from looking at the code, not from testing.


Ideas :
* Also add all headers that can be found in the include dirs.
* add all headers of all projects in the workspace
* ...

off course the more files being added the slower things will get.

But as it is now, I agree, most of the time the completion for include files just fails.

[killerbot]

I can confirm :

Code

        ed->GetControl()->AutoCompSetIgnoreCase(caseSens);

should be

Code

        ed->GetControl()->AutoCompSetIgnoreCase(!caseSens);

Will already fix this in svn.

[oBFusCATed]

Hi, great review for the official code, can you do the same with my patch? 

My patch is concerned with making the completion usable and adding files outside of the project is out of its scope.  :lol:

But you are right, it would be nice to have the compiler includes completed, also on linux /usr/include/ is a good candidate.
But this requires some kind of caching, there is no need to traverse the filesystem every time the user wants to complete an include!

There should be some discussion and if there is an agreement how the thing should be implemented I can do it  :lol:

[danselmi]

Hi

Are you sure you tested the last patch from http://developer.berlios.de/patch/?func=detailpatch&patch_id=2746&group_id=5358?

Because one of the first tests in BraceCompletion patch is

Code

 if ( IsComment(style) || IsPreprocessor(style) ) return; 

[killerbot]

I updated the application of the patch, I was using an older version.  :?

Seems to work better, and at first sight seems to work better on other issues too ....

[MortenMacFly]

I updated the application of the patch, I was using an older version.  :?

Just for the record: Please be careful with CC patches. Remember we are testing a massively enhanced version. I'll try to keep everything in sync but probably it's better to inform (me) before adding to trunk. Otherwise it's getting more and more hard for me... :roll:

[killerbot]

that patch kicks in on the cbeditor, not the CC.

I am currently only looking at CC for the : "CodeCompleteIncludes()"

[oBFusCATed]

Is my patch being applied? Because it seems so...

[killerbot]

I think I did, can you double check the source code ?

[oBFusCATed]

I've checked and it seems that it is fully applied. I'm looking at the CC branch.

Thanks