Add Basic Support for Named Command Handlers

N1k145 · N1k145 · commit d78d16423bcc · 2024-01-30T11:36:42.000+01:00
Fixes Regression: eclipse-platform#1314 With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
diff --git a/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF b/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.commands
-Bundle-Version: 3.11.300.qualifier
+Bundle-Version: 3.12.0.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.core.commands,
diff --git a/bundles/org.eclipse.core.commands/src/org/eclipse/core/commands/IHandler.java b/bundles/org.eclipse.core.commands/src/org/eclipse/core/commands/IHandler.java
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Niklas Kellner  - Named Handler API
  *******************************************************************************/
 package org.eclipse.core.commands;
 
@@ -90,4 +91,14 @@ public interface IHandler {
 	 *            performed.
 	 */
 	void removeHandlerListener(IHandlerListener handlerListener);
+
+	/**
+	 * Called by the framework to allow the handler to specify a name.
+	 *
+	 * @return name of the Handler
+	 * @since 3.12
+	 */
+	default String getHandlerLabel() {
+		return null;
+	}
 }
diff --git a/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java b/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java
@@ -44,7 +44,7 @@ public class HandlerServiceHandler extends AbstractHandlerWithState {
 
 	protected final String commandId;
 	// Remove state from currentStateHandler when it goes out of scope
-	private WeakReference<IObjectWithState> currentStateHandler = new WeakReference<IObjectWithState>(null);
+	protected WeakReference<IObjectWithState> currentStateHandler = new WeakReference<IObjectWithState>(null);
 
 	public HandlerServiceHandler(String commandId) {
 		this.commandId = commandId;
@@ -265,6 +265,8 @@ private void switchHandler(Object handler) {
 				typed.addState(id, getState(id));
 			}
 		}
+
+		fireHandlerChanged(new HandlerEvent(this, false, false));
 	}
 
 }
diff --git a/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/HandledContributionItem.java b/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/HandledContributionItem.java
@@ -24,7 +24,10 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import org.eclipse.core.commands.Command;
 import org.eclipse.core.commands.ExecutionException;
+import org.eclipse.core.commands.IHandler;
 import org.eclipse.core.commands.IStateListener;
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.core.commands.State;
@@ -54,6 +57,7 @@
 import org.eclipse.jface.bindings.TriggerSequence;
 import org.eclipse.jface.menus.IMenuStateIds;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.MenuItem;
 import org.eclipse.swt.widgets.ToolItem;
@@ -120,11 +124,10 @@ public void setModel(MItem item) {
 	}
 
 	/**
-	 * This method seems to be necessary for calls via reflection when called
-	 * with MHandledItem parameter.
+	 * This method seems to be necessary for calls via reflection when called with
+	 * MHandledItem parameter.
 	 *
-	 * @param item
-	 *            The model item
+	 * @param item The model item
 	 */
 	public void setModel(MHandledItem item) {
 		setModel((MItem) item);
@@ -148,10 +151,8 @@ private void generateCommand() {
 				WorkbenchSWTActivator.trace(Policy.DEBUG_MENUS_FLAG, "command: " + parmCmd, null); //$NON-NLS-1$
 			}
 			if (parmCmd == null) {
-				logger.error(
-						"Unable to generate the parameterized " + "command with the id \"" + cmdId + "\" with the " //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-								+ parameters
-								+ " parameter(s). Model details: " + getModel());//$NON-NLS-1$
+				logger.error("Unable to generate the parameterized " + "command with the id \"" + cmdId + "\" with the " //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+						+ parameters + " parameter(s). Model details: " + getModel());//$NON-NLS-1$
 				return;
 			}
 
@@ -169,6 +170,8 @@ private void generateCommand() {
 			} else if (radioState != null) {
 				radioState.addListener(stateListener);
 			}
+
+			parmCmd.getCommand().addCommandListener(event -> Display.getDefault().asyncExec(() -> update()));
 		}
 	}
 
@@ -264,8 +267,7 @@ protected void updateMenuItem() {
 				if (mnemonics != null && !mnemonics.isEmpty()) {
 					int idx = text.indexOf(mnemonics);
 					if (idx != -1) {
-						text = text.substring(0, idx) + '&'
-								+ text.substring(idx);
+						text = text.substring(0, idx) + '&' + text.substring(idx);
 					}
 				}
 			}
