Bug in 'swap header / source'

[ironhead]

I have a project that comprises many directories, and in those directories the same file names are generally repeated.  Basically:

<root>
---<sub dir1>
    Config.cpp
    Config.h
---<sub dir2>
    Config.cpp
    Config.h

What I find is that if I open Config.cpp in <sub dir2> and right click on the tab and select 'swap header / source', I get Config.h from <sub dir1>.

What I find interesting is that if I go to the '#include <Config.h>' line in Config.cpp in <sub dir2> and right click and select 'Open #include file: Config.h', the proper Config.h (from <sub dir2>) is opened.

[stahta01]

Might be a Code::Blocks bug.

FYI, You should be using '#include "Config.h"' instead of '#include <Config.h>'  the "<>" means system header in many C compilers. But, this should not cause the problem you are seeing. I have no idea where to look for cause and not the time to duplicate it.

Tim S.

[oBFusCATed]

I've observed the same bug/missing feature

[ollydbg]

rev 5850

* applied (modified) patch #2803 - faster header source swapping (thanks techy)

This function has recently improved. seems a regression.

[Zini]

Confirmed. If there is more than one file with the same name, it is semi-random which file you get.
From looking at the source it seems like Code::Blocks is simply listing all possible directories and then picks a file with a suitable name. At least I guess that is what it is doing. A single function consisting of over 200 lines of codes (several screen pages) is a bit hard to parse. Am I the only one who thinks, that Code::Blocks is in dire need of some refactoring?

A quick fix might be to first look in the same direcotry as the original file. But actually the situation is a bit more complicated. Let's assume this directory layout:

A
-- include
---- foo.h
-- src
---- foo.cpp
B
-- include
---- foo.h
-- src
---- foo.cpp

If you want to support these, you need a way to tell the Swap-feature about your directory layout (some additional configuration data in the project file, maybe). One *could* try to derive an algorithm instead for guessing the right file, but to me this sounds just lke asking for trouble.

[stahta01]

One *could* try to derive an algorithm instead for guessing the right file, but to me this sounds just lke asking for trouble.

The proper algorithm is the same one the Compiler Uses. Look for it the same way using the search directories from project settings.
Just it seems hard to do from my point of view.

Tim S.

[oBFusCATed]

I would prefer a dialog, so I can choose the file I want

[Zini]

The proper algorithm is the same one the Compiler Uses. Look for it the same way using the search directories from project settings.

Sorry, that is not correct. The algorithm for choosing include files (I assume that this is the one you mean) is entirely useless here. IIRC if you have more than one header file with the same name in your collection of include paths, gcc usually picks the first one.

[Zini]

I would prefer a dialog, so I can choose the file I want

That might be an option, though it would not handle the case where the corresponding h/cpp file does not exist yet (the swap feature used to offer an option to create it in this case).

[stahta01]

The proper algorithm is the same one the Compiler Uses. Look for it the same way using the search directories from project settings.

Sorry, that is not correct. The algorithm for choosing include files (I assume that this is the one you mean) is entirely useless here. IIRC if you have more than one header file with the same name in your collection of include paths, gcc usually picks the first one.

If the proper header file exists; it MUST use the same method. Or the solution is in-correct.
The problem is what to do if it does not exist.

Tim S.

[ironhead]

FYI, You should be using '#include "Config.h"' instead of '#include <Config.h>'  the "<>" means system header in many C compilers.

Yep, I use '#include "Config.h"', just a typo in my original post...

Cheers!

Chris

[ollydbg]

we can have a look at this function in editormanager.cpp

Code

