Migrate commands from E3 to E4 in org.eclipse.terminal.view.ui bundle #2205
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="ASCII"?> | ||
| <fragment:ModelFragments xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:commands="http://www.eclipse.org/ui/2010/UIModel/application/commands" xmlns:fragment="http://www.eclipse.org/ui/2010/UIModel/fragment" xmi:id="_fragment"> | ||
| <fragments xsi:type="fragment:StringModelFragment" xmi:id="_commands_fragment" featurename="commands" parentElementId="org.eclipse.e4.legacy.ide.application"> | ||
| <elements xsi:type="commands:Command" xmi:id="_command_launch" elementId="org.eclipse.terminal.view.ui.command.launch" commandName="Launch Terminal" description="Launch Terminal"/> | ||
| <elements xsi:type="commands:Command" xmi:id="_command_launchConsole" elementId="org.eclipse.terminal.view.ui.command.launchConsole" commandName="Launch Terminal" description="Launch Terminal"/> | ||
| <elements xsi:type="commands:Command" xmi:id="_command_launchToolbar" elementId="org.eclipse.terminal.view.ui.command.launchToolbar" commandName="Open a Terminal" description="Open a Terminal"/> | ||
| <elements xsi:type="commands:Command" xmi:id="_command_disconnect" elementId="org.eclipse.terminal.view.ui.command.disconnect" commandName="Disconnect" description="Disconnect Terminal"/> | ||
| <elements xsi:type="commands:Command" xmi:id="_command_newview" elementId="org.eclipse.terminal.view.ui.command.newview" commandName="New Terminal View" description="New Terminal View"/> | ||
| </fragments> | ||
| <fragments xsi:type="fragment:StringModelFragment" xmi:id="_handlers_fragment" featurename="handlers" parentElementId="org.eclipse.e4.legacy.ide.application"> | ||
| <elements xsi:type="commands:Handler" xmi:id="_handler_launch" elementId="org.eclipse.terminal.view.ui.handler.launch" contributionURI="bundleclass://org.eclipse.terminal.view.ui/org.eclipse.terminal.view.ui.internal.handler.LaunchTerminalCommandHandler" command="_command_launch"/> | ||
| <elements xsi:type="commands:Handler" xmi:id="_handler_launchConsole" elementId="org.eclipse.terminal.view.ui.handler.launchConsole" contributionURI="bundleclass://org.eclipse.terminal.view.ui/org.eclipse.terminal.view.ui.internal.handler.LaunchTerminalCommandHandler" command="_command_launchConsole"/> | ||
| <elements xsi:type="commands:Handler" xmi:id="_handler_launchToolbar" elementId="org.eclipse.terminal.view.ui.handler.launchToolbar" contributionURI="bundleclass://org.eclipse.terminal.view.ui/org.eclipse.terminal.view.ui.internal.handler.LaunchTerminalCommandHandler" command="_command_launchToolbar"/> | ||
| <elements xsi:type="commands:Handler" xmi:id="_handler_disconnect" elementId="org.eclipse.terminal.view.ui.handler.disconnect" contributionURI="bundleclass://org.eclipse.terminal.view.ui/org.eclipse.terminal.view.ui.internal.handler.DisconnectTerminalCommandHandler" command="_command_disconnect"/> | ||
| <elements xsi:type="commands:Handler" xmi:id="_handler_newview" elementId="org.eclipse.terminal.view.ui.handler.newview" contributionURI="bundleclass://org.eclipse.terminal.view.ui/org.eclipse.terminal.view.ui.internal.handler.NewTerminalViewHandler" command="_command_newview"/> | ||
| </fragments> | ||
| </fragment:ModelFragments> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,15 @@ | |
|
|
||
| <plugin> | ||
|
|
||
| <!-- E4 model fragment --> | ||
| <extension | ||
| id="fragment" | ||
| point="org.eclipse.e4.workbench.model"> | ||
| <fragment | ||
| uri="fragment.e4xmi"> | ||
| </fragment> | ||
| </extension> | ||
|
|
||
| <!-- Extension points --> | ||
| <extension-point id="launcherDelegates" name="%ExtensionPoint.launcherDelegates.name" schema="schema/launcherDelegates.exsd"/> | ||
|
|
||
|
|
@@ -309,104 +318,13 @@ | |
| </menuContribution> | ||
|
|
||
| </extension> | ||
| <extension point="org.eclipse.ui.bindings"> | ||
| <key | ||
| commandId="org.eclipse.terminal.view.ui.command.launchToolbar" | ||
| contextId="org.eclipse.ui.contexts.window" | ||
| schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" | ||
| sequence="CTRL+SHIFT+M3+T"/> <!-- Bug 435111: Don't use M1 since COMMAND+Option T already taken on Mac --> | ||
| </extension> | ||
|
Comment on lines
-372
to
-377
There was a problem hiding this comment. For me it looks like this should become a core expression instead of doing it "in code" @mickaelistria wdyt? @vogella @opcoach @ruspl-afed it seems handler in E4 can currently only use annotated code based enablement, should we allow core expressions as well? If yes please add an issue so we can track missing E4 features! There was a problem hiding this comment. Submitted a task now to support core expressions for handlers: There was a problem hiding this comment. @laeubi In e4 the handler can be assigned to the part directly, instead of "enabling it" whenever the part gets selected. If you look at the part in the model editor you see that the handler can be assigned directly to it. When Paul did redesign the handlers he wanted to get aways from the "all handlers are on the same level and you need to have additonal logic to make them available for only one part". There was a problem hiding this comment. Btw, I think it would be better to have an issue for discussion instead of lots of different PR with questions. There was a problem hiding this comment. @vogella PR are issues ... just in case anyways before the PR one can hardly know what issues arise. Can you provide an example how this should be handled to the migration docs for reference? There was a problem hiding this comment. @vogella thanks, I'm lloking into it, I assume the
would then be need to be implemented in code additionally so one can only "save" the check for the part itself? Also it means it can only be accomplished when migrating the part first... Also I'm a bit confused how to distinguish the case where There was a problem hiding this comment. IIRC the replacement for is the CanExecute and you should be able to inject properties into this method. You can check via the context spy. |
||
| <extension point="org.eclipse.help.contexts"> | ||
| <contexts | ||
| file="contexts.xml"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,16 @@ public abstract class AbstractTriggerCommandHandler extends AbstractHandler { | |
| * @param selection The selection to pass on to the command or <code>null</code>. | ||
| */ | ||
| protected void triggerCommand(String commandId, ISelection selection) { | ||
| triggerCommandStatic(commandId, selection); | ||
| } | ||
|
|
||
| /** | ||
| * Static method to trigger a command to be executed. Can be used from E4 handlers. | ||
| * | ||
| * @param commandId The command id. Must not be <code>null</code>. | ||
| * @param selection The selection to pass on to the command or <code>null</code>. | ||
| */ | ||
| protected static void triggerCommandStatic(String commandId, ISelection selection) { | ||
|
There was a problem hiding this comment. @opcoach @vogella @ruspl-afed we need a link to some documentation what is the correct E4 way to trigger commands so we can instruct copilote here to not use PlatformUI! At best this would be done by an enhancement to the https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/docs/Eclipse4_Migration.md file so it is discoverable by copilote in the future. There was a problem hiding this comment. I started a documentation task here now: There was a problem hiding this comment. @copilot the migration doc was enhanced to support how to programmatically call commands in E4: please adjust the code to use the E4 Command service instead of There was a problem hiding this comment. Updated to use E4 command services. QuickAccessHandler, MaximizeViewHandler, and NewTerminalViewHandler now inject ECommandService and EHandlerService instead of accessing ICommandService through PlatformUI singleton. Commit: The changes will be in the next commit. |
||
| Assert.isNotNull(commandId); | ||
|
|
||
| ICommandService service = PlatformUI.getWorkbench().getService(ICommandService.class); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i18n was lost