@@ -307,6 +309,7 @@ protected void updateToolItem() {
 		item.setToolTipText(tooltip);
 		item.setSelection(getModel().isSelected());
 		item.setEnabled(getModel().isEnabled());
+
 	}
 
 	private String getToolTipText(boolean attachKeybinding) {
@@ -317,6 +320,8 @@ private String getToolTipText(boolean attachKeybinding) {
 			parmCmd = getModel().getWbCommand();
 		}
 
+		text = legacyActionLabelSupport(text, parmCmd);
+
 		if (parmCmd != null && text == null) {
 			try {
 				text = parmCmd.getName();
@@ -332,6 +337,12 @@ private String getToolTipText(boolean attachKeybinding) {
 		return text;
 	}
 
+	private String legacyActionLabelSupport(String text, ParameterizedCommand command) {
+
+		return java.util.Optional.of(command).map(ParameterizedCommand::getCommand).map(Command::getHandler)
+				.map(IHandler::getHandlerLabel).filter(Objects::nonNull).orElse(text);
+	}
+
 	@Override
 	protected void handleWidgetDispose(Event event) {
 		if (event.widget == widget) {
@@ -389,7 +400,7 @@ public void dispose() {
 
 	@Override
 	protected void handleHelpRequest() {
-		if(helpService==null)
+		if (helpService == null)
 			return;
 		String helpContextId = getModel().getPersistedState().get(EHelpService.HELP_CONTEXT_ID);
 		if (helpContextId != null) {
diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/commands/ActionHandler.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/commands/ActionHandler.java
@@ -14,12 +14,15 @@
  *******************************************************************************/
 package org.eclipse.jface.commands;
 
+import java.util.Optional;
+
 import org.eclipse.core.commands.AbstractHandler;
 import org.eclipse.core.commands.ExecutionEvent;
 import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.commands.HandlerEvent;
 import org.eclipse.core.commands.IHandlerListener;
 import org.eclipse.jface.action.IAction;
+import org.eclipse.jface.action.LegacyActionTools;
 import org.eclipse.jface.util.IPropertyChangeListener;
 import org.eclipse.swt.widgets.Event;
 
@@ -168,4 +171,9 @@ public final String toString() {
 
 		return buffer.toString();
 	}
+
+	@Override
+	public String getHandlerLabel() {
+		return Optional.of(action).map(IAction::getText).map(LegacyActionTools::removeAcceleratorText).orElse(null);
+	}
 }
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchHandlerServiceHandler.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchHandlerServiceHandler.java
@@ -15,6 +15,8 @@
 package org.eclipse.ui.internal;
 
 import java.util.Map;
+import org.eclipse.core.commands.IHandler;
+import org.eclipse.core.commands.IObjectWithState;
 import org.eclipse.e4.core.commands.internal.HandlerServiceHandler;
 import org.eclipse.e4.core.commands.internal.HandlerServiceImpl;
 import org.eclipse.e4.core.contexts.IEclipseContext;
@@ -42,4 +44,15 @@ public void updateElement(UIElement element, Map parameters) {
 		}
 	}
 
+	@Override
+	public String getHandlerName() {
+
+		IObjectWithState handler = currentStateHandler.get();
+
+		if (handler instanceof IHandler namedHandler) {
+			return namedHandler.getHandlerLabel();
+		}
+
+		return null;
+	}
 }
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/E4HandlerProxy.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/handlers/E4HandlerProxy.java
@@ -226,4 +226,9 @@ public void removeState(String stateId) {
 		}
 	}
 
+	@Override
+	public String getHandlerLabel() {
+		return handler.getHandlerLabel();
+	}
+
 }
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/menus/CommandContributionItem.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/menus/CommandContributionItem.java
@@ -15,6 +15,8 @@
 package org.eclipse.ui.menus;
 
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 import org.eclipse.core.commands.Command;
 import org.eclipse.core.commands.CommandEvent;
 import org.eclipse.core.commands.ExecutionException;
@@ -494,6 +496,9 @@ private void updateMenuItem() {
 		MenuItem item = (MenuItem) widget;
 
 		String text = label;
+
+		text = legacyActionLabelSupport(text);
+
 		if (text == null) {
 			if (command != null) {
 				try {
@@ -535,11 +540,20 @@ private void updateMenuItem() {
 		}
 	}
 
+	private String legacyActionLabelSupport(String text) {
+		return Optional.of(command).map(ParameterizedCommand::getCommand).map(Command::getHandler)
+				.map(IHandler::getHandlerLabel).filter(Objects::nonNull)
+				.orElse(text);
+	}
+
 	private void updateToolItem() {
 		ToolItem item = (ToolItem) widget;
 
 		String text = label;
-		String tooltip = label;
+
+		text = legacyActionLabelSupport(text);
+
+		String tooltip = text;
 
 		if (text == null) {
 			if (command != null) {
@@ -579,6 +593,8 @@ private void updateButton() {
 		Button item = (Button) widget;
 
 		String text = label;
+		text = legacyActionLabelSupport(text);
+
 		if (text == null) {
 			if (command != null) {
 				try {

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ public class HandlerServiceHandler extends AbstractHandlerWithState {`
`44`	`44`
`45`	`45`	`protected final String commandId;`
`46`	`46`	`// Remove state from currentStateHandler when it goes out of scope`
`47`		`- private WeakReference<IObjectWithState> currentStateHandler = new WeakReference<IObjectWithState>(null);`
	`47`	`+ protected WeakReference<IObjectWithState> currentStateHandler = new WeakReference<IObjectWithState>(null);`
`48`	`48`
`49`	`49`	`public HandlerServiceHandler(String commandId) {`
`50`	`50`	`this.commandId = commandId;`
`@@ -265,6 +265,8 @@ private void switchHandler(Object handler) {`
`265`	`265`	`typed.addState(id, getState(id));`
`266`	`266`	`}`
`267`	`267`	`}`
	`268`	`+`
	`269`	`+ fireHandlerChanged(new HandlerEvent(this, false, false));`
`268`	`270`	`}`
`269`	`271`
`270`	`272`	`}`
Original file line number	Diff line number	Diff line change
`@@ -226,4 +226,9 @@ public void removeState(String stateId) {`
`226`	`226`	`}`
`227`	`227`	`}`
`228`	`228`
	`229`	`+ @Override`
	`230`	`+ public String getHandlerLabel() {`
	`231`	`+ return handler.getHandlerLabel();`
	`232`	`+ }`
	`233`	`+`
`229`	`234`	`}`