bool EditorManager::SwapActiveHeaderSource()
{
    cbEditor* ed = GetBuiltinEditor(GetActiveEditor());
    if (!ed)
        return false;

    FileType ft = FileTypeOf(ed->GetFilename());
    if (ft != ftHeader && ft != ftSource)
        return false;

    // because ft == ftHeader || ftSource, the extension has at least 1 character
    bool extStartsWithCapital = wxIsupper(wxFileName(ed->GetFilename()).GetExt()[0]);

    // create a list of search dirs
    wxArrayString dirs;
    cbProject* project = 0;

    // if the file in question belongs to a different open project,
    // then use that project instead.
    // this fixes locating the file's pair in a workspace when the active file
    // does not belong to the active project.
    ProjectFile* opf = ed->GetProjectFile();
    if (opf)
        project = opf->GetParentProject();

    // if we didn't get a valid project, try the active one
    if (!project)
        project = Manager::Get()->GetProjectManager()->GetActiveProject();

    if (project)
    {
        // get project's include dirs
        dirs = project->GetIncludeDirs();

        if (opf)
        {
            wxString const &activeName = opf->file.GetName();

            // first try to find the file among the opened files
            for (int i = 0; i < GetEditorsCount(); ++i)
            {
                cbEditor* edit = GetBuiltinEditor(GetEditor(i));
                if (!edit)
                    continue;

                ProjectFile* pf = edit->GetProjectFile();
                if (!pf)
                    continue;

                if (pf->file.GetName() == activeName)
                {
                    wxFileName const & fname = pf->file;
                    FileType ft_other = FileTypeOf(fname.GetFullName());
                    if (   (    ((ft == ftHeader) && (ft_other == ftSource))
                             || ((ft == ftSource) && (ft_other == ftHeader)) )
                        && (wxIsupper(fname.GetExt()[0]) == extStartsWithCapital) )
                    {
                        if (fname.FileExists())
                        {
                            cbEditor* newEd = Open(fname.GetFullPath());
                            if (newEd!=0L) // we found and were able to open it
                                return true; // --> RETURN
                        }
                    }
                }
            }

            // second try to find in the project files - at the same time
            // build the directory list for further searching if not
            // successful now
            for (int i = 0; i < project->GetFilesCount(); ++i)
            {
                ProjectFile* pf = project->GetFile(i);
                if (!pf)
                    continue;

                wxString dir = pf->file.GetPath(wxPATH_GET_VOLUME);
                if (dirs.Index(dir) == wxNOT_FOUND)
                    dirs.Add(dir);

                if (pf->file.GetName() == activeName)
                {
                    wxFileName const & fname = pf->file;
                    FileType ft_other = FileTypeOf(fname.GetFullName());
                    if (   (    ((ft == ftHeader) && (ft_other == ftSource))
                             || ((ft == ftSource) && (ft_other == ftHeader)) )
                        && (wxIsupper(fname.GetExt()[0]) == extStartsWithCapital) )
                    {
                        if (fname.FileExists())
                        {
                            cbEditor* newEd = Open(fname.GetFullPath());
                            if (newEd!=0L) // we found and were able to open it
                                return true; // --> RETURN
                        }
                    }
                }
            }
        }
        else // no opf
        {
            // build the directory list for further searching if opf not available
            for (int i = 0; i < project->GetFilesCount(); ++i)
            {
                ProjectFile* pf = project->GetFile(i);
                if (!pf)
                    continue;

                wxString dir = pf->file.GetPath(wxPATH_GET_VOLUME);
                if (dirs.Index(dir) == wxNOT_FOUND)
                    dirs.Add(dir);
            }
        }

        // if not found, continue building the list of directories for further searching

        // get targets include dirs
        for (int i = 0; i < project->GetBuildTargetsCount(); ++i)
        {
            ProjectBuildTarget* target = project->GetBuildTarget(i);
            if (target)
            {
                for (unsigned int ti = 0; ti < target->GetIncludeDirs().GetCount(); ++ti)
                {
                    // TODO (Morten#5#): target include dirs might override project include dirs, take append/prepend option into account
                    wxString dir = target->GetIncludeDirs()[ti];
                    if (dirs.Index(dir) == wxNOT_FOUND)
                        dirs.Add(dir);
                }
            }
        }
    } // project

    wxFileName fname;
    wxFileName fn(ed->GetFilename());
    dirs.Insert(fn.GetPath(wxPATH_GET_VOLUME), 0); // add file's dir

    for (unsigned int i = 0; i < dirs.GetCount(); ++i)
    {
        ProjectManager *pm = Manager::Get()->GetProjectManager();
        if ( !pm )
            break;

        wxString dir = dirs[i]; // might contain macros -> replace them
        Manager::Get()->GetMacrosManager()->ReplaceMacros(dir);

        fname.Assign(dir + wxFileName::GetPathSeparator() + fn.GetFullName());
//        Manager::Get()->GetLogManager()->DebugLog(F(_T("Looking for '%s', dir='%s'."), fname.GetFullPath().c_str(), dir.c_str()));
        if (!fname.IsAbsolute() && project)
        {
            fname.Normalize(wxPATH_NORM_ALL & ~wxPATH_NORM_CASE, project->GetBasePath());
//            Manager::Get()->GetLogManager()->DebugLog(F(_T("Normalizing dir to '%s'."), fname.GetFullPath().c_str()));
        }

        wxString HeaderSource = pm->GetHeaderSource(fname);
        if (!HeaderSource.IsEmpty())
        {
            fname.SetFullName(HeaderSource);
//            Manager::Get()->GetLogManager()->DebugLog(F(_T("Located '%s'."), fname.GetFullPath().c_str()));
            break;
        }
    }

    if (fname.FileExists())
    {
        //Manager::Get()->GetLogManager()->DebugLog("ed=%s, pair=%s", ed->GetFilename().c_str(), pair.c_str());
        cbEditor* newEd = Open(fname.GetFullPath());
        if (newEd!=0L) // we found and were able to open it
            return true; // --> RETURN;
    }

    // We couldn't find the file, maybe it does not exist. Ask the user if we
    // should create it:
    if (cbMessageBox(_("The file seems not to exist. Do you want to create it?"),
                _("Error"), wxICON_QUESTION | wxYES_NO) == wxID_YES)
    {
        cbProject* project = Manager::Get()->GetProjectManager()->GetActiveProject();
        if (project)
            wxSetWorkingDirectory(project->GetBasePath());

        // Create a suggestion for the new file name:
        if (ft == ftHeader)
            fn.SetExt(FileFilters::CPP_EXT);
        else if (ft == ftSource)
            fn.SetExt(FileFilters::H_EXT);
        // else? Well, if the filename is not changed we could possibly
        // overwrite an existing file with our suggestion.

        cbEditor* newEd = New(fn.GetFullPath());
        if (project)
        {
            if (cbMessageBox(_("Do you want to add this new file in the active project?"),
                        _("Add file to project"),
                        wxYES_NO | wxICON_QUESTION) == wxID_YES)
            {
                wxArrayInt targets;
                if (Manager::Get()->GetProjectManager()->AddFileToProject(newEd->GetFilename(), project, targets) != 0)
                {
                    ProjectFile* pf = project->GetFileByFilename(newEd->GetFilename(), false);
                    newEd->SetProjectFile(pf);
                    Manager::Get()->GetProjectManager()->RebuildTree();
                }
            }
        }

        // verify that the open files are still in sync
        // the new file might have overwritten an existing one)
        Manager::Get()->GetEditorManager()->CheckForExternallyModifiedFiles();
    }

    return false;
}

