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".

[Urxae]

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

Imagine the following situation: You have a lot of editors open. One of them is modified (or maybe you're not sure if any are modified). Unfortunately, it's not the current one, and the modified one is not visible in the row of tabs at the top of the screen. You now either have to scroll the tab list¹ or use the open files list² to get to the tab and hit "Save".
Wouldn't it be easier to just hit Ctrl-Shift-S to "Save all"? Wouldn't it make sense for that option to be enabled then?
Besides, I've never seen a multi-document interface that didn't allow you to "Save all" if there's only one document modified. (I've seen some take the easy route and never disable "Save all", though)

Of course, I might not even notice, since the only time I ever need to save anything is right before a build, when it happens automatically .

1) And possibly guess which way to scroll if both directions are possible.
2) Which is rather small, at least for me. The project view is usually more important to me.

[Pecan]

Note that the left hand editor tab has been modified,  but neither
save nor save all is enabled, because I right clicked on an unmodified
editor/tab.

This is because modified>1 is false

thanks
pecan


[attachment deleted by admin]

[thomas]

Imagine the following situation: You have a lot of editors open. One of them is modified (or maybe you're not sure if any are modified). Unfortunately, it's not the current one, and the modified one is not visible

Imagine you have two or three modified... you don't want to do this. It is russian roulette.

[Urxae]

Imagine the following situation: You have a lot of editors open. One of them is modified (or maybe you're not sure if any are modified). Unfortunately, it's not the current one, and the modified one is not visible

Imagine you have two or three modified... you don't want to do this. It is russian roulette.

Imagine you actually want to (*gasp*) save all your changes, but the only way to make sure there isn't a lone unmodified tab is to scroll through and look at all the tabs...

[thomas]

What's the problem? If you have two or more modified documents and you really want to save all, you can just use "save all".

But you should know that "all" means "more than one". If "one" and "all" is the same, you can save several documents when in fact you don't want to do that.

[Pecan]

Maybe we should take this code out of core and go back to the
TabSave plugin with a lot of config options that could please everyone.
People could define their own save/saveall behavior

thanks
pecan

[rickg22]

Thomas: Allow me to express an opinion on Microsoft products.

DO *NOT* EVER take decisions for the users! It's acceptable to provide some guidance for joe-users, but hindering power users is a big no-no. If the user wants to save all, let him. If he screws up, it's his fault, not ours.

We don't want to walk in the big M's footsteps, do we?

As for me, I agree with Urxae.

[Pecan]

I too would rather have "save" enabled if the editor/tab
clicked on has been modified.

I would rather have "saveall" enabled if *any* file
has been modified.

Having them enabled like this also raises a flag when
the tab is out of eye range.

Edit: also, having saveAll disabled when only "this"
file has been modified shows that I dont have to worry
about other (unseen) tabs.

thanks
pecan

[thomas]

I don't use the tab menu anyway, and if you absolutely wish to shoot your foot... I care not.
But this was the last time I modify editormanager today... :lol:

[rickg22]

lol Anyway, thanks. Tonight i'm compiling C::B and copying it to my flash drive so i can keep working while i'm on vacations Yay!