Bug 396418 - Command org.eclipse.ui.file.save always disabled

Update Save and SaveAll handlers to use the current workbench saveable
story.

Replace ActionFactory actions with WorkbenchCommandActions

Fix EvalutionService so requestEvalution(*) takes into account variables
as well as properties.

Change-Id: I2b8a94c9b4744d1327fca87578b6957b7f600f66

5 files changed, 60 insertions, 12 deletions

diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/actions/ActionFactory.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/actions/ActionFactory.java
index 46bb06396b6..b4774e9f5e3 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/actions/ActionFactory.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/actions/ActionFactory.java
@@ -23,9 +23,6 @@ import org.eclipse.ui.internal.IWorkbenchHelpContextIds;
 import org.eclipse.ui.internal.IntroAction;
 import org.eclipse.ui.internal.NavigationHistoryAction;
 import org.eclipse.ui.internal.OpenPreferencesAction;
-import org.eclipse.ui.internal.SaveAction;
-import org.eclipse.ui.internal.SaveAllAction;
-import org.eclipse.ui.internal.SaveAsAction;
 import org.eclipse.ui.internal.ToggleEditorsVisibilityAction;
 import org.eclipse.ui.internal.WorkbenchImages;
 import org.eclipse.ui.internal.WorkbenchMessages;
@@ -1283,8 +1280,12 @@ public abstract class ActionFactory {
             if (window == null) {
                 throw new IllegalArgumentException();
             }
-            IWorkbenchAction action = new SaveAction(window);
+			WorkbenchCommandAction action = new WorkbenchCommandAction(getCommandId(), window);
+			action.setText(WorkbenchMessages.SaveAction_text);
+			action.setToolTipText(WorkbenchMessages.SaveAction_toolTip);
             action.setId(getId());
+			window.getWorkbench().getHelpSystem()
+					.setHelp(action, IWorkbenchHelpContextIds.SAVE_ACTION);
             return action;
         }
     };
@@ -1303,7 +1304,11 @@ public abstract class ActionFactory {
             if (window == null) {
                 throw new IllegalArgumentException();
             }
-            IWorkbenchAction action = new SaveAllAction(window);
+			IWorkbenchAction action = new WorkbenchCommandAction(getCommandId(), window);
+			action.setText(WorkbenchMessages.SaveAll_text);
+			action.setToolTipText(WorkbenchMessages.SaveAll_toolTip);
+			window.getWorkbench().getHelpSystem()
+					.setHelp(action, IWorkbenchHelpContextIds.SAVE_ALL_ACTION);
             action.setId(getId());
             return action;
         }
@@ -1323,7 +1328,11 @@ public abstract class ActionFactory {
             if (window == null) {
                 throw new IllegalArgumentException();
             }
-            IWorkbenchAction action = new SaveAsAction(window);
+			IWorkbenchAction action = new WorkbenchCommandAction(getCommandId(), window);
+			action.setText(WorkbenchMessages.SaveAs_text);
+			action.setToolTipText(WorkbenchMessages.SaveAs_toolTip);
+			window.getWorkbench().getHelpSystem()
+					.setHelp(action, IWorkbenchHelpContextIds.SAVE_AS_ACTION);
             action.setId(getId());
             return action;
         }
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/AbstractSaveHandler.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/AbstractSaveHandler.java
index 61d3e507686..956f9d5c139 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/AbstractSaveHandler.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/AbstractSaveHandler.java
@@ -66,6 +66,10 @@ public abstract class AbstractSaveHandler extends AbstractEvaluationHandler {
 		if (activePart instanceof ISaveablePart)
 			return (ISaveablePart) activePart;
 
+		ISaveablePart part = (ISaveablePart) Util.getAdapter(activePart, ISaveablePart.class);
+		if (part != null)
+			return part;
+
 		return InternalHandlerUtil.getActiveEditor(context);
 	}
 	
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationReference.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationReference.java
index b9b4838b47f..9f3b68cc0c1 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationReference.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationReference.java
@@ -15,6 +15,8 @@ import org.eclipse.core.expressions.EvaluationResult;
 import org.eclipse.core.expressions.Expression;
 import org.eclipse.core.expressions.IEvaluationContext;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.ISafeRunnable;