[Zini]

The proper algorithm is the same one the Compiler Uses. Look for it the same way using the search directories from project settings.

Sorry, that is not correct. The algorithm for choosing include files (I assume that this is the one you mean) is entirely useless here. IIRC if you have more than one header file with the same name in your collection of include paths, gcc usually picks the first one.

If the proper header file exists; it MUST use the same method. Or the solution is in-correct.

No, it can't. If you have two header files with the same name, you can't use this method for swapping. And it doesn't work well for the compiler either. Therefore most people specify the name of the directory explicitly, when writing include instructions, e.g.:

#include "A/Header.h"
#include "B/Header.h"

This resolves the ambiguousity. But obviously this don't help with the swap feature (unless someone wants to imlement oBFusCATed's selection dialogue proposal, which would offer a way to explicitly specify the directory too).


Edit: Just noticed that my example a few post above was bad. Should look like this:

include
-- A
---- foo.h
-- B
---- foo.h
src
-- A
---- foo.cpp
-- B
---- foo.cpp

[stahta01]

#include "A/Header.h"
#include "B/Header.h"

The above is NOT legal for many embedded C Compilers.

But, it is a valid example of another issue to code around to solve the problem in most cases of using wxWidgets like includes.

I suggest someone makeup an dummy project with all the problem(s) and empty files except for includes and comment of the project paths in the file.

Tim S.

[Zini]

I suggest someone makeup an dummy project with all the problem(s) and empty files except for includes and comment of the project paths in the file.

Okay. These are all source layout-options I can think of.

Edit: re-uploaded the file with an additional edge-case.


[attachment deleted by admin]