Tab Save/SaveAll fix maybe??

[Pecan]

I've never sumitted a fix before, nor have I ever modified
the core code. But I wanted to try and fix the notebook tab Save/SaveAll
entries.

So... Is the following acceptable. If not, please comment.
If so, I'll submit it to sf

thanks
pecan

Code

C:\Usr\Proj\cbBeta\trunk\src\sdk>PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;c:\usr\bin;c:\usr\bin\subversion\bin 

C:\Usr\Proj\cbBeta\trunk\src\sdk>SET APR_ICONV_PATH=C:\usr\bin\Subversion\iconv 

C:\Usr\Proj\cbBeta\trunk\src\sdk>svn diff 
Index: editormanager.cpp
===================================================================
--- editormanager.cpp	(revision 1563)
+++ editormanager.cpp	(working copy)
@@ -227,7 +227,20 @@
             pop->AppendSeparator();
             pop->Append(idNBTabSave, _("Save"));
             if (GetPageCount() > 1)
-                pop->Append(idNBTabSaveAll, _("Save all"));
+            {    pop->Append(idNBTabSaveAll, _("Save all"));
+                // --begin pecan 12/19/2005 1:40 PM-------------------------
+                EditorBase* ed = Manager::Get()->GetEditorManager()->GetEditor(m_RightClickSelected);
+                int editorsModified = 0;
+                for (int i = 0; i < Manager::Get()->GetEditorManager()->GetEditorsCount(); ++i)
+                {
+                    EditorBase* other = Manager::Get()->GetEditorManager()->GetEditor(i);
+                    if (other == ed) continue;
+                    if (other && other->GetModified() ) editorsModified++;
+                }
+                pop->Enable(idNBTabSaveAll, editorsModified>0 );
+                //  --end pecan 12/19/2005 1:40 PM ---------------------------
+            }//if(getPage....
+
             EditorBase* ed = Manager::Get()->GetEditorManager()->GetEditor(m_RightClickSelected);
             if (ed)
                 pop->Enable(idNBTabSave, ed->GetModified());
@@ -269,6 +282,8 @@
     EVT_MENU(idNBTabClose, EditorNotebook::OnClose)
     EVT_MENU(idNBTabCloseAll, EditorNotebook::OnCloseAll)
     EVT_MENU(idNBTabCloseAllOthers, EditorNotebook::OnCloseAllOthers)
+    EVT_MENU(idNBTabSave, EditorNotebook::OnSave)           //pecan 12/19/2005 1:11 PM
+    EVT_MENU(idNBTabSaveAll, EditorNotebook::OnSaveAll)     //pecan 12/19/2005 1:11 PM
     EVT_MIDDLE_DOWN(EditorNotebook::OnMiddleDown)
     EVT_RIGHT_DOWN(EditorNotebook::OnRightDown)
 END_EVENT_TABLE() 

[280Z28]

What was the bug? (link?)

Edit: You are making it only enable "SaveAll" when there is at least one modified file open, correct?

[thomas]

