Community
Participate
Working Groups
Basically CommandService.getHelpContextId(command) delegates to the CommandManager instance, which looks to see if a helpContextId has been registered for the provided command's handler. Since we set most Command handlers to an instance MakeHandlersGo, and LegacyHandlerService doesn't actually associate and record the helpContextIds, most handlers don't have a helpContextId.
When trying to fix this bug I noticed the following problems: 1. Command.getHelpContextId() is package private, hence I need to use CommandManager to retrieve the value (or keep the command-helpContextId in a separate place). 2. HandlerServiceHandler does not provide any method to determine the current 'real' handler, so I need to add a new method to it or store the information somewhere else. 3. CommandManager does not allow to get helpContextId by handler (but only by command), therefore it cannot be used for commands with assigned HandlerServiceHandler. Thus, the question is whether the fix should be a minimal change that restores the missing functionality or a new approach to managing helpContextId is necessary. I uploaded to gerrit a simple patch that experiments with the first approach, but I will try to provide a better solution. The Gerrit URL is: https://git.eclipse.org/r/23343 Any comments and suggestions are welcome.
(In reply to Wojciech Sudol from comment #1) > When trying to fix this bug I noticed the following problems: > 1. Command.getHelpContextId() is package private, hence I need to use > CommandManager to retrieve the value (or keep the command-helpContextId in a > separate place). We do have access to the CommandManager in the IEclipseContext, if you need it. > 2. HandlerServiceHandler does not provide any method to determine the > current 'real' handler, so I need to add a new method to it or store the > information somewhere else. I see we're using the HandlerServiceHandler to look up the IHandler. Would it not make more sense to do that in the CommandService implementation, instead of going to the HandlerServiceHandler? > > I uploaded to gerrit a simple patch that experiments with the first > approach, but I will try to provide a better solution. > The Gerrit URL is: https://git.eclipse.org/r/23343 What difference can I see in the UI if we enable this functionality? PW
I can see we can't use the CommandManager to manage help-per-handler any more, since the Command will return the HandlerServiceHandler. I guess the primary thing to solve is the HandlerContributionItem responding to an SWT.Help event. I think having your internal IHelpService would be OK. It can take a command and handler, figure out a helpId, and display it. The HandleContributionItem can provide the context and then look up the handler the same was as HandlerServiceHandler. Then if it's an E4ProxyHandler, we can get the IHandler from it and calculate the helpId from that and fall back to the Command. Then the call the workbench level help display. The implementation of the IHelpService can live in the workbench, where it can see all of the players. The secondary thing to solve is org.eclipse.ui.commands.ICommandService.getHelpContextId(String) mostly works like when called from org.eclipse.ui.menus.CommandContributionItem.CommandContributionItem(CommandContributionItemParameter) PW
When I rebase the change on master, I get some failures in UIAllTest: UIAllTests-fixed org.eclipse.e4.ui.tests.UIAllTests org.eclipse.e4.ui.tests.workbench.MMenuItemTest testElementHierarchyInContext_HandledItem(org.eclipse.e4.ui.tests.workbench.MMenuItemTest) junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:55) at junit.framework.Assert.assertTrue(Assert.java:22) at junit.framework.Assert.assertNotNull(Assert.java:256) at junit.framework.Assert.assertNotNull(Assert.java:248) at junit.framework.TestCase.assertNotNull(TestCase.java:417) at org.eclipse.e4.ui.tests.workbench.MMenuItemTest.testElementHierarchyInContext_HandledItem(MMenuItemTest.java:764) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at junit.framework.TestCase.runTest(TestCase.java:176) at junit.framework.TestCase.runBare(TestCase.java:141) at junit.framework.TestResult$1.protect(TestResult.java:122) at junit.framework.TestResult.runProtected(TestResult.java:142) at junit.framework.TestResult.run(TestResult.java:125) at junit.framework.TestCase.run(TestCase.java:129) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:131) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382) at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62) at org.eclipse.pde.internal.junit.runtime.CoreTestApplication.run(CoreTestApplication.java:23) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:379) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:233) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465) at org.eclipse.equinox.launcher.Main.main(Main.java:1438) testMHandledMenuItem_Check_Bug316752(org.eclipse.e4.ui.tests.workbench.MMenuItemTest) junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:55) at junit.framework.Assert.assertTrue(Assert.java:22) at junit.framework.Assert.assertNotNull(Assert.java:256) at junit.framework.Assert.assertNotNull(Assert.java:248) at junit.framework.TestCase.assertNotNull(TestCase.java:417) at org.eclipse.e4.ui.tests.workbench.MMenuItemTest.testMHandledMenuItem_Check_Bug316752(MMenuItemTest.java:276) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at junit.framework.TestCase.runTest(TestCase.java:176) at junit.framework.TestCase.runBare(TestCase.java:141) at junit.framework.TestResult$1.protect(TestResult.java:122) at junit.framework.TestResult.runProtected(TestResult.java:142) at junit.framework.TestResult.run(TestResult.java:125) at junit.framework.TestCase.run(TestCase.java:129) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:131) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382) at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62) at org.eclipse.pde.internal.junit.runtime.CoreTestApplication.run(CoreTestApplication.java:23) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:379) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:233) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465) at org.eclipse.equinox.launcher.Main.main(Main.java:1438)
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2f4ee54fdf2cb0b1f5a9fe73a52917b6b1e92779 Thanks Wojtek. PW
*** Bug 366562 has been marked as a duplicate of this bug. ***
Verified in I20140428-2000.