+import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.contexts.RunAndTrack;
 import org.eclipse.e4.ui.internal.workbench.Activator;
@@ -38,6 +40,7 @@ public class EvaluationReference extends RunAndTrack implements IEvaluationRefer
 	boolean participating = true;
 	boolean postingChanges = true;
 	boolean hasRun = false;
+	Runner runner = new Runner();
 
 	public EvaluationReference(IEclipseContext context, Expression expression,
 			IPropertyChangeListener listener, String property) {
@@ -78,6 +81,26 @@ public class EvaluationReference extends RunAndTrack implements IEvaluationRefer
 		return sourcePriority;
 	}
 
+	class Runner implements ISafeRunnable {
+		public IEvaluationContext localContext;
+
+		public void run() throws Exception {
+			try {
+				cache = expression.evaluate(localContext) != EvaluationResult.FALSE;
+			} catch (CoreException e) {
+				Activator.trace(Policy.DEBUG_CMDS, "Failed to calculate active", e); //$NON-NLS-1$
+			}
+		}
+
+		public void handleException(Throwable exception) {
+			if (exception instanceof Error) {
+				// errors are deadly, we shouldn't ignore these
+				throw (Error) exception;
+			}
+			Activator.trace(Policy.DEBUG_CMDS, "Failed with throwable: " + expression, exception); //$NON-NLS-1$
+		}
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * 
@@ -85,15 +108,13 @@ public class EvaluationReference extends RunAndTrack implements IEvaluationRefer
 	 * org.eclipse.ui.internal.services.IEvaluationResultCache#evaluate(org.
 	 * eclipse.core.expressions.IEvaluationContext)
 	 */
-	public boolean evaluate(IEvaluationContext context) {
+	public boolean evaluate(final IEvaluationContext context) {
 		if (expression == null) {
 			cache = true;
 		} else {
-			try {
-				cache = expression.evaluate(context) != EvaluationResult.FALSE;
-			} catch (CoreException e) {
-				Activator.trace(Policy.DEBUG_CMDS, "Failed to calculate active", e); //$NON-NLS-1$
-			}
+			runner.localContext = context;
+			SafeRunner.run(runner);
+
 		}
 		return cache;
 	}
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationService.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationService.java
index c225ace39aa..98260e4690e 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationService.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/services/EvaluationService.java
@@ -319,14 +319,26 @@ public final class EvaluationService implements IEvaluationService {
 		for (EvaluationReference ref : refs) {
 			Expression expr = ref.getExpression();
 			if (expr != null) {
+				boolean evaluated = false;
 				ExpressionInfo info = expr.computeExpressionInfo();
 				String[] names = info.getAccessedPropertyNames();
 				for (String name : names) {
 					if (propertyName.equals(name)) {
+						evaluated = true;
 						ref.evaluate();
 						break;
 					}
 				}
+				if (!evaluated) {
+					names = info.getAccessedVariableNames();
+					for (String name : names) {
+						if (propertyName.equals(name)) {
+							evaluated = true;
+							ref.evaluate();
+							break;
+						}
+					}
+				}
 			}
 		}
 		endSourceChange(sourceNames);
diff --git a/bundles/org.eclipse.ui/plugin.xml b/bundles/org.eclipse.ui/plugin.xml
index 1e3dfe75b1d..b8c61ba5f00 100644
--- a/bundles/org.eclipse.ui/plugin.xml
+++ b/bundles/org.eclipse.ui/plugin.xml
@@ -842,6 +842,7 @@
       <command
             name="%command.save.name"
             description="%command.save.description"
+            defaultHandler="org.eclipse.ui.internal.handlers.SaveHandler"
             categoryId="org.eclipse.ui.category.file"
             id="org.eclipse.ui.file.save" />
       <command
@@ -853,6 +854,7 @@
       <command
             name="%command.saveAll.name"
             description="%command.saveAll.description"
+            defaultHandler="org.eclipse.ui.internal.handlers.SaveAllHandler"
             categoryId="org.eclipse.ui.category.file"
             id="org.eclipse.ui.file.saveAll" />
       <command