Looks ok (though the calls to Manager are not needed, and I'd have used a bool).
Have a bit of patience please, we're lagging a bit behind with applying non-vital patches.

[Pecan]

What was the bug? (link?)

Edit: You are making it only enable "SaveAll" when there is at least one modified file open, correct?

There was no bug report on SF. I was going to do that if what I
modified was proper.

It enables "save" when the focused editor has been modified.
It enables "saveAll" when any other (not focused) file is modified.

thanks
pecan

[Pecan]

Looks ok (though the calls to Manager are not needed, and I'd have used a bool).
Have a bit of patience please, we're lagging a bit behind with applying non-vital patches.

OK, I'll re-work it. Could you give me a one-liner example of
NOT using the Manager here.

I figure, if I can get this right, I can get others right.

EDIT: 12/20/2005 9:59 AM
Ohh, I get it. Since the code is in editorManager I dont need
"Manager::Get()->GetEditorManager()->"

thanks
pecan

[thomas]

I'll do the modifications quickly... need a break from the other stuff anyway...

[thomas]

...and I was wrong. You do need a call to Manager::Get() because we're in an embedded class there, had not seen that, sorry

Here's what it looks like now:

Code

pop->AppendSeparator();
pop->Append(idNBTabSave, _("Save"));
pop->Append(idNBTabSaveAll, _("Save all"));

EditorManager *em = Manager::Get()->GetEditorManager();
unsigned int num_modified = 0;

for (int i = 0; i < em->GetEditorsCount(); ++i)
{
    EditorBase* ed = em->GetEditor(i);
    if (ed && ed->GetModified() )
    ++num_modified;
}
pop->Enable(idNBTabSave, num_modified);
pop->Enable(idNBTabSaveAll, num_modified > 1 );

It always adds the menu items for consistency instead of only showing "all" on multiple tabs.

[Pecan]

thanks thomas

Nice lesson. Beautiful efficiency.

thanks
pecan

[Pecan]

code from HEAD 1573 sdk/ediitormanager.cpp (12/20/2005 12:35 PM)

Code

			for (int i = 0; i < em->GetEditorsCount(); ++i)
			{
				EditorBase* ed = em->GetEditor(i);
				if (ed && ed->GetModified() )
					++num_modified;
			}
			pop->Enable(idNBTabSave, num_modified);
			pop->Enable(idNBTabSaveAll, num_modified > 1 );

Put two tabs/editors up. Modify the left one.
If the left tab has been modified and the right one NOT,
then cliick on the right tab as save. It *doesn't* save. AND saveAll is disabled
because test is for "num_modified>1" and the cursor is on the
unmodified editor tab.

In my not so professional code in message 1 above, I ran across the same
problem. Thats why I tested to differentiate from "this editor" and
"other editors":

Code

     EditorBase* other = Manager::Get()->GetEditorManager()->GetEditor(i);
     if (other == ed) continue;
     if (other && other->GetModified() ) editorsModified++;

thanks
pecan

[thomas]

Hmm... works perfectly here.

I have added debug output just to be 100% sure, it counts correctly, too.
For me, it does save, and it displays all entries correctly, too. Tried with 1, 2, 3, 5, and 8 tabs, having 0, 1, 2, 3, or 4 modified.

[Pecan]

I've test the follow mods. They seem to work the way I believe
you want them to.

Code

===================================================================
--- sdk/editormanager.cpp	(revision 1573)
+++ sdk/editormanager.cpp	(working copy)
@@ -237,8 +237,10 @@
 				if (ed && ed->GetModified() )
 					++num_modified;
 			}
-			pop->Enable(idNBTabSave, num_modified);
-			pop->Enable(idNBTabSaveAll, num_modified > 1 );
+			//pop->Enable(idNBTabSave, num_modified);
+			pop->Enable(idNBTabSave, em->GetEditor(m_RightClickSelected)->GetModified()); //pecan 12/20/2005 12:42 PM
+			//-pop->Enable(idNBTabSaveAll, num_modified > 1 );
+			pop->Enable(idNBTabSaveAll, num_modified > 0 );//pecan 12/20/2005 12:42 PM
 
             PopupMenu(pop, event.GetPosition().x, event.GetPosition().y);
             delete pop; 

thanks
pecan

[Pecan]

@thomas

Ok, I tried again. I cannot get it to save when two tabs are up.
The left modified, the right not.

Right click on the right (unmodified) tab. "Save" is enabled (and shouldn't be)
and "Save all" is disabled because "modified>1" is false.

What am I missing??  I can't right click on an unmodified tab and save it.
I should "save all" which is disabled.

Am I nuts??


thanks
pecan

[thomas]

Cannot reproduce your problem, but I have found another small bug... you can save a document that is actually not modified if one of its siblings is modified. But that's luckily easily fixed

[Urxae]

Wait, is "Save all" only enabled if more than one editor is modified? So if only one is modified switching to it and hitting "Save" is the only way to save it without modifying anything else first?
That doesn't seem right. "Save all" should be enabled if at least one editor is modified (i.e. if there's anything to save), so it can also be activated from other editors. I expect "Save all" to actually save all my modifications, however many (but > 0) files are modified and whichever editor is currently active...
To be absolutely clear: "Save all" should only be disabled if there are no modifications to save.

...

Wow, I might have gone a little overboard with all the italics .

[thomas]

Allthough mathematically 1 ∈ all, it really only makes sense to enable "all" if n > 1. Else, we don't need "save".