Skip to content

Commit eb090da

Browse files
iloveeclipseiloveeclipse
committed
[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).
1 parent 328646d commit eb090da

File tree

12 files changed

+151
-58
lines changed

12 files changed

+151
-58
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Manifest-Version: 1.0
22
Bundle-ManifestVersion: 2
33
Bundle-SymbolicName: org.eclipse.e4.ui.bindings;singleton:=true
4-
Bundle-Version: 0.14.500.qualifier
4+
Bundle-Version: 0.14.600.qualifier
55
Bundle-Name: %pluginName
66
Bundle-Vendor: %providerName
77
Bundle-Localization: plugin
@@ -20,7 +20,9 @@ Require-Bundle: org.eclipse.swt;bundle-version="[3.6.0,4.0.0)",
2020
org.eclipse.core.commands;bundle-version="[3.5.0,4.0.0)",
2121
org.eclipse.e4.core.contexts;bundle-version="1.0.0",
2222
org.eclipse.e4.core.di;bundle-version="1.1.0",
23-
org.eclipse.e4.ui.services;bundle-version="1.0.0"
23+
org.eclipse.e4.ui.services;bundle-version="1.0.0",
24+
org.eclipse.e4.core.services;bundle-version="2.5.100",
25+
org.eclipse.e4.ui.model.workbench
2426
Export-Package: org.eclipse.e4.ui.bindings;
2527
x-friends:="org.eclipse.e4.ui.workbench,
2628
org.eclipse.e4.ui.workbench.renderers.swt,

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

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@
2020
import java.util.Comparator;
2121
import java.util.HashMap;
2222
import java.util.Iterator;
23+
import java.util.List;
2324
import java.util.Map;
2425
import org.eclipse.core.commands.ParameterizedCommand;
2526
import org.eclipse.core.commands.contexts.Context;
27+
import org.eclipse.e4.core.services.contributions.IContributionFactory;
28+
import org.eclipse.e4.ui.model.application.MApplication;
2629
import org.eclipse.jface.bindings.Binding;
2730
import org.eclipse.jface.bindings.Trigger;
2831
import org.eclipse.jface.bindings.TriggerSequence;
@@ -131,8 +134,13 @@ private final int countStrokes(final Trigger[] triggers) {
131134
private Map<TriggerSequence, ArrayList<Binding>> conflicts = new HashMap<>();
132135
private Map<TriggerSequence, ArrayList<Binding>> orderedBindingsByTrigger = new HashMap<>();
133136

134-
public BindingTable(Context context) {
137+
private IContributionFactory contributionFactory;
138+
139+
private MApplication application;
140+
141+
public BindingTable(Context context, MApplication application) {
135142
tableId = context;
143+
this.application = application;
136144
}
137145

138146
public Context getTableId() {
@@ -309,34 +317,63 @@ private void evaluateOrderedBindings(TriggerSequence sequence, Binding binding)
309317
}
310318

311319
public Binding getPerfectMatch(TriggerSequence trigger) {
312-
return bindingsByTrigger.get(trigger);
320+
Binding binding = bindingsByTrigger.get(trigger);
321+
if (isActive(binding)) {
322+
return binding;
323+
}
324+
return null;
313325
}
314326

315327
public Binding getBestSequenceFor(ParameterizedCommand command) {
316328
ArrayList<Binding> sequences = bindingsByCommand.get(command);
317329
if (sequences != null && sequences.size() > 0) {
318-
return sequences.get(0);
330+
Binding binding = sequences.get(0);
331+
if (isActive(binding)) {
332+
return binding;
333+
}
319334
}
320335
return null;
321336
}
322337

323338
@SuppressWarnings("unchecked")
324339
public Collection<Binding> getSequencesFor(ParameterizedCommand command) {
325340
ArrayList<Binding> triggers = bindingsByCommand.get(command);
326-
return (Collection<Binding>) (triggers == null ? Collections.emptyList() : triggers.clone());
341+
return (Collection<Binding>) (triggers == null ? Collections.emptyList() : getActive(triggers));
327342
}
328343

329344
public Collection<Binding> getPartialMatches(TriggerSequence sequence) {
330-
return bindingsByPrefix.get(sequence);
345+
return getActive(bindingsByPrefix.get(sequence));
331346
}
332347

333348
public boolean isPartialMatch(TriggerSequence seq) {
334349
ArrayList<Binding> values = bindingsByPrefix.get(seq);
335-
return values != null && !values.isEmpty();
350+
return values != null && !getActive(values).isEmpty();
336351
}
337352

338353
public Collection<Binding> getBindings() {
339-
return Collections.unmodifiableCollection(bindings);
354+
return Collections.unmodifiableCollection(getActive(bindings));
355+
}
356+
357+
List<Binding> getActive(List<Binding> bindings) {
358+
return bindings == null ? null : bindings.stream().filter(b -> isActive(b)).toList();
340359
}
341360

361+
private boolean isActive(final Binding binding) {
362+
if (binding == null) {
363+
return false;
364+
}
365+
ParameterizedCommand command = binding.getParameterizedCommand();
366+
if (command == null) {
367+
return true;
368+
}
369+
String identifierId = command.getId();
370+
if (contributionFactory == null) {
371+
contributionFactory = application.getContext().get(IContributionFactory.class);
372+
}
373+
if (contributionFactory == null) {
374+
return true;
375+
}
376+
boolean enabled = contributionFactory.isEnabled(identifierId);
377+
return enabled;
378+
}
342379
}

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.eclipse.core.commands.Command;
2626
import org.eclipse.core.commands.ExecutionException;
2727
import org.eclipse.core.commands.IHandler;
28+
import org.eclipse.core.commands.NotEnabledException;
2829
import org.eclipse.core.commands.ParameterizedCommand;
2930
import org.eclipse.core.commands.common.CommandException;
3031
import org.eclipse.core.commands.contexts.ContextManager;
@@ -33,6 +34,7 @@
3334
import org.eclipse.e4.core.contexts.EclipseContextFactory;
3435
import org.eclipse.e4.core.contexts.IEclipseContext;
3536
import org.eclipse.e4.core.di.annotations.Optional;
37+
import org.eclipse.e4.core.services.contributions.IContributionFactory;
3638
import org.eclipse.e4.core.services.log.Logger;
3739
import org.eclipse.e4.ui.bindings.EBindingService;
3840
import org.eclipse.e4.ui.bindings.internal.KeyAssistDialog;
@@ -63,7 +65,16 @@
6365
*/
6466
public class KeyBindingDispatcher {
6567

66-
private KeyAssistDialog keyAssistDialog = null;
68+
private KeyAssistDialog keyAssistDialog;
69+
70+
private IContributionFactory contributionFactory;
71+
72+
/**
73+
*
74+
*/
75+
public KeyBindingDispatcher() {
76+
super();
77+
}
6778

6879
/**
6980
* 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
280291
// Reset the key binding state (close window, clear status line, etc.)
281292
resetState(false);
282293

294+
if (!isActive(parameterizedCommand)) {
295+
throw new NotEnabledException("Command should have been disabled via activity: " + parameterizedCommand); //$NON-NLS-1$
296+
}
297+
283298
final EHandlerService handlerService = getHandlerService();
284299
final Command command = parameterizedCommand.getCommand();
285300

@@ -620,6 +635,12 @@ public boolean press(List<KeyStroke> potentialKeyStrokes, Event event) {
620635
return !sequenceBeforeKeyStroke.isEmpty();
621636
}
622637

638+
private boolean isActive(final ParameterizedCommand command) {
639+
String identifierId = command.getId();
640+
boolean enabled = contributionFactory.isEnabled(identifierId);
641+
return enabled;
642+
}
643+
623644
/**
624645
* <p>
625646
* Actually performs the processing of the key event by interacting with the
@@ -687,6 +708,7 @@ final public KeySequence getBuffer() {
687708
@Inject
688709
public void setContext(IEclipseContext context) {
689710
this.context = context;
711+
contributionFactory = context.get(IContributionFactory.class);
690712
}
691713

692714
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ private void defineBindingTable(MBindingTable bindingTable) {
158158
final Context bindingContext = contextManager.getContext(bindingTable.getBindingContext().getElementId());
159159
BindingTable table = bindingTables.getTable(bindingTable.getBindingContext().getElementId());
160160
if (table == null) {
161-
table = new BindingTable(bindingContext);
161+
table = new BindingTable(bindingContext, application);
162162
bindingTables.addTable(table);
163163
}
164164
for (MKeyBinding binding : bindingTable.getBindings()) {
@@ -290,7 +290,7 @@ private void subscribeBindingTableContainerTopicBindingTables(
290290
if (newObj instanceof MBindingTable) {
291291
MBindingTable bt = (MBindingTable) newObj;
292292
final Context bindingContext = contextManager.getContext(bt.getBindingContext().getElementId());
293-
final BindingTable table = new BindingTable(bindingContext);
293+
final BindingTable table = new BindingTable(bindingContext, application);
294294
bindingTables.addTable(table);
295295
List<MKeyBinding> bindings = bt.getBindings();
296296
for (MKeyBinding binding : bindings) {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,11 @@ public Bundle getBundle(String uriString) {
5858

5959
@Override
6060
public boolean isEnabled(String uriString) {
61-
if (uriString != null && uriString.startsWith(BUNDLE_CLASS_PREFIX)) {
62-
String identifierId = uriString.substring(BUNDLE_CLASS_PREFIX.length());
61+
if (uriString != null) {
62+
String identifierId = uriString;
63+
if (uriString.startsWith(BUNDLE_CLASS_PREFIX)) {
64+
identifierId = uriString.substring(BUNDLE_CLASS_PREFIX.length());
65+
}
6366
if (activitySupport == null) {
6467
activitySupport = context.get(IWorkbenchActivitySupport.class);
6568
}

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,20 @@
1919
import org.eclipse.core.commands.common.NotDefinedException;
2020
import org.eclipse.jface.bindings.Binding;
2121
import org.eclipse.jface.viewers.Viewer;
22+
import org.eclipse.ui.activities.IActivityManager;
23+
import org.eclipse.ui.activities.IIdentifier;
2224
import org.eclipse.ui.dialogs.PatternFilter;
2325
import org.eclipse.ui.internal.keys.model.BindingElement;
2426

2527
class CategoryPatternFilter extends PatternFilter {
2628
private boolean filterCategories;
2729
final Category uncategorized;
2830

29-
public CategoryPatternFilter(boolean filterCategories, Category c) {
31+
private IActivityManager activityManager;
32+
33+
public CategoryPatternFilter(boolean filterCategories, Category c, IActivityManager activityManager) {
3034
uncategorized = c;
35+
this.activityManager = activityManager;
3136
filterCategories(filterCategories);
3237
}
3338

@@ -55,10 +60,20 @@ protected boolean isLeafMatch(Viewer viewer, Object element) {
5560
} catch (NotDefinedException e) {
5661
return false;
5762
}
63+
if (!isActive(cmd)) {
64+
return false;
65+
}
5866
}
5967
return super.isLeafMatch(viewer, element);
6068
}
6169

70+
private boolean isActive(final ParameterizedCommand command) {
71+
String identifierId = command.getId();
72+
IIdentifier identifier = activityManager.getIdentifier(identifierId);
73+
boolean enabled = identifier.isEnabled();
74+
return enabled;
75+
}
76+
6277
private ParameterizedCommand getCommand(Object element) {
6378
if (element instanceof BindingElement) {
6479
Object modelObject = ((BindingElement) element).getModelObject();

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
import org.eclipse.ui.IWorkbench;
9494
import org.eclipse.ui.IWorkbenchPreferencePage;
9595
import org.eclipse.ui.PlatformUI;
96+
import org.eclipse.ui.activities.IActivityManager;
9697
import org.eclipse.ui.commands.ICommandImageService;
9798
import org.eclipse.ui.commands.ICommandService;
9899
import org.eclipse.ui.dialogs.FilteredTree;
@@ -203,6 +204,12 @@ public class NewKeysPreferencePage extends PreferencePage implements IWorkbenchP
203204

204205
private IWorkbench fWorkbench;
205206

207+
/**
208+
* The workbench's activity manager. This activity manager is used to see if
209+
* certain commands should be filtered from the user interface.
210+
*/
211+
private IActivityManager activityManager;
212+
206213
/**
207214
* A FilteredTree that provides a combo which is used to organize and display
208215
* elements in the tree according to the selected criteria.
@@ -472,7 +479,7 @@ protected Control createContents(Composite parent) {
472479

473480
IDialogSettings settings = getDialogSettings();
474481

475-
fPatternFilter = new CategoryPatternFilter(true, commandService.getCategory(null));
482+
fPatternFilter = new CategoryPatternFilter(true, commandService.getCategory(null), activityManager);
476483
if (settings.get(TAG_FILTER_UNCAT) != null) {
477484
fPatternFilter.filterCategories(settings.getBoolean(TAG_FILTER_UNCAT));
478485
}
@@ -854,7 +861,7 @@ public String getColumnText(Object o, int index) {
854861
}
855862

856863
private void createTree(Composite parent) {
857-
fPatternFilter = new CategoryPatternFilter(true, fDefaultCategory);
864+
fPatternFilter = new CategoryPatternFilter(true, fDefaultCategory, activityManager);
858865
fPatternFilter.filterCategories(true);
859866

860867
GridData gridData;
@@ -1089,6 +1096,7 @@ public void init(IWorkbench workbench) {
10891096
fBindingService = workbench.getService(IBindingService.class);
10901097

10911098
commandImageService = workbench.getService(ICommandImageService.class);
1099+
activityManager = workbench.getActivitySupport().getActivityManager();
10921100
}
10931101

10941102
@Override

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ Require-Bundle: org.eclipse.core.commands;bundle-version="3.5.0",
1515
org.eclipse.swt;bundle-version="3.6.0",
1616
org.eclipse.e4.core.contexts,
1717
org.eclipse.e4.core.di,
18-
org.mockito.mockito-core
18+
org.mockito.mockito-core,
19+
org.eclipse.e4.ui.model.workbench,
20+
org.eclipse.e4.ui.workbench,
21+
org.eclipse.e4.ui.workbench.swt
1922
Eclipse-BundleShape: dir
2023
Export-Package: org.eclipse.e4.ui.bindings.tests;x-internal:=true
2124
Automatic-Module-Name: org.eclipse.e4.ui.bindings.tests

‎tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/BindingLookupTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.eclipse.e4.ui.bindings.EBindingService;
3838
import org.eclipse.e4.ui.bindings.internal.BindingTable;
3939
import org.eclipse.e4.ui.bindings.internal.BindingTableManager;
40+
import org.eclipse.e4.ui.model.application.MApplication;
4041
import org.eclipse.e4.ui.services.ContextServiceAddon;
4142
import org.eclipse.e4.ui.services.EContextService;
4243
import org.eclipse.jface.bindings.Binding;
@@ -64,6 +65,7 @@ public class BindingLookupTest {
6465
private static final String TEST_ID2 = "test.id2";
6566

6667
private IEclipseContext workbenchContext;
68+
private MApplication application;
6769

6870
private void defineCommands(IEclipseContext context) {
6971
ECommandService cs = workbenchContext.get(ECommandService.class);
@@ -79,7 +81,7 @@ public void setUp() {
7981
ContextInjectionFactory.make(CommandServiceAddon.class, workbenchContext);
8082
ContextInjectionFactory.make(ContextServiceAddon.class, workbenchContext);
8183
ContextInjectionFactory.make(BindingServiceAddon.class, workbenchContext);
82-
84+
application = globalContext.get(MApplication.class);
8385
defineCommands(workbenchContext);
8486
defineContexts(workbenchContext);
8587
defineBindingTables(workbenchContext);
@@ -100,9 +102,9 @@ private void defineContexts(IEclipseContext context) {
100102
private void defineBindingTables(IEclipseContext context) {
101103
BindingTableManager btm = context.get(BindingTableManager.class);
102104
ContextManager cm = context.get(ContextManager.class);
103-
btm.addTable(new BindingTable(cm.getContext(ID_DIALOG_AND_WINDOW)));
104-
btm.addTable(new BindingTable(cm.getContext(ID_WINDOW)));
105-
btm.addTable(new BindingTable(cm.getContext(ID_DIALOG)));
105+
btm.addTable(new BindingTable(cm.getContext(ID_DIALOG_AND_WINDOW), application));
106+
btm.addTable(new BindingTable(cm.getContext(ID_WINDOW), application));
107+
btm.addTable(new BindingTable(cm.getContext(ID_DIALOG), application));
106108
}
107109

108110
@After

0 commit comments

Comments
 (0)