| Summary: | Run/Debug in the context menu | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Eugene M. Steinbacher <gsteinbacher> |
| Component: | Debug | Assignee: | Chris Tilt <chris> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P2 | CC: | andrea.aime, andy, ashackleford, darin.eclipse, Darin_Swanson, ebelisar, eclipse, erich_gamma, ftonello, jared_burns, pepblast, robvarga, who |
| Version: | 2.0 | ||
| Target Milestone: | 3.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | 11114 | ||
| Bug Blocks: | 50922 | ||
| Attachments: | |||
|
Description
Eugene M. Steinbacher
Darin here is the first context menu request. The properties menu no longer makes sense for "runtime properties", as there can be more than on configuration per class. As well, the properties menu is limiting, becuase you can actually launch programs that are external to the workspace using confiugurations (i.e. there may be no resource in the workspace with which to associate the properties). To change launch config properties, use the launch config dialog. It can be opened via the "Run..." and "Debug..." actions. The Run/Deubg As actions will launch the selection using the specified launch type. For example "Debug As -> Java Application". Marking as later, to consider adding "Run/Debug As" into the pop-up menu. *** Bug 25787 has been marked as a duplicate of this bug. *** I can see where having to parse the file to determine the contents of the popup menu may present some problems. As a first pass at this, perhaps the 'Run' items available in the toolbar dropdown could just be re-added to the context menu, as you mention in your 5-31 comment. As far as parsing is concerned, apparently some is already happening because the Outline view is decorated with the "runner" icon for classes that define a main(). Erich has filed bug 25963 as a reminder to also decorate classes containing JUnit Tests. It should be possible to use this to populate the context menu appropriately. -Which is exactly what JBuilder does. ;-) Re-opening for investigation in 3.0 - i.e. context menu launching. It would be nice to have Run/Debug available from the editor tab's context menu as well. *** Bug 42340 has been marked as a duplicate of this bug. *** From Bug 42340, consider as many as three options: 1. Run As->Type: Run with a default config of the chosen type. This would be easy to implement, but we have to consider whether it's worth an additional menu item (I think it is). 2. Run As...->Type...: Open the launch configuration dialog on a config of the chosen type (creating a new one if necessary). This would be like the current "Run Ant..." action. Same considerations as 1. 3. Run Like->Config: Duplicate the chosen config and replace the appropriate attributes for the selected file. This one is interesting, but we'd need to add a new mechanism if we wanted to make it work generically. Launch configuration type providers would have to be able to do the work of substituting the appropriate attributes. For local Java applications, we'd just replace the main type. We need to investigate if we can get context sensitive launching into the context menus. That is, when a java type with a main is selected, Run/Debug in the context menu should do the right thing - launch a local java app. This needs some exploration to see if we can support such a feature efficiently (i.e. determine which run/debug actions are applicable without having to parse the entire workspace). *** Bug 43803 has been marked as a duplicate of this bug. *** Proposal: Currently, launch shortcuts (ILaunchShortcut) are replacements for run/debug in the context menu. Determining which launch shortcuts were applicable to a selection was problematic, so we forced the user to manullay choose a launch shortcut from the cascading run/debug menus, for a seleciton/active editor. JDT is experimenting with an "expression evaluation" language that provides more power to extensions (contributions) that determine when a contribution should be enabled/visible. The expressions are coded in plug-in XML as part of the contribution/extension. It is hoped that this support will be pushed down into the platform such that all extensions can leverage this support. The debugger could use the support to determine what launch shortcuts are applicable to a selection/active editor, thus providing run/debug/profile shortcuts in the context menu. Some issues still exist: * If more than one launch shortcut is applicable for a selection, the user must choose how they want to launch the selection. For example, when a JUnit test is selected that has a main method, the class can be lanuched as either a "Java Application" or a "JUnit Test". I believe these types of collisions are rare (usually, there is just one way to launch a selection). * If a container (project/package/folder) is selected, the potential executables are unknown without searching (which cannot be done in order to display a context menu). * Clients may want to change the label of the actions. For example, usually "Run" is a good description, but in the case of testing a web-site, something like "Publish & Preview" might be a better label. Why don't we just mimic what's available from the Run menu? We could just put the "Foo As->" items in the menu. We'll need to have a cascading menu even with filtering because of cases where multiple config types are valid for an extension. The expression language allows us to use more than file extension to determine what launch shortcuts are avaiable for a selection. For example, we can determine if the selection is a compilation unit, has a main method, is a subtype of applet, etc. Thus, we should have minimal collisions. I think we do want to have the cascade menu when there are collisions, and this could also be used for containers. For example, when a package is selected, the "Run As ->" menu appears with possible options. I'm just afraid that this filtering could end up being expensive for a pop-up menu. If it's simple and fast, then it would be great to have. When we get around to implementing this, I think we should use this mechanism as a filter. If someone specifies a filter in their config type extension, that will allow us to remove shortcuts that we know are invalid. If they don't specify a filter, we should add their type for backwards happiness. *** Bug 50319 has been marked as a duplicate of this bug. *** Here is a proposal for the design. Comments are welcomed. It would be great to simply add a couple of additional properties to the existing LaunchShortCut extension and build choices dynamically. However, object contributions do not support dynamic items. (See Nick's comments in the UI-DEV archive http://dev.eclipse.org/mhonarc/lists/platform-ui-dev/msg01522.html). Thus, I propose we contribute a base contextual "Launch" menu and its two sub-menus "Run As" and "Debug As" from o.e.d.ui. An abstract class, something like AbstractContextualLaunchAction, will do the heavy lifting and sub-classes for Java, Applet, JUnit, and others can provide a couple of additional arguments to denote the run/debug mode, as well as a filter for use in the XML expression language for discovering visibility based on type and main/test methods, etc. Plugins can then contribute their contextual launch actions, within XML "visibility/enablement" expressions, that sub-class the AbstractContextualLaunchAction and extend (via XML) the "Launch", "Run As", or "Debug As" menu groups. This also allows maximum flexibility for using action labels other than "Launch-->Run As", e.g. "Launch-->Publish and Preview". AbstractContextualLaunchAction will do the work of consulting the LaunchConfigManager and either launching an existing config type or creating a new one with default parameters (some of this may need to be provided by the sub-class). The idea is to leverage the existing launch short cut extension. If there is a conflict (multiple launch types are appropriate), then a dialog will be presented that allows the user to choose. We can not make the choices available in the pop-up menu because of the reason stated initially. As much of the existing LaunchAsAction, and its helpers, will be used as possible. This should be efficient at run-time because the XML expr filters are only loaded if the plugin is loaded. Unfortunately, if the plugin is not loaded, the tri-state logic will enable the action and we will discover (after the launch is attempted) wether there is actually an appropriate launch type for the selected resource. This seems like an acceptable tradeoff. Since we already have Run/Debug As menu items in the Run menu (and drop-down hitstory menus), I would prefer not to add similar actions in their own "launch" menu. (Note: we have tried to avoid the use of the word "launch" in the UI - I think it only appears in a few places). The goal is to provide context menu launching, so top level menu items are not ideal. The lack of cascading menu object contributions is a definite limitation. I need to think about what else we might try. Why don't we just ask the Platform UI to add support for dynamic context menus? OK, after some discussion, it seems that we are stuffed without the dynamic menu support. The reason is that we only want to show launch options if they are relevant. Because the specific shortcut types are contributed in different plugin files, the XML expression feature does not evaluate in a wide enough scope for us to detect wether there are appropriate shortcuts and thus to provide menu contributions for RunAs and DebugAs. The same problem exists if we try to push everything to the top-level pop-up menu. For cases where there is zero or one applicable shortcut, it's ok. But when there are more, we can not know - the scope spans multiple contribution files. Thus, we can't collapse the multiple choices into a dialogue box. This means we might populate the pop-up with several shortcuts, overwhelming the user. So, I'll claim that we really need dynamic content in pop-ups before we can continue. Another proposal. Thanks for the feedback. More is welcomed. This assumes that we add some dynamic pop-up menu support. Contributor Experience: Contributors of launch shortcuts would add two more properties to their plugin.xml. The context menu will then show applicable Run/Debug/Profile actions based on loaded plugins (i.e. if the supporting plugin isn't loaded, the launch short cut isn't shown in the context menu). Conextual launch actions have meaningful labels such as "Run" for Java Applications, "Run on Server" for JSP files, "Run Unit Test" for java files with JUnit entry points, etc. This strikes a balance for performance and customization. See notes below for details. Specifically, the launchConfigurationType extension would accept optional properties "contextLabel" and "filterClass". ContextLabel provides the meaningful action label, though it is not dynamically computed at run-time. FilterClass specifies the fully qualified name of the java class that implements org.eclipse.ui.IActionFilter, with the selected object as the target of the testAttribute() method call. If the plugin that supplies the filter class is not loaded, it will not be loaded automatically, and the shortcut will not appear in the context menu. Implementation Thoughts: Assuming we can get some API for dynamic content in a pop-up, we should build generic support for run/debug/profile in our debug plugin. Since we already have a launch shortcut extension point, this is the best place to add the new properties; then we can just consult the launch configuration manager and build appropriate context menu additions. Debug plugin would add a single menu group to the "additions" group of the context menu where it will populate (dynamically) all applicable (filter class returns true) launch shortcuts for loaded plugins. I am not clear on the issues of persistence, but worst case, we will forget which shortcuts are applicable until they load the supplying plugin. This seems ok because the workbench will persist the state of the views and the right plugins should still be there. Maybe it means doing a little extra work at run-time, but not much I don't think. Nick wrote: "The simplest approach would be to add pulldown="true" support to objectContributions, and have it work the same way as for action sets. The class would have to implement IWorkbenchWindowPulldownDelegate2 in order to get the getMenu(Menu) call." To clarify, I think Nick meant to add style="pulldown" support to the action element of an objectContribution in the pop-up menu extension point. I don't know if this will let us populate our launch shortcuts directly in the top level of the context menu. I need to poke at it some. But, worst case, we will have a single pull-right entry that has all applicable short cuts. This will make for a clean context menu that is still a good performer. Created attachment 7595 [details]
Experimental Code, part one
Created attachment 7596 [details]
Experimental Code, part two
Created attachment 7597 [details]
Sample project to easily test the JavaApplication and JavaApplet context menus
This example project has a few java classes that contain combinations of Main
methods, and Applet extensions, and nothing, to test the contextual launch
pop-up actions.
Darin (w), Please apply these patches and try the example project files in your target Eclipse. This prototype follows my latest proposal for implementation. Please note the new "filter" elements that are now allowed in the shortcut extension specification. I get error log entries that complain about not being able to find an executable extension for the other shortcuts that haven't been updated yet (JUnit, etc.). I don't know how to avoid these message yet, but I will correct that. Otherwise, it feels pretty clean. Your comments are much desired. Cheers, Chris Created attachment 7621 [details]
Part one of (semi-final) release
Created attachment 7622 [details]
Part two of (semi-final) release
Darin(w),
These two patches are applied to their respective projects (as named). They
implement the complete set of changes we discussed on the phone, namely:
o shortcut only appears on sub-menu if it's plugin is loaded. No forced
activation
o filters are only run once for each shortcut, not per shortcut per mode
o Pair has been moved to a non-API project
o "contextLabel" elements allow specific labels for each mode(.e.g. run,debug)
o modes are discovered dynamically and not assumed to be run/debug/profile
o copyright messages and class/method documentation added
Please verify and add to HEAD so that I can make patches for the other teams
(JUnit, etc.) that have shortcuts. Thanks. -Chris
Released code to HEAD. Here are some comments/issues: * When there are no potential launch shortcuts applicable to a selection, we should show a single disabled action in the cascade menu saying so (i.e. "None Applicable", or something of the sort). This avoids an empty cascade menu, that the user does not expect. * While using the support, I thought it would be neat for the "java" launch shortcut labels to say what they will actually launch. Not sure how dynamic we can be, but something like "Debug MyApplet" instead of "Debug Java Applet" would be neat (i.e. substitute type name for launch config type name). This would likely require that the launch shortcut be queried for a label. Could we add this as an option? Thanks. I will address your issues.
* I built a prototype that added a FakeAction (I think the code is still there, but commented), but
my problem was how to get locale text from code. Can I query the properties file for a name/value
pair? In general, I agree that this feature is desirable and planned on adding when I can achieve the
above.
* Making the launch label in the context menu be dynamic is interesting, but has issues. First, the
contextLabel element already lets the shortcut provider specify the label for each mode. It's
important to contributors to be able to control the label, so we can't just use the type name.
Further, we don't always want to assume the mode {run,debug,profile} should appear in the label. I
would rather not add another class entry point to query for the label; the launch shortcut
extension is getting ugly already. However, we might make the label text, that appears in the
property file, capable of simple formatting. We could recognize $T for type name, etc. Of course,
"%" is already meaningful in the properties file. Other variables could be supported as needed.
Notes: * Property files are translated to other languages - so that is the mechanism for internationalization (no need to worry about this one - just use a label that is externalized, and all will be well). * The dynamic label could be an option, and should not be Java/type specific. You could add an attribute to the contextLabel node like "delegate", which can be used instead of a static "label". The delegate would implement an interface responsible for creating a label for a selection/mode. However, this is delux. Chris, I noticed the following problem (bug?) with the current support: * Open a new empty workspace * Create a Java project, with a new "main" type. * Select the associated compilation unit in the package explorer and attempt to run the new main type from the context menu. * The "Run" cascade context menu remains empty. I assumed this was due to the fact that the Java debug plug-in was not yet activated. However, I then ensured the plug-in was activated by opening the launch config dialog and manually creating a launch config for the new type. I then returned to the context menu in the package explorer, but the cascade was still empty. *** Bug 51074 has been marked as a duplicate of this bug. *** Darin, I tried to reproduce the empty workspace issue you mentioned and was unable to, both with HEAD code and with my latest patches. To be clear, the main method must have the complete signature of "public static void main(String[] args)", although I suspect you ensured that. Don't what else to try right now. -Chris Created attachment 7723 [details]
Part one of final release
Created attachment 7724 [details]
Part two of final release.
These patches address all known issues, including:
o Extraneous ErrorLog entries when filterClass not provided by shortcut(s)
o classFilter implements ILaunchFilter instead of IActionFilter
o class cast handling improvements
o code reduction/elimination
o javadoc
Chris, I released the updates. Is there anything we can do to get rid of the cascade arrow when there are no entries? Or perhaps disable the action? Darin, Thanks. I don't know of any way to get rid of the Run action, assuming that we want to be lazy in our search for applicable shortcuts. To remove the Run action, we have to evaluate all of the shortcut filters. Caching would help, but we'd increase the complexity quite a lot, adding listeners for every resource change event to invalidate our cached entries. Presently, the filters are not evaluated until the user hovers on the Run action so that menu traversal is fast until they explore Run options. It would look stupid to remove the Run action *after* we evaluate the filters since it would reappear the next time they right-click on the resource. If the filters were fast, we could eagerly evaluate them and hide the Run action, but we'd pay the price every time the context menu was raised. I am afraid that would be expensive when several shortcuts are loaded and we run all of their filters. It's worth trying, but I'm tempted to remain lazy. Do you think otherwise? Cheers, Chris Darin W., can you please verify. If there are more features to add, I suggest we open a new bug. The most important functionality seems to be working well. Verified. The XML team has a requirement for multiple-selection support for the Run context menu. In the scenario for running or debugging XSL Transformations, one of the expected user gestures is the following: The user selects an XML resource _and_ a XSL resource and chooses to Run > Run XSL Transformation. The launch configuration can then take the two selected resources as input into the launch. Should I open a new Bugzilla to address this requirement? We have an open bug to address multi-selection launching - bug# 51506. *** Bug 39807 has been marked as a duplicate of this bug. *** |