Patch #2803 2009-07-30 20:48
techy
Faster header/source swapping- Download
- 2803-Faster_header.patch (17.2 KB)
Index: src/sdk/editormanager.cpp
===================================================================
--- src/sdk/editormanager.cpp (revision 5986)
+++ src/sdk/editormanager.cpp (working copy)
@@ -953,21 +953,70 @@
m_isCheckingForExternallyModifiedFiles = false;
}
+bool EditorManager::IsHeaderSource(const wxFileName& testedFileName, const wxFileName& activeFileName, FileType ftActive)
+{
+ if (testedFileName.GetName() == activeFileName.GetName())
+ {
+ FileType ftTested = FileTypeOf(testedFileName.GetFullName());
+ if ( ((ftActive == ftHeader) && (ftTested == ftSource))
+ || ((ftActive == ftSource) && (ftTested == ftHeader)) )
+ {
+ if (testedFileName.FileExists())
+ {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+wxFileName EditorManager::FindHeaderSource(const wxArrayString& fileArray, const wxFileName& activeFileName, bool& isCandidate)
+{
+ wxFileName candidateName;
+ FileType ftActive = FileTypeOf(activeFileName.GetFullName());
+ // because ftActive == ftHeader || ftSource, the extension has at least 1 character
+ bool extStartsWithCapital = wxIsupper(activeFileName.GetExt()[0]);
+
+ for (unsigned i = 0; i < fileArray.GetCount(); ++i)
+ {
+ wxFileName testedFile(fileArray[i]);
+
+ if (IsHeaderSource(testedFile, activeFileName, ftActive))
+ {
+ bool isUpper = wxIsupper(testedFile.GetExt()[0]);
+ if (isUpper == extStartsWithCapital)
+ {
+ // we found the header/source we were searching for
+ isCandidate = false;
+ return testedFile;
+ }
+ else
+ {
+ // the header/source has a different capitalization of its extension
+ // use this if nothing better is found
+ candidateName = testedFile;
+ }
+ }
+ }
+ isCandidate = true;
+ // may be invalid (empty) file name
+ return candidateName;
+}
+
bool EditorManager::SwapActiveHeaderSource()
{
cbEditor* ed = GetBuiltinEditor(GetActiveEditor());
if (!ed)
return false;
+ ProjectManager *pm = Manager::Get()->GetProjectManager();
+ if (!pm)
+ 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,
@@ -980,94 +1029,91 @@
// if we didn't get a valid project, try the active one
if (!project)
- project = Manager::Get()->GetProjectManager()->GetActiveProject();
+ project = pm->GetActiveProject();
- if (project)
+ wxFileName fn(ed->GetFilename());
+ wxFileName candidateFile;
+ bool isCandidate;
+ wxArrayString fileArray;
+
+ // find all files with the same name as the active file, but with possibly different extension
+ // search in the directory of the active file
+ wxDir::GetAllFiles(fn.GetPath(wxPATH_GET_VOLUME), &fileArray, fn.GetName() + _T(".*"), wxDIR_FILES | wxDIR_HIDDEN);
+ // try to find the header/source in the list
+ wxFileName foundFile = FindHeaderSource(fileArray, fn, isCandidate);
+
+ if (isCandidate)
{
- // get project's include dirs
- dirs = project->GetIncludeDirs();
+ candidateFile = foundFile;
+ }
+ else if (foundFile.IsOk())
+ {
+ cbEditor* newEd = Open(foundFile.GetFullPath());
+ if (newEd!=0L) // we found and were able to open it
+ return true; // --> RETURN;
+ }
- if (opf)
- {
- wxString const &activeName = opf->file.GetName();
+ // try to find the file among the opened files
- // 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;
+ // build a list of opened files
+ fileArray.Clear();
+ for (int i = 0; i < GetEditorsCount(); ++i)
+ {
+ cbEditor* edit = GetBuiltinEditor(GetEditor(i));
+ if (!edit)
+ continue;
- ProjectFile* pf = edit->GetProjectFile();
- if (!pf)
- 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 =
download for full patch...
History
Right now when swapping header/source, CodeBlocks first builds a list of directories where the header/source might be located and then tries to find it in these directories (with all possibilities of extensions). This is a rather brute-force approach which is terribly slow for projects with lots of directories (it takes like 3 seconds when pressing F11 when the full project I'm developing at work is loaded). Instead, we can first look if we find the file among the opened files (which is quite probable - it is definitely there when pressing F11 for the second time on the same pair of header/source) and then among the project files. I kept the original way of header/source swapping when this method fails (it's up to you if you keep it or decide to remove it). Actually, the directory list for the project is built while the file is being searched in the project files so no time is lost.
A nice side-effect (actually the original reason why I started looking at the code) is that with this patch the capitalization of extensions doesn't matter. For instance in our project we use .C and .H extensions and the original method just failed to find them because it was searching for .c and .h.
The capitalisation modification is no good, unfortunately. It breaks some projects on Linux, where there is a difference between a file named "file.c" and "file.C". Both can be in one directory. Could you pinpoint me to the part that ignores the case? (I would need to remove that.) Thanks!
Well, I'd really prefer to avoid the original way of swapping headers/sources. What you originally did was:
1. you find all directories where the sources are located
2. for each of these directories you take the current filename without the extension and append all possible header (source) extensions and test if this filename exists
Point 2 is the place where the case sensitiveness problem arises as you do this:
if (ft == ftHeader)
{
fname.SetExt(FileFilters::C_EXT);
if (fname.FileExists())
break;
fname.SetExt(FileFilters::CC_EXT);
if (fname.FileExists())
break;
fname.SetExt(FileFilters::CPP_EXT);
if (fname.FileExists())
break;
fname.SetExt(FileFilters::CXX_EXT);
if (fname.FileExists())
break;
fname.SetExt(FileFilters::INL_EXT);
if (fname.FileExists())
break;
FileFilters::C_EXT, ..., FileFilters::INL_EXT are all lowercase and when you test for the presence of these files when extensions .C or .H are used, the lowercase variants don't exist of course.
As I said before, this approach is a source of a serious performance problem when the sources are present in hundreds of directories. Suppose that the sources exist in 500 directories - then 500 * 5 = 2500 tests of file existence are needed and as the kernel has to be called each time during this test, this leads to a slow header/source swapping (3s on my machine, which is very annoying because this is something you do very frequently). Of course, if you fix the lower/upper case issue by trying to append .c, .C, .cpp, .CPP, ... you double the number of necessary tests so in the above example you get to 5000 tests of file existence, which is something I want to avoid.
Please look at the updated patch. There are only three extra lines compared to the first version. In addition I test here the case of the first letter of the extension of the tested file, which has to correspond to the case of the first letter of the extension of the active file. (I haven't tested this but the change is so small that I hope I haven't screwed up anything - anyway, I hope you got the idea.)
I realised a bug in this patch (that cases C:B to crash).
Notice this line:
wxString const &activeName = opf->file.GetName();
in
EditorManager::SwapActiveHeaderSource()
...
"opf" might not be available at this point. So you are accessing a NULL pointer. Could your fix this, probably?
(Sorry for late reply, I have been very busy recently.)
Do you still experience the problem? Do you have a testcase how to reproduce it? It is very strange because when looking into the code there is
if (opf)
previously, so this shouldn't happen.
Anyway, I'm sending a second revision of the patch, which, contrary to the original version that was a quick hack, changes more things in header/source swapping to:
1. make it less messy
2. make it work with extensions regardless of their capitalization even for include files of the project
3. make it find header/source even if the capitalization is not correct, but no file with correct capitalization was found.
So what swapping header/source does now:
1. searches for header/source in opened files - the extension must have the same capitalization as the active header/source
2. if not successful, searches in project files - it prefers the same capitalization of the extension as the active header/source - however, if no file with the same capitalization is found and there exists some header/source with different extension capitalization, it opens this file
3. if not successful, it builds the list containing the project include dir, the target include dir and the dir where the active file is located.
4. for each of the directories from step 3 it finds all files with the same base name (without extension) and for each of these files it tests whether it is header/source - again, prefer the same capitalization (this fixes the case where previously, files in the include dirs ending with e.g. Hpp weren't taken as headers, because the tests were done only for existence of files with hpp and HPP ext)
5. If nothing found, show the dialog about new file creation.
This patch completely removes ProjectManager::GetHeaderSource as it is unnecessary now.
OK, after looking at Patch #2841and Bug #016338, here is a revised version of the previous patch. Basically, there is one more initial step:
0. Seatch for header/source in the active file's directory
(the remaining 5 steps are as before)
Note that Patch #2841 isn't correct - it searches for header/source _only_ in the active file's directory so it wouldn't work for header/sources that are placed in separate directories.
Why did you remove
ProjectManager::GetHeaderSource(const wxFileName &fname)
from project manager again?
IMHO it's not the focus f the editor manager to locate files in a project. I did that on purpose.
Hi, I removed it because the function is not needed any more (and it wasn't completely correct because it would find only e.g. "cpp" or "CPP", but not "Cpp"). For project and target include files the header/source search works differently now if you have a closer look.
This is a very slightly modified previous version of the patch - we can try to find the file among the open files even if no project is opened - moved this part of the code before we check whether we have a project opened. Apart from that, the patch is unchanged.
Ahhh - C++ implicit conversion related bug. There was this code:
if (wxIsupper(testedFile.GetExt()[0]) == extStartsWithCapital)
{
//something
}
where extStartsWithCapital was bool. Apparently wxIsupper(testedFile.GetExt()[0]) returns int (255) so bool gets converted to int (1) and the expression is always false. (I hate the implicit conversions that C and C++ makes!) This fixes it:
bool isUpper = wxIsupper(testedFile.GetExt()[0]);
if (isUpper == extStartsWithCapital)
{
//something
}
Otherwise no change.
Good catch! ;-)
I am about to commit some of your patches, soon. Probably already today evening if I hopefully find the time. I've quite some patches pending... It's hard to have some spare time these days... :-(
Great!
One more (hopefully last) change - the dialogue saying "The file seems not to exist. Do you want to create it?" when header/source is not found is pretty annoying - most of the time I hit this is when the source file doesn't have its own header file and I have never pressed "Yes" in this dialogue. Therefore I think that "No" should be the default value, which is the only change made in this patch.