| Summary: | Contextual Launch should use XML Expression Language | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Chris Tilt <chris> | ||||||||
| Component: | Debug | Assignee: | Darin Wright <darin.eclipse> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P2 | CC: | darin.eclipse, Darin_Swanson, dirk_baeumer, erich_gamma | ||||||||
| Version: | 3.0 | ||||||||||
| Target Milestone: | 3.0 M8 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 51506, 54717 | ||||||||||
| Attachments: |
|
||||||||||
Changing this to P2 because we need to settle on the API before M9. I think this is a good direction, however XML Expression language does not readily solve the other problem of wanting to have some filtering *before* the supporting launch shortcut is loaded. The nameFilter, which was added to the LaunchShortcut extension was done so for the ability to specify some filtering that would allow shortcuts to appear on the Run menu before loading the plugin. Failure to do so is an obvious user trap - you can't launch until you've launched once the old-fashioned way. Please see bug #51420 If we can get the platform to provide a name filter expression tester, then we avoid agressive plug-in loading? I.e. if the porperty tester is defined in a public way in a lower level plug-in? Darin, that's a good idea. I was planning on puttin my nameFilter expression tester in the contextual menu support code, but it would be better if it were generally available. I will first implement it in my context menu code and when it is well-tested and committed to debug, we should offer it to a platform plugin. Created attachment 8516 [details]
Patches org.eclipse.debug.ui
Replaces previous context menu enablement logic with new XML Expression
language. This will require plugins to adapt to the new extension point
grammar. Patches will be provided to JDT/debug-ui and JDT/JUnit.
Jared, please apply patch and request for one of the Darins to verify. Thanks. Verified and committed Chris' contribution. Changes to: org.eclipse.debug.ui/schema/launchShortcuts.exsd - updated schema for <enablement> support ResourceExtender - type extender to test properties ContextualLaunchObjectActionDelegate - use the new support to populate the context menu LaunchShortcutExtension - provide access to the new extension point elements plugin.xml - provides property tester for "matchesPattern", "projectNature", and "canDelete" Can't verify without the Java shortcut contributions - without it, the context launch menu is always empty :-) Darin, if I can ever get the system to build again, I will post the patch. There are so many changes since integration build; it's taking me hours just to get my workspace compiling again :-( These are the projects I have from HEAD: org.apache.ant org.eclipse.ant.core org.eclipse.ant.ui org.eclipse.core.variables org.eclipse.debug.core org.eclipse.debug.ui org.eclipse.jdt.debug org.eclipse.jdt.debug.tests org.eclipse.jdt.debug.ui org.eclipse.jdt.launching org.eclipse.ui.console org.eclipse.ui.externaltools Created attachment 8540 [details]
patches org.eclipse.jdt.debug.ui
This patch applies most of the required changes to replace the enablement logic
for Run in the context menu. There was something that broke the implementation
of the <test property="org.eclipse.jdt.ui.hasMainType"> since yesterday. I've
commented that test out so that we can finish verifying the first part of the
work. I will pursue getting the last property test finished once these changes
are applied to HEAD.
The last required patch will provide the typeExtender additions in jdt.ui.
Darin, as my notes in the patch describe, there was something that broke my solution since yesterday. I will investigate. In the mean time, please apply the patch. It will mean that Run-> will not work for Ant, JUnit, or any other shortcuts that don't have the new expression language support yet. Once we get this stuff in HEAD, I will also try and make patches for them. Thanks, Chris Entered bug#54705 to resolve missing propertyTester. Will continue once I hear from Dirk on that one. Created attachment 8542 [details]
Replaces previous patch for jdt.debug.ui
I have moved all launch type extender support to jdt.debug.ui and used an
IResource instead of IFile. This is more generic and works for both .class and
.java files too. Both isApplet and hasMain are supported from the same place
and this also eliminates dependency on jdt.ui. The mysterious problem with the
missing propertyTester is also gone, but I don't know why.
This should complete the work for this bug. I will post new PRs to mod the
JUnit and Ant shortcuts.
Darin, you should everything you need to verify. Pleaes let me know if otherwise. Thanks, Chris Removing dependency on XML expr bug#54705 since I have worked around it. Also, it may not even be a bug. Applied and released the jdt debug ui patch Changes to JavaAppletLaunchShortcut, JavaApplicationLaunchShortcut and the addition of ResourceExtender. Changes to the plugin.xml. Verified |
There is now available the XML Expression Language in plugin declarations. The Contextual Launch (Run Context Menu Item) should use this support to minimize the duplication of Filtering that is becoming apparent with the addition of refactoring support. Erich made some good points, which are pasted below... [Erich G. wrote...] I'm just experimenting with the expression language (org.eclipse.core.expressions) support and I notice that there are parallels between the expression language and the contextual launch. In particular as a client I will have to implement two different but very similar mechanisms when I want to participate in both contextual launching and in refactoring for example. For contextual launch you define a file name pattern plus a property that is evaluated by ILaunchFilter. To participate in refactoring you define an IPropertyTester (a generalization of ILaunchFilter) plus you can define a general expression when a particpant should be involved (and, or...). Here is an example from the JUnit plug-in: 1) the definitioin of the property that will be tested <extension point="org.eclipse.core.expressions.propertyTesters"> <propertyTester properties="isTest" type="org.eclipse.jdt.core.IType" class="org.eclipse.jdt.internal.junit.ui.JavaTypeExtender" id="org.eclipse.jdt.junit.ITypeExtender"> </propertyTester> </extension> Its implementation: public class JavaTypeExtender extends PropertyTester { private static final String IS_TEST= "isTest"; public boolean test(Object receiver, String method, Object[] args, Object expectedValue) { IType type= (IType)receiver; try { if (IS_TEST.equals(method)) return TestSearchEngine.isTestOrTestSuite(type); } catch (JavaModelException e) { return false; } return false; } 2) the expression, which states that I want to participate when the selected element is an IType and the isTest property returns true (default is an And connection) <instanceof value="org.eclipse.jdt.core.IType"/> <test property="org.eclipse.jdt.junit.isTest"/> The implementations of the isTest property and the ILaunchFilter are very similar. Here is the implementation of ILaunchFilter. public boolean testAttribute(IResource target, String name, String value) { if ("ContextualLaunchActionFilter".equals(name)) { //$NON-NLS-1$ if (target != null) { IJavaElement element = JavaCore.create(target); if (element instanceof ICompilationUnit) { ICompilationUnit cu = (ICompilationUnit) element; IType mainType= cu.getType(Signature.getQualifier( cu.getElementName())); try { return TestSearchEngine.isTestOrTestSuite( mainType); } catch (JavaModelException e) { return false; } } } return false; } When I compare the two mechanisms I see: 1) contextual launch is simpler 2) contextual launch is more limited you can only define a fixed expression: file name pattern and a single property 1) is nice but 2) can become problematic, in particular when I see the requirement to support a launch from a multiple selection. A file name pattern plus one property might not be sufficient. This means as you make the contextual launch more general you will get more similar to the expression language. I understand that these two mechanisms evolved together and it was a wise move to not track the expression language as it was still evolving. However, we should revisit whether the two different but similar mechanism are really needed.