Patch #1186 2006-07-04 03:26
bjoernp
Bug #7979 -> ProjectManager::pTopEditor removed- Download
- 1186-Bug_7979_Proje.patch (2.7 KB)
Index: src/sdk/cbproject.cpp
===================================================================
--- src/sdk/cbproject.cpp (revision 2661)
+++ src/sdk/cbproject.cpp (working copy)
@@ -533,7 +533,7 @@
EditorBase* eb = Manager::Get()->GetEditorManager()->Open(f->file.GetFullPath());
if(eb)
{
- Manager::Get()->GetProjectManager()->SetTopEditor(eb);
+//! Manager::Get()->GetProjectManager()->SetTopEditor(eb);
eb->Activate();
}
}
Index: src/sdk/projectmanager.cpp
===================================================================
--- src/sdk/projectmanager.cpp (revision 2661)
+++ src/sdk/projectmanager.cpp (working copy)
@@ -195,7 +195,7 @@
ProjectManager::ProjectManager()
: m_pTree(0),
m_pWorkspace(0),
- m_pTopEditor(0),
+//! m_pTopEditor(0),
m_TreeCategorize(false),
m_TreeUseFolders(true),
m_TreeFreezeCounter(0),
@@ -960,7 +960,7 @@
}
return m_pWorkspace;
}
-
+/*!
void ProjectManager::SetTopEditor(EditorBase* ed)
{
m_pTopEditor = ed;
@@ -970,10 +970,10 @@
{
return m_pTopEditor;
}
-
+*/
bool ProjectManager::LoadWorkspace(const wxString& filename)
{
- m_pTopEditor = 0;
+ //! m_pTopEditor = 0;
if (!CloseWorkspace())
return false; // didn't close
m_IsLoadingWorkspace=true;
@@ -992,8 +992,8 @@
m_IsLoadingWorkspace=false;
Manager::Get()->GetEditorManager()->RebuildOpenedFilesTree();
m_pTree->SetItemText(m_TreeRoot, m_pWorkspace->GetTitle());
- if(m_pTopEditor)
- m_pTopEditor->Activate();
+ // if(m_pTopEditor)
+ // m_pTopEditor->Activate();
Manager::Get()->GetEditorManager()->RefreshOpenedFilesTree(true);
UnfreezeTree(true);
}
Index: src/sdk/projectmanager.h
===================================================================
--- src/sdk/projectmanager.h (revision 2661)
+++ src/sdk/projectmanager.h (working copy)
@@ -346,9 +346,9 @@
*/
wxMenu* GetProjectMenu();
/** Sets the Top Editor (the active editor from the last session) */
- void SetTopEditor(EditorBase* ed);
+//! void SetTopEditor(EditorBase* ed);
/** @return The Top Editor */
- EditorBase* GetTopEditor() const;
+//! EditorBase* GetTopEditor() const;
/** @return The workspace icon index in the image list. */
int WorkspaceIconIndex();
@@ -399,7 +399,7 @@
ProjectsArray* m_pProjects;
DepsMap m_ProjectDeps;
cbWorkspace* m_pWorkspace;
- EditorBase* m_pTopEditor;
+//! EditorBase* m_pTopEditor;
bool m_TreeCategorize;
bool m_TreeUseFolders;
FilesGroupsAndMasks* m_pFileGroups;
History
This bug crashed into sdk/projectmanager.cpp +996
The patch removes ProjectManager::pTopEditor and references to it.
ProjectManager::SetTopEditor(...)
ProjectManager::GetTopEditor()
Changes/breaks sdk-interface => forces lots of recompilation!
But it seemed the only sensible way to fix this bug, because IMO no references to Editor-Windows should be held anywhere outside of EditorManager, if I got that pattern right.
There were no dependent references to this interface anyway.
Awaiting your comments!
Cheers,
Björn
I believe there is an easier and cleaner solution:
cbEditor::~cbEditor()
{
/* ... */
ProjectManager *pm = Manager::Get()->GetProjectManager();
if(pm)
pm->SetTopEditor(0);
delete m_pData;
}
Please try revision 2671. On my machine, this fixes the problem.
Yes, this looks easier, but I am not so sure that it is also cleaner. I have a few arguments against this solution:
Now the cbEditor needs to know about a ProjectManager that it is some part of. Doesn't feel right to me: It shouldn't need to know that.
pTopEditor points to EditorBase, not to the derived cbEditor. Given a future alternative implementation of this interface the same problem would rise again.
pTopEditor and its Set/Get-Methods have a very questionable reason for being there, but I am not into it enough to really judge this.
As I understand the EditorManager and all the other Manager-Gizmos around, they are there telling us: "Hey, we take care about this sh** and you need not be concerned about our business. If you need to know about the Editors around, ask the EditorManager, gotcha?"
If that is so, then holding a private reference to some instance of an Editor anywhere breaks this design.
In your solution pTopEditor will be zeroed in case _any_ of the open editors is destroyed. On the other hand it is only set once int cbProject::LoadLayout().
Then Activate() is already called there on the last open Editor-Panel, the call that crashes on the meanwhile invalidated pointer in ProjectManager - fatal and redundant. And right this Activate()-Call is the only use of pTopEditor within the ProjectManager.
So my favourite easy and simple solution is just dropping the line that causes the crash. It's not needed anyway (well, so it seems).
The whole rest of my arguments could be postponed then.
What about this?
Greetz,
Björn
>Now the cbEditor needs to know about a ProjectManager...
>pTopEditor points to EditorBase
The former is the most normal thing, happens everywhere throughout the code, the latter is technically true, but no problem, putting the same instruction into EditorBase instead is a fiffy (should have done that anyway).
However, you are certainly right insofar as the top editor does no longer serve any real purpose. It is a relict none of us remembers what it was once good for, so it can as well be entirely removed. :)
Revision 2686
All right, then I will update the patch to the latest revision and remove that thing.
I think the info on "What was the last editor/file open on top with this project" could go into cbProject, if anybody has some use for it.
By the way I feel a bit silly throwing such big arguments at you for such a rather small issue.
Well,. it's my first dig on this project, so I am just slightly over-motivated. ;-) Hope you don't mind.
rev 2689
Aahhh!
m_pTopEditor is gone!
You did it?
Fine then... :)