Community
Participate
Working Groups
Starting with Kepler, the ExecutionEvent in IExecutionListener.preExecute no longer includes trigger information. A sample stack trace from a command executed via a keybinding: CommandManager$ExecutionListener.preExecute(String, ExecutionEvent) line: 126 Command.firePreExecute(ExecutionEvent) line: 693 Command.executeWithChecks(ExecutionEvent) line: 465 ParameterizedCommand.executeWithChecks(Object, Object) line: 508 HandlerServiceImpl.executeHandler(ParameterizedCommand, IEclipseContext) line: 213 KeyBindingDispatcher.executeCommand(ParameterizedCommand, Event) line: 285 KeyBindingDispatcher.press(List<KeyStroke>, Event) line: 504 KeyBindingDispatcher.processKeyEvent(List<KeyStroke>, Event) line: 555 KeyBindingDispatcher.filterKeySequenceBindings(Event) line: 376 KeyBindingDispatcher.access$0(KeyBindingDispatcher, Event) line: 322 KeyBindingDispatcher$KeyDownFilter.handleEvent(Event) line: 84 .... The code in the HandlerServiceImpl explicity does not get/include the trigger in the information passed that creates the ExecutionEvent public Object executeHandler(ParameterizedCommand command, IEclipseContext staticContext) { final IEclipseContext executionContext = getExecutionContext(); addParms(command, staticContext); // executionContext.set(STATIC_CONTEXT, staticContext); push(executionContext, staticContext); try { // Command cmd = command.getCommand(); return command.executeWithChecks(null, peek()); } catch (ExecutionException e) {... As a result the ExecutionEvent.getTrigger will always return null making it completely useless in this context and breaks code that relied on the information contained therein... Why the change? It would be useful to Emacs+ to support the previous behavior
Might be a dup of bug 412681 PW
I do not think so. The description is different and looking at the diff associated with 412681, it still shows null being explicitly set as the trigger: @@ -170,7 +171,7 @@ public class HandlerServiceImpl implements EHandlerService { push(executionContext, staticContext); try { Command cmd = command.getCommand(); - cmd.setEnabled(peek()); + cmd.setEnabled(new ExpressionContext(peek().context)); return cmd.isEnabled(); } finally { pop(); @@ -210,7 +211,7 @@ public class HandlerServiceImpl implements EHandlerService { push(executionContext, staticContext); try { // Command cmd = command.getCommand(); - return command.executeWithChecks(null, peek()); + return command.executeWithChecks(null, new ExpressionContext(peek().context)); } catch (ExecutionException e) { staticContext.set(HANDLER_EXCEPTION, e); } catch (NotDefinedException e) { (In reply to Paul Webster from comment #1) > Might be a dup of bug 412681 > > PW
(In reply to comment #1) > Might be a dup of bug 412681 > > PW Definitely not a dup. The trigger is being stored in the static context (under Event.class), but then not being recovered in HandlerServiceImpl, as comment 2 shows.
Pushed patch to Gerrit for review: https://git.eclipse.org/r/17011
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=3151a509be6eae7055d9c0aaba2c9ecb0b3db6e6 PW
Fixed in master per comment 5.
Verified in 4.4.0.I20131028-2000. Did a Ctrl-M and traced the execution code which showed that the 'trigger' parameter was now an set.Event.