Community
Participate
Working Groups
Programmatic Contribution Factories are not supported in the compatibility layer of eclipse Build 20111110-1543. Check org.eclipse.ui.internal.menus.MenuPersistence: there is some removed (commented out) code in the readAdditions() method together with a system-out informing that these factories are not supported. This feature is important to the Eclipse Scout Technology Project. We set the severity to critical because this is needed so that the Scout Project can be part of EPP for Juno.
I've pushed my work in progress to pwebster/bug365903 PW
I've released code to activate the programmatic factories. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=29f5576700d9276a469f660e0c03d88a8bbe6e39 PW
Matthias, could you please pick up version of 4.2 [1] and confirm your behaviour has improved as we head into M5 today? PW [1] http://download.eclipse.org/eclipse/downloads/drops4/I20120123-0100/index.php
Hi Paul I actually started testing your new code using the 4.2 Stream Integration Build (I20120123-0100) this morning :) The good first: All our context menus are now shown successfully. Thank you very much! However, we maybe found some issues: 1. We have registered a VisibilityTester using the org.eclipse.core.expressions.propertyTesters extension point. Now when we register our contribution items in the ExtensionContributionFactory.createContributionItems() we also create a TestExpression like this: TestExpression ex = new TestExpression("whatEverNamespace", "whatEverProperty", args, Boolean.TRUE); Then we add the contribution item together with the visibleWhen Expression: additions.addContributionItem(item, ex); As far as I can see, these visibleWhen expressions are not yet evaluated (I have checked org.eclipse.ui.internal.menus.WorkbenchMenuService.registerVisibleWhen())? 2. Furthermore when the menu is clicked by the user, the handler is not executed because org.eclipse.e4.core.commands.internal.HandlerServiceImpl.lookUpHandler() does not find our handler. Do we now need to register it somewhere? 3. We dynamically assign keystrokes to our UI context using the IBindingService.addBinding(). These bindings have been displayed in past releases after the name of the context menu (like "Paste [Ctrl+V]"). These bindings don't seem to work anymore. I cannot tell if they are just not shown or if they actually are not executed when pressed (see point 2). Thank you very much for your work so far and for furhter looking into it! Matthias
(In reply to comment #4) > Hi Paul > I actually started testing your new code using the 4.2 Stream Integration Build > (I20120123-0100) this morning :) Thank you for that, that's great. > However, we maybe found some issues: > 1. > TestExpression ex = new TestExpression("whatEverNamespace", "whatEverProperty", > args, Boolean.TRUE); > > Then we add the contribution item together with the visibleWhen Expression: > > additions.addContributionItem(item, ex); Yes, that's unimplemented but definitely needs to be. I'll look at that today. > 2. Furthermore when the menu is clicked by the user, the handler is not > executed because > org.eclipse.e4.core.commands.internal.HandlerServiceImpl.lookUpHandler() does > not find our handler. Do we now need to register it somewhere? For a given CommandContributionItem (and matching o.e.ui.commands definition) how are you associating a handler for it today? org.eclipse.ui.handlers? Or a call to IHandlerService.activateHandler(*)? > 3. We dynamically assign keystrokes to our UI context using the > IBindingService.addBinding(). These bindings have been displayed in past > releases after the name of the context menu (like "Paste [Ctrl+V]"). These > bindings don't seem to work anymore. I cannot tell if they are just not shown > or if they actually are not executed when pressed (see point 2). The keybindings on a context menu or through IBindingService won't be ready for M5, but we definitely want to look at that in M6. I'd like to make sure that #1 and #2 work to close of this bug and then we might open another for the keybindings. PW
(In reply to comment #5) > (In reply to comment #4) > > Hi Paul > > I actually started testing your new code using the 4.2 Stream Integration Build > > (I20120123-0100) this morning :) > > Thank you for that, that's great. > > > However, we maybe found some issues: > > 1. > > TestExpression ex = new TestExpression("whatEverNamespace", "whatEverProperty", > > args, Boolean.TRUE); > > > > Then we add the contribution item together with the visibleWhen Expression: > > > > additions.addContributionItem(item, ex); > > Yes, that's unimplemented but definitely needs to be. I'll look at that today. > Perfect! Thank you very much! > > 2. Furthermore when the menu is clicked by the user, the handler is not > > executed because > > org.eclipse.e4.core.commands.internal.HandlerServiceImpl.lookUpHandler() does > > not find our handler. Do we now need to register it somewhere? > > For a given CommandContributionItem (and matching o.e.ui.commands definition) > how are you associating a handler for it today? org.eclipse.ui.handlers? Or a > call to IHandlerService.activateHandler(*)? We first create and define a command using the ICommandService.getCommand(*) and Command.define(). On this command wet set the handler using setHandler(). Afterwards we create a CommandContributionItemParameter and pass the Id of the command just created. Then we can create a CommandContributionItem with the created CommandContributionItemParameter passed to the constructor. Something like that: ICommandService cs = (ICommandService) serviceLocator.getService(ICommandService.class); Command cmd = cs.getCommand("someId"); if (!cmd.isDefined()) { cmd.define("name", "desc", cs.getCategory(ICommandService. AUTOGENERATED_CATEGORY_ID)); cmd.setHandler(ourHandler); } CommandContributionItemParameter p = new CommandContributionItemParameter(serviceLocator, cmd.getId(), cmd.getId(), SWT.PUSH); ... CommandContributionItem item = new CommandContributionItem(p); Should we change that code or activate the handler using the IHandlerService first? Or do you have any other recommendations that will make it work? Thanks! > > > > 3. We dynamically assign keystrokes to our UI context using the > > IBindingService.addBinding(). These bindings have been displayed in past > > releases after the name of the context menu (like "Paste [Ctrl+V]"). These > > bindings don't seem to work anymore. I cannot tell if they are just not shown > > or if they actually are not executed when pressed (see point 2). > > The keybindings on a context menu or through IBindingService won't be ready for > M5, but we definitely want to look at that in M6. > > I'd like to make sure that #1 and #2 work to close of this bug and then we > might open another for the keybindings. > > PW Ok, thats fine with me. The first two points are more important and to have the keybindings implemented in M6 is absolutely sufficient. Thank you very much. Matthias
(In reply to comment #6) > On this command wet set the handler using setHandler(). I think this is it. Using the ICommandService to create the command works, but you need to use the IHandlerService to activate your handler for that command. Otherwise the IHandlerService is free to unset your handler as its own internal logic dictates (this is true in 3.x as well, but it sounds like you didn't hit that scenario). You can replace setHandler(ourHandler) with: IHandlerService hs = (IHandlerService) serviceLocator.getService(IHandlerService.class); hs.activateHandler("someId", ourHandler); The gotcha? You have to deactivate that handler before you activate it again, or you will get a conflict. As long as nothing undefines your command, that shouldn't be a problem. PW
Created attachment 209944 [details] Re-activate the registerVisibleWhen v01 This looks to be more involved, pulling in code from 3.7 to use the IEvaluationService to manage the IContribionItem visibleWhen. I don't believe I can use this in M5, it should probably wait until M6 PW
(In reply to comment #7) > You can replace setHandler(ourHandler) with: > > IHandlerService hs > = (IHandlerService) serviceLocator.getService(IHandlerService.class); > hs.activateHandler("someId", ourHandler); > > The gotcha? You have to deactivate that handler before you activate it again, > or you will get a conflict. As long as nothing undefines your command, that > shouldn't be a problem. > > PW That helped, thank you. The handler is now successfully executed. But I had to keep the Command.setHandler(*). As soon as I remove this code, the contribution items are always disabled. This is because the instanceof check in Command.setEnabled(Object context) then returns false as the handler is null. Did I miss something else here?
(In reply to comment #9) > That helped, thank you. The handler is now successfully executed. But I had to > keep the Command.setHandler(*). As soon as I remove this code, the contribution > items are always disabled. That's a bug in my system. Lemme see if I can address it. PW
(In reply to comment #9) > That helped, thank you. The handler is now successfully executed. But I had to > keep the Command.setHandler(*). As soon as I remove this code, the contribution > items are always disabled. Newly defined commands now get the correct system handler: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=96bc96e06ea255517c8b2d0d7b93ac0f01ff2728 That should be in tonight's 4.2 build, I20120123-2200, and part of our M5 contribution to Juno. When you can see this, it should be OK to remove your setHandler(*) call. PW
(In reply to comment #11) > > Newly defined commands now get the correct system handler: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=96bc96e06ea255517c8b2d0d7b93ac0f01ff2728 > > That should be in tonight's 4.2 build, I20120123-2200, and part of our M5 > contribution to Juno. When you can see this, it should be OK to remove your > setHandler(*) call. > > PW Yes, it is working now. The items are now enabled without setting the handler manually to the command. Thank you! So we are only waiting for the visibleWhen expressions to be implemented to resolve this bug. As discussed earlier: the keyBindings will be tracked in another bug (not yet created). Matthias
I've released http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=fc05e669af716337103c542f7fcfa98e4f622574 for tonight's I build, I20120202-2200 Matthias, when you get a chance I'd like to hear your feedback. If I haven't got all the usecases, could you just comment with an expression you feel isn't working correctly? PW
(In reply to comment #13) > > Matthias, when you get a chance I'd like to hear your feedback. If I haven't > got all the usecases, could you just comment with an expression you feel isn't > working correctly? > > PW I have checked with build I20120202-2200. Unfortunately the visibleWhen expressions do not work yet. Our tester class (extends org.eclipse.core.expressions.PropertyTester) is not invoked and still all context menus are are always shown. matthias
(In reply to comment #14) > I have checked with build I20120202-2200. Unfortunately the visibleWhen > expressions do not work yet. Our tester class (extends > org.eclipse.core.expressions.PropertyTester) is not invoked and still all > context menus are are always shown. > > matthias Thanx for checking. It looks like my fix didn't make it into the build. I'm trying to get that fixed. PW
For your property tester, are you using org.eclipse.ui.services.IEvaluationService.requestEvaluation(String)? PW
(In reply to comment #16) > For your property tester, are you using > org.eclipse.ui.services.IEvaluationService.requestEvaluation(String)? > > PW I am not sure about that. We just register a property tester (see below) and add the contribution item with that TestExpression (see comment 4). We register our tester like that: <extension point="org.eclipse.core.expressions.propertyTesters"> <propertyTester class="org.eclipse.scout.sdk.ui.menu.MenuVisibilityTester" id="whatEverNamespace.whatEverProperty" namespace="whatEverNamespace" properties="whatEverProperty" type="java.lang.Object"> </propertyTester> </extension>
OK, applied to menus that should be good in I20120203-0615 (coming soon). PW
Ok, thank you. I will test it as soon as possible. regards, m
I have tested the visibleWhen expressions (used build I20120203-1530) and it works great! Thanks a lot! However I maybe found another issue: when using the context menu the very first time (so the first right click in the view) it seems as if the menu items are doubled (see left side of attached screenshot). All further right clicks are correct (right side of attached screenshot). Thanks matthias
Created attachment 210559 [details] Context Menu Screenshot
(In reply to comment #21) > Created attachment 210559 [details] > Context Menu Screenshot Could you put a breakpoint in your ProgrammatiContributionFactory.createContributionItems(IServiceLocator, IContributionRoot)? Or a java.lang.Thread.dumpStack()? I'd like to see the stack traces for the case where you see 2 copies of everything. I can't reproduce that with my popup factory here. PW
Created attachment 210671 [details] StackTrace printed during startup of eclipse
Created attachment 210672 [details] StackTrace printed on first right-click after eclipse startup is completed
I have added two stack trace attachments: The first is printed while eclipse is starting up (splash screen visible). The second one is then printed after the view has become visible, when the first right-click is performed (this is the time where we have two copies of all items). matthias
(In reply to comment #25) > The second one is then printed after the view has become visible, when the > first right-click is performed (this is the time where we have two copies of > all items). > Thanx Matthias, that helps. PW
(In reply to comment #26) > > Thanx Matthias, that helps. Matthias, when you register the MenuManager for the part in question, does the manager have removeAllWhenShown set to true or false? PW
OK, I got rid of the duplicate menus on the first call, although there are probably still 2 calls to the programmatic contribution factories. PW
Hi Paul, thank you very much. I will test it tomorrow. However, I also checked the removeAllWhenShown value: It is set to false (default). Setting it to true already solves the issue with the doubled entries for build I20120203-1530. But anyway I will test it again tomorrow with your latest changes. Thanks a lot! Matthias
(In reply to comment #29) > However, I also checked the removeAllWhenShown value: It is set to false > (default). Setting it to true already solves the issue with the doubled entries > for build I20120203-1530. But anyway I will test it again tomorrow with your > latest changes. > Most Part context menus set removeAllWhenShown to true, but it's not an absolute hard requirement. This fix will work with your current menus, although if you set it to true you will also be fine. PW
I tested again using build I20120208-2200 and the menus work fine! Thank you very much Paul for solving this!
In I20120313-0610 PW
I don't happen to see any linked tickets/bugs for the IBindingService.addBinding issue here. Is there one opened for that? I'm attempting to define new commands, hook up bindings for them and add expressions for enablement that are dynamic - all programmatically. When I attempt to add the bindings, that part is failing. It looks up application.getCommands() and doesn't find the programmatically defined command in BindingService.createMKeyBinding, line 679. It looks like the programmatcially defined commands got to a different set than the Here's the basic code I'm running to set this up. (Am I doing things wrong?) // Define our commandElement model as an Eclipse command String commandId = "com.example.command." + commandElement.getDisplayName(); Command c = cs.getCommand(commandId); if (!c.isDefined()) { c.define(commandElement.getDisplayName(), commandElement.getDisplayName(), cat); Expression expression = new Expression() { @Override public EvaluationResult evaluate(IEvaluationContext context) throws CoreException { IWorkbenchPart workbenchPart = (IWorkbenchPart) context.getVariable(ISources.ACTIVE_PART_NAME); if (workbenchPart instanceof IEditorPart) { // do more checking return EvaluationResult.TRUE; } return EvaluationResult.FALSE; } }; } KeySequence[] keySequences = commandElement.getKeySequences(); if (!ArrayUtil.isEmpty(keySequences)) { if (bindingService instanceof BindingService) { final BindingService theBindingService = (BindingService) bindingService; // Add to the set for (KeySequence keySequence : keySequences) { KeyBinding binding = new KeyBinding(keySequence, new ParameterizedCommand(c, null), "org.eclipse.ui.defaultAcceleratorConfiguration", "myContext", null, null, null, Binding.USER); theBindingService.addBinding(binding); } } }
(In reply to comment #33) > I don't happen to see any linked tickets/bugs for the > IBindingService.addBinding issue here. Is there one opened for that? > BindingService.addBinding(*) is an internal method ... you can't use it. While I didn't see where your expression is used, you should override org.eclipse.core.expressions.Expression.collectExpressionInfo(ExpressionInfo) to specify which variables your expression uses, like ISources.ACTIVE_PART_NAME. You can open a new bug about ICommandService.getCommand(*) and then define(*) not creating a new MCommand model element. PW
I've opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=379162 for the Commande.define issue. As for BindingService.addBinding, I am aware that it's an internal method so I _shouldn't_ be accessing it right now, but I don't see any other way of programmatically assigning bindings for a programmatically generated command. Is there a supported way to do so? In the 3.x series we had a giant hack that effectively unhooked the key up/down listener, attached our own, and then re-attached the Eclipse one. Then when we got key events we had to handle checking against the bindings we had a list for and delegate to our "commands" if there was a match.
