[WIP] Add activity support for keybindings contributions

Currently Eclipse processes key bindings from "disabled" activities,
activity support for key bindings was broken during e4 transition in
2010.

This is a draft change that re-implements activity support for
keybindings contributions.

- Don't show filtered key bindings in Keys preference page
- Hide bindings for filtered keys to BindingTableManager
- Fail command execution for filtered activity

TODOs:

- check if that is the way to go
- check if KeyBindingDispatcher change is in the correct place
- check whether one should cache BindingTable.isActive(Binding) results
(note: this would probably need an activity listener in e4 layer).

Author: iloveeclipse

bundles/org.eclipse.e4.ui.bindings/META-INF/MANIFEST.MF

@@ -20,7 +20,9 @@ Require-Bundle: org.eclipse.swt;bundle-version="[3.6.0,4.0.0)",
  org.eclipse.core.commands;bundle-version="[3.5.0,4.0.0)",
  org.eclipse.e4.core.contexts;bundle-version="1.0.0",
  org.eclipse.e4.core.di;bundle-version="1.1.0",
- org.eclipse.e4.ui.services;bundle-version="1.0.0"
+ org.eclipse.e4.ui.services;bundle-version="1.0.0",
+ org.eclipse.e4.core.services;bundle-version="2.5.100",
+ org.eclipse.e4.ui.model.workbench
 Export-Package: org.eclipse.e4.ui.bindings;
   x-friends:="org.eclipse.e4.ui.workbench,
    org.eclipse.e4.ui.workbench.renderers.swt,

bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTable.java

@@ -20,9 +20,12 @@
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.core.commands.contexts.Context;
+import org.eclipse.e4.core.services.contributions.IContributionFactory;
+import org.eclipse.e4.ui.model.application.MApplication;
 import org.eclipse.jface.bindings.Binding;
 import org.eclipse.jface.bindings.Trigger;
 import org.eclipse.jface.bindings.TriggerSequence;
@@ -131,8 +134,13 @@ private final int countStrokes(final Trigger[] triggers) {
 	private Map<TriggerSequence, ArrayList<Binding>> conflicts = new HashMap<>();
 	private Map<TriggerSequence, ArrayList<Binding>> orderedBindingsByTrigger = new HashMap<>();
 
-	public BindingTable(Context context) {
+	private IContributionFactory contributionFactory;
+
+	private MApplication application;
+
+	public BindingTable(Context context, MApplication application) {
 		tableId = context;
+		this.application = application;
 	}
 
 	public Context getTableId() {
@@ -309,34 +317,63 @@ private void evaluateOrderedBindings(TriggerSequence sequence, Binding binding)
 	}
 
 	public Binding getPerfectMatch(TriggerSequence trigger) {
-		return bindingsByTrigger.get(trigger);
+		Binding binding = bindingsByTrigger.get(trigger);
+		if (isActive(binding)) {
+			return binding;
+		}
+		return null;
 	}
 
 	public Binding getBestSequenceFor(ParameterizedCommand command) {
 		ArrayList<Binding> sequences = bindingsByCommand.get(command);
 		if (sequences != null && sequences.size() > 0) {
-			return sequences.get(0);
+			Binding binding = sequences.get(0);
+			if (isActive(binding)) {
+				return binding;
+			}
 		}
 		return null;
 	}
 
 	@SuppressWarnings("unchecked")
 	public Collection<Binding> getSequencesFor(ParameterizedCommand command) {
 		ArrayList<Binding> triggers = bindingsByCommand.get(command);
-		return (Collection<Binding>) (triggers == null ? Collections.emptyList() : triggers.clone());
+		return (Collection<Binding>) (triggers == null ? Collections.emptyList() : getActive(triggers));
 	}
 
 	public Collection<Binding> getPartialMatches(TriggerSequence sequence) {
-		return bindingsByPrefix.get(sequence);
+		return getActive(bindingsByPrefix.get(sequence));
 	}
 
 	public boolean isPartialMatch(TriggerSequence seq) {
 		ArrayList<Binding> values = bindingsByPrefix.get(seq);
-		return values != null && !values.isEmpty();
+		return values != null && !getActive(values).isEmpty();
 	}
 
 	public Collection<Binding> getBindings() {
-		return Collections.unmodifiableCollection(bindings);
+		return Collections.unmodifiableCollection(getActive(bindings));
+	}
+
+	List<Binding> getActive(List<Binding> bindings) {
+		return bindings.stream().filter(b -> isActive(b)).toList();
 	}
 
+	private boolean isActive(final Binding binding) {
+		if (binding == null) {
+			return false;
+		}
+		ParameterizedCommand command = binding.getParameterizedCommand();
+		if (command == null) {
+			return true;
+		}
+		String identifierId = command.getId();
+		if (contributionFactory == null) {
+			contributionFactory = application.getContext().get(IContributionFactory.class);
+		}
+		if (contributionFactory == null) {
+			return true;
+		}
+		boolean enabled = contributionFactory.isEnabled(identifierId);
+		return enabled;
+	}
 }

bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/keys/KeyBindingDispatcher.java

@@ -25,6 +25,7 @@
 import org.eclipse.core.commands.Command;
 import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.commands.IHandler;
+import org.eclipse.core.commands.NotEnabledException;
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.core.commands.common.CommandException;
 import org.eclipse.core.commands.contexts.ContextManager;
@@ -33,6 +34,7 @@
 import org.eclipse.e4.core.contexts.EclipseContextFactory;
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.di.annotations.Optional;
+import org.eclipse.e4.core.services.contributions.IContributionFactory;
 import org.eclipse.e4.core.services.log.Logger;
 import org.eclipse.e4.ui.bindings.EBindingService;
 import org.eclipse.e4.ui.bindings.internal.KeyAssistDialog;
@@ -63,7 +65,16 @@
  */
 public class KeyBindingDispatcher {
 
-	private KeyAssistDialog keyAssistDialog = null;
+	private KeyAssistDialog keyAssistDialog;
+
+	private IContributionFactory contributionFactory;
+
+	/**
+	 *
+	 */
+	public KeyBindingDispatcher() {
+		super();
+	}
 
 	/**
 	 * A display filter for handling key bindings. This filter can either be enabled or disabled. If
@@ -280,6 +291,10 @@ public final boolean executeCommand(final ParameterizedCommand parameterizedComm
 		// Reset the key binding state (close window, clear status line, etc.)
 		resetState(false);
 
+		if (!isActive(parameterizedCommand)) {
+			throw new NotEnabledException("Command should have been disabled via activity: " + parameterizedCommand); //$NON-NLS-1$
+		}
+
 		final EHandlerService handlerService = getHandlerService();
 		final Command command = parameterizedCommand.getCommand();
 
@@ -620,6 +635,12 @@ public boolean press(List<KeyStroke> potentialKeyStrokes, Event event) {
 		return !sequenceBeforeKeyStroke.isEmpty();
 	}
 
+	private boolean isActive(final ParameterizedCommand command) {
+		String identifierId = command.getId();
+		boolean enabled = contributionFactory.isEnabled(identifierId);
+		return enabled;
+	}
+
 	/**
 	 * <p>
 	 * Actually performs the processing of the key event by interacting with the
@@ -687,6 +708,7 @@ final public KeySequence getBuffer() {
 	@Inject
 	public void setContext(IEclipseContext context) {
 		this.context = context;
+		contributionFactory = context.get(IContributionFactory.class);
 	}
 
 	/**

bundles/org.eclipse.e4.ui.workbench.swt/src/org/eclipse/e4/ui/workbench/swt/util/BindingProcessingAddon.java

@@ -158,7 +158,7 @@ private void defineBindingTable(MBindingTable bindingTable) {
 		final Context bindingContext = contextManager.getContext(bindingTable.getBindingContext().getElementId());
 		BindingTable table = bindingTables.getTable(bindingTable.getBindingContext().getElementId());
 		if (table == null) {
-			table = new BindingTable(bindingContext);
+			table = new BindingTable(bindingContext, application);
 			bindingTables.addTable(table);
 		}
 		for (MKeyBinding binding : bindingTable.getBindings()) {
@@ -290,7 +290,7 @@ private void subscribeBindingTableContainerTopicBindingTables(
 				if (newObj instanceof MBindingTable) {
 					MBindingTable bt = (MBindingTable) newObj;
 					final Context bindingContext = contextManager.getContext(bt.getBindingContext().getElementId());
-					final BindingTable table = new BindingTable(bindingContext);
+					final BindingTable table = new BindingTable(bindingContext, application);
 					bindingTables.addTable(table);
 					List<MKeyBinding> bindings = bt.getBindings();
 					for (MKeyBinding binding : bindings) {

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchContributionFactory.java

@@ -58,8 +58,11 @@ public Bundle getBundle(String uriString) {
 
 	@Override
 	public boolean isEnabled(String uriString) {
-		if (uriString != null && uriString.startsWith(BUNDLE_CLASS_PREFIX)) {
-			String identifierId = uriString.substring(BUNDLE_CLASS_PREFIX.length());
+		if (uriString != null) {
+			String identifierId = uriString;
+			if (uriString.startsWith(BUNDLE_CLASS_PREFIX)) {
+				identifierId = uriString.substring(BUNDLE_CLASS_PREFIX.length());
+			}
 			if (activitySupport == null) {
 				activitySupport = context.get(IWorkbenchActivitySupport.class);
 			}

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/keys/CategoryPatternFilter.java

@@ -19,15 +19,20 @@
 import org.eclipse.core.commands.common.NotDefinedException;
 import org.eclipse.jface.bindings.Binding;
 import org.eclipse.jface.viewers.Viewer;
+import org.eclipse.ui.activities.IActivityManager;
+import org.eclipse.ui.activities.IIdentifier;
 import org.eclipse.ui.dialogs.PatternFilter;
 import org.eclipse.ui.internal.keys.model.BindingElement;
 
 class CategoryPatternFilter extends PatternFilter {
 	private boolean filterCategories;
 	final Category uncategorized;
 
-	public CategoryPatternFilter(boolean filterCategories, Category c) {
+	private IActivityManager activityManager;
+
+	public CategoryPatternFilter(boolean filterCategories, Category c, IActivityManager activityManager) {
 		uncategorized = c;
+		this.activityManager = activityManager;
 		filterCategories(filterCategories);
 	}
 
@@ -55,10 +60,20 @@ protected boolean isLeafMatch(Viewer viewer, Object element) {
 			} catch (NotDefinedException e) {
 				return false;
 			}
+			if (!isActive(cmd)) {
+				return false;
+			}
 		}
 		return super.isLeafMatch(viewer, element);
 	}
 
+	private boolean isActive(final ParameterizedCommand command) {
+		String identifierId = command.getId();
+		IIdentifier identifier = activityManager.getIdentifier(identifierId);
+		boolean enabled = identifier.isEnabled();
+		return enabled;
+	}
+
 	private ParameterizedCommand getCommand(Object element) {
 		if (element instanceof BindingElement) {
 			Object modelObject = ((BindingElement) element).getModelObject();

bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/keys/NewKeysPreferencePage.java

@@ -93,6 +93,7 @@
 import org.eclipse.ui.IWorkbench;
 import org.eclipse.ui.IWorkbenchPreferencePage;
 import org.eclipse.ui.PlatformUI;
+import org.eclipse.ui.activities.IActivityManager;
 import org.eclipse.ui.commands.ICommandImageService;
 import org.eclipse.ui.commands.ICommandService;
 import org.eclipse.ui.dialogs.FilteredTree;
@@ -203,6 +204,12 @@ public class NewKeysPreferencePage extends PreferencePage implements IWorkbenchP
 
 	private IWorkbench fWorkbench;
 
+	/**
+	 * The workbench's activity manager. This activity manager is used to see if
+	 * certain commands should be filtered from the user interface.
+	 */
+	private IActivityManager activityManager;
+
 	/**
 	 * A FilteredTree that provides a combo which is used to organize and display
 	 * elements in the tree according to the selected criteria.
@@ -472,7 +479,7 @@ protected Control createContents(Composite parent) {
 
 		IDialogSettings settings = getDialogSettings();
 
-		fPatternFilter = new CategoryPatternFilter(true, commandService.getCategory(null));
+		fPatternFilter = new CategoryPatternFilter(true, commandService.getCategory(null), activityManager);
 		if (settings.get(TAG_FILTER_UNCAT) != null) {
 			fPatternFilter.filterCategories(settings.getBoolean(TAG_FILTER_UNCAT));
 		}
@@ -854,7 +861,7 @@ public String getColumnText(Object o, int index) {
 	}
 
 	private void createTree(Composite parent) {
-		fPatternFilter = new CategoryPatternFilter(true, fDefaultCategory);
+		fPatternFilter = new CategoryPatternFilter(true, fDefaultCategory, activityManager);
 		fPatternFilter.filterCategories(true);
 
 		GridData gridData;
@@ -1089,6 +1096,7 @@ public void init(IWorkbench workbench) {
 		fBindingService = workbench.getService(IBindingService.class);
 
 		commandImageService = workbench.getService(ICommandImageService.class);
+		activityManager = workbench.getActivitySupport().getActivityManager();
 	}
 
 	@Override