Community
Participate
Working Groups
Build ID: M20071023-1652 Steps To Reproduce: Trying to use the org.eclipse.ui.menus extension point from the platform, to contribute custom commands to the XML menu when the XML Editor is open, does not seem to work. According to the org.eclipse.ui.menus extension point it allows contribution to any menu on the main menu that a plugin may create. I can successfully add options to the File menu, as well as to the main menu itself, however adding optons to the XML menu never seem to take. There doesn't appear to be any error message returned in the logs either. The id, I've tried adding the option to are: org.eclipse.core.runtime.xml.design.xmlmenu The following is a sample test contribution using the extension point: <extension point="org.eclipse.ui.menus"> <menuContribution locationURI="menu:org.eclipse.core.runtime.xml.design.xmlmenu"> <command commandId="org.eclipse.ui.help.aboutAction" icon="icons/xmldoc.gif" label="Validate" tooltip="Validate"> </command> </menuContribution> </extension> This should, if I've identified the correct ID for the XML menu, add a Validate option, that just brings up the About dialog box. I've tried this with 3.3.1.1/wtp 2.0.1 and 3.4 and WTP 3.0. Neither work. Figure I would report it here first, and if it needs to be moved to platform ui, then it can be moved. More information:
I'll let the experts answer more directly, but at one point there was a desire to get rid of the 'XML' menu item ... it was inconsistent with other language treatment in Eclipse. If I recall right, it was put back in to work around some other limitation, but ... you might also consider if you really need something there ... what if it disappeared? Would you have a suggestion for more consistent approach to handling XML specific functions? Perhaps the real problem to solve?
Personally, I have no problem if it is removed. I can contribute the function command I wanted either to the Project, or Edit menu options. Right now, I could easily see the XSD, XML, and WSDL Editor menu options being removed. The XSD menu option just contains Zoom In/Zoom Out options and an Export Image option. Zoom In/Zoom Out could technically go into the Navigate menu, or the Edit Menu. Export Image should go into the standard export option, along with keeping the toolbar item. The XML options for reloading dependencies, and could go into either Edit or Source. Most of this could be controlled by using the org.eclipse.ui.menus extension point and appropriate handlers to know when to make the options visible and when to not have them visible. The actionSets, popmenu, viewActions, and editorActions are/or will be deprecated and the new recommended way to do menus, toolsbars, and other contributions is with the org.eclipse.ui.menus extension point. This means that most of the WTP UI plugins will need to be refactored eventually to be used, as the older methods will eventually disappear (probably in eclipse 4.0 from what I've been reading). You probably already have the UI guidelines resource link as well as the Menu Contributions link, but I'm putting them here for reference. http://wiki.eclipse.org/User_Interface_Guidelines http://wiki.eclipse.org/index.php/Menu_Contributions Just in the User Interface Guidelines for IDs, there would need to be some refactoring. As the way WTP does the IDs and where they are stored isn't that consistent with the UI guidelines (took me most of the day just to find where the xmlMenu id was stored and used to create the menu).
The XML menu is currently created programatically in org.eclipse.wst.xml.ui.internal.tabletree.XMLTableTreeActionBarContributor. The menu created has an id of"org.eclipse.core.runtime.xml.design.xmlmenu" As David said, the XML menu was somewhat added as an afterthought, and it was not meant to be extensible/contributed to which was why the xml menu id was so hidden. None of the menus/actions created by SSE are currently using the new menus extension points yet. In the past, when I've investigated moving over to the new extension point, I ran into problems making sure our actions appear and are enabled when in the source editor of the appropriate content type and disabled/hidden when not in the source editor or not in the expected content type. The main issue was with other editors that that included one of our source editors in their multipage editor. Moving to the new menus (and handlers) extension point is something we would really like to do. Especially since more problems are surfacing with us using the old actions framework (like bug 190694 & bug 199730. But we just haven't had the time/manpower to do so.
Thanks, Amy. Has the platform team been informed about the issues you have run into with the org.eclipse.ui.menus and controling it through the various handlers so that they are enabled for the appropriate content types? From what I have read in the UI guidelines and it appears that we could move the items to other points. I'd be willing to spend some cycles on this as I think it'll be good for the XDT project to start off using the new extension points, the key though is that the underlying framework should be upgraded at the same time. We could do it in iterations to make sure that we get portions working correctly first and then move it out to the overall framework as we go. If somebody wants to be a reference point for me on this, I'd be willing to at least take a stab at it. It would be a good learning experience anyways for me and help get me more familiar with the underlying code. Any issues I'd run into that were caused by the org.eclipse.ui.menus extension point, I'd make sure were open against the platform so that they could be addressed.
(In reply to comment #4) > Thanks, Amy. Has the platform team been informed about the issues you have > run into with the org.eclipse.ui.menus and controling it through the various > handlers so that they are enabled for the appropriate content types? Yes, I should've clarified my statement about past investigations. The last time I investigated moving to the command/handlers/menu framework was probably around Eclipse 3.2. At that point, the framework was still new and no one was really using it (even now, I'm not sure how much of the base is really using it) and I don't think the menus extension point was API yet. I did raise any issues I had with the Platform team, and I think they've responded to a good deal of them since then from the little bit of investigation I did of the new framework over the weekend. I think now, it's just a matter of figuring out the right core expressions to use. > If somebody wants to be a reference point for me on this, I'd be willing to > at least take a stab at it. It would be a good learning experience anyways > for me and help get me more familiar with the underlying code. Sure, that'd be great. I can be your reference point. But are you talking about taking a stab at: a) moving the XML, WSDL, XSD menu items to a more appropriate location? b) converting the XML, WSDL, XSD editor actions to the command/handler/menu framework + a) c) converting all SSE editor actions to the command/handler/menu framework (and moving any items that need to be moved)
(In reply to comment #5) > > If somebody wants to be a reference point for me on this, I'd be willing to > > at least take a stab at it. It would be a good learning experience anyways > > for me and help get me more familiar with the underlying code. > > Sure, that'd be great. I can be your reference point. But are you talking > about taking a stab at: > a) moving the XML, WSDL, XSD menu items to a more appropriate location? > b) converting the XML, WSDL, XSD editor actions to the command/handler/menu > framework + a) > c) converting all SSE editor actions to the command/handler/menu framework (and > moving any items that need to be moved) Actually, all three. But I think we need to break it down into small chunks, test it out and then move on to the next piece. The long term goal would be to at least have the WST base items, moved into the new menu extension and tool bar contribution mechanism. The same with the context menus and popmenu contributions. I'll proceed how you and the wtp team think it would be best to do so.
Alright then, let's get started. In sse.ui, we already have some commands and bindings for those commands set up. We also have a context defined for the bindings to be active in SSE editors. In xml.ui, we currently have some editorActions defined that say what the SSE commands should do when you invoke them in an SSE editor with XML content. First step should be to turn these editorActions into handlers. These handlers should be active/enabled when you're in an SSE editor with XML content. Step after that would be to define the appropriate menu items for these commands/handlers (this is currently done in the ActionContributor classes defined in the editor extension point). The menu items would be visible as long as you're in an editor that contains an SSE editor with XML content type, but they would only be enabled when the SSE editor is the active page. This part may be tricky. Another tricky part may be setting the Source menu up correctly. After that, you're practically done. Just repeat with any other actions/SSE supported content type.
Okay, thanks, Amy. I'll see what I can do over the next couple of weeks. I'll start at the SSE.ui and xml.ui. I'll post patches here to this bug for review.
Created attachment 85240 [details] Proof of Concept for SSE UI This is a proof of concept attempt for the SSE UI. 1. Disabled the programmatic menu contribution for the SSE, and XML Editors. 2. Started with Source main menu contribution. id = sourceMenuId 3. Added Toggle Comment, Add Block Comment, Remove Block Comment, Shift Right, Shift Left, Format, Format Active Element to the Source Menu. 4. Added Smart Insert Mode based on the platforms editor insert mode command. 5. Added an "additions" group so that other editors and plugins can contribute commands to the menu. this is done by using the following menu uri: menu:sourceMenuId?after=additions 6. Gave each menu item it's own id in the menu, so that additions can also specify a particular command to add after. Some of the commands work automatically like Format, Format Active element, Shift Right, and Shift Left. The Source menu at the momement is always active, so will need to put in a <visibleWhen> condition and handler for that to have it only display the menu when the an SSE editor is active. The comment related commands aren't enabled but do appear in the menu, these may need their own handlers. Extensibility: The menu is full extensible so that other plugins can add to the menu, as well as the editors. They just need to contribute to the sourceMenuId menu.
Created attachment 85305 [details] Tweaked Patch that correctly activates only when the XML Editor is opened. This patch tweaks the org.eclipse.ui.menus contribution so that it only activates when the XML Editor is opened. This is how it works. 1. The SSE UI still contributes the Source menu with an id of sourceMenuId. 2. The sourceMenuId is empty of any commands but has seperators that are included with ids of sourceBegin, sourceEnd, shiftBegin, shiftEnd, additions 3. SSE UI defines the default commands to be added that affect all SSE based editors. These commands can be referenced by their id from other plugins to be added to the menu if it meets their needs. The sole purpose of the SSE UI is to contribute the base menu which other plugins can add their contributions. It is left intentionally blank. As stated, the base commands that work with any SSE UI should be defined in the SSE plugin with the appropriate commands. Because there are no commands contributed to the menu, it will not display. To get Editor specific commands, and to have the Source menu only display when the editor itself is active (i.e. has been opened or receives active focus), the use of the <visibleWhen> and org.eclipse.core.expression.definitions extension point is used. 1. define an org.eclipse.core.expressions.definitions extension point with the following information: <extension point="org.eclipse.core.expressions.definitions"> <definition id="org.eclipse.wst.xml.ui.xmlSourceContributions.definition"> <with variable="activeEditorId"> <equals value="org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorPart"></equals> </with> </definition> </extension> This sets a defintion to check that the activeEditorId is the XMLMultiPageEditor. This definition is referenced later by the commands. 1. define an org.eclipse.ui.menus extension point with the following locationURL: menu:sourceMenuId?after=sourceBegin 2. add commands for Toggle Comments, Add Block Comments, Remove Block Comments 3. For each of the commands that have been added, add an <visibleWhen> element with a subelement of <reference> where the definitionId points to the previously defined defintion id. The above adds the commands to the sourceMenuId, and only has them be displayed in the Source menu if the XML Editor is the activeEditor, if it isn't the active Editor, the commands become hidden, and if all commands are hidden, then the Source menu itself is hidden.
I've started a wiki page entry to keep track of the various ids, that are used through the various extension points that contribute to the ui. http://wiki.eclipse.org/Table_of_WTP_IDs Also included are links to various resources for working with the org.eclipse.ui.menus extension point, commands, and handlers.
Created attachment 85343 [details] Shows and Hides Menu options based on Active Contexts This patch leverages the ability to check the activeContexts to determine when commands can be enabled or disabled, or even shown. If implementing a MultiPage Editor that needs to contribute commands to a menu and only have them active when the appropriate editor has focus, it is suggested to use a Context for that editor and then check it. This patch checks for the org.eclipse.wst.sse.ui.structuredTextEditorScope for determing when particular commands are to appear in the Source menu. If the expected context is not active, then the command does not appear. This also allows the menu to only show relevant commands for the particular editor. i.e. Don't show Toggle Comments if in the Design editor, but do display it if in the Source Editor. The Core Command Expression language is used to check the contexts.
A general UI related question. Currently the XML editor, populates the Occurrences In File command in the Source menu, where the Java Editor puts the equivalent command in the Search menu. I'm wondering if we should move this command to the Search menu with this refactoring?
Created attachment 85359 [details] Implements all existing XML Editing Source menu commands. This patch implements all ActionDelgates that were contributing to the Source menu, as Handlers. The handlers are enabled or visible only when it is appropriate. As was stated in prior comments, the key to controlling is the activeContext, and checking that the appropriate one is available. A couple of further tweaks may have to be made to the checks for enablement of the handlers. Will probably need to check the SSE Context as well as either the XMLMultiPageEditorPart class or a specific context or content-type for XML is the Active XML Editor. This would allow for finer grain control when these commands are active so that a 0 or 1 handler relationship may be maintained. Items left to do: 1. Get Generate->XML option working on popup menus that use an IFile resource selection. 2. Remove current contribution for the general TextEditor popups, and implement with the new commands. Questions in regards to the text Editor Popups. The current SourceEditor popup, doesn't make available all of the Source editing commands. The Java Editor makes these available under a Submenu called Source. For consistency sake, should we implement these there as well? With the org.eclipse.ui.menu extension point this is a fairly simple matter of adding them to the appropriate popup, and there wouldn't need to be additional java code to implement. Overall, once you figure out the pattern, using the new menu extension point does reduce the amount of java code that has to be written to initialize menu structures, and to control when they appear. All current Source menu functionality seems to be able to be implemented using the new extension point.
Wow. That was pretty fast. I haven't had a chance to look over your patch and wiki page (and I probably won't be able to till January/M5), but I can address some of the questions you asked in this bug. (In reply to comment #13) > A general UI related question. Currently the XML editor, populates the > Occurrences In File command in the Source menu, where the Java Editor puts the > equivalent command in the Search menu. I'm wondering if we should move this > command to the Search menu with this refactoring? Yes, it sounds like the Occurrences in File command should be moved to the Search menu. I believe we had some issue in the past where we couldn't place it in that menu, or we had to make our command different than the base's because of keybinding conflicts, or some other possible outdated reason. But if you can't seen any problems with moving it there, then it sounds like it should be moved there. (In reply to comment #14) > A couple of further tweaks may have to be made to the checks for enablement of > the handlers. Will probably need to check the SSE Context as well as either > the XMLMultiPageEditorPart class or a specific context or content-type for XML > is the Active XML Editor. This would allow for finer grain control when > these commands are active so that a 0 or 1 handler relationship may be > maintained. I would rather not have to check for a specific part or editor. But rather for an SSE type editor and a specific content-type. Having to check for a specific editor/part limits the ability for other adopters to pick up these commands if they add the SSE editor to their multipage editor. > Questions in regards to the text Editor Popups. The current SourceEditor > popup, doesn't make available all of the Source editing commands. The Java > Editor makes these available under a Submenu called Source. For consistency > sake, should we implement these there as well? With the org.eclipse.ui.menu > extension point this is a fairly simple matter of adding them to the > appropriate popup, and there wouldn't need to be additional java code to > implement. My hesitation with this would be that Format & Cleanup would have to be moved into that submenu, making them more hidden. But I guess that wouldn't be too bad. I believe the items missing are the shift & comment commands. As long as the commands still work on the current selection, it sounds fine. So sure, create a Source submenu.
(In reply to comment #15) > Yes, it sounds like the Occurrences in File command should be moved to the > Search menu. I believe we had some issue in the past where we couldn't place > it in that menu, or we had to make our command different than the base's > because of keybinding conflicts, or some other possible outdated reason. But > if you can't seen any problems with moving it there, then it sounds like it > should be moved there. I'll get that moved into the Search menu. I don't see a problem moving it there. > > (In reply to comment #14) > > A couple of further tweaks may have to be made to the checks for enablement of > > the handlers. Will probably need to check the SSE Context as well as either > > the XMLMultiPageEditorPart class or a specific context or content-type for XML > > is the Active XML Editor. This would allow for finer grain control when > > these commands are active so that a 0 or 1 handler relationship may be > > maintained. > > I would rather not have to check for a specific part or editor. But rather for > an SSE type editor and a specific content-type. Having to check for a specific > editor/part limits the ability for other adopters to pick up these commands if > they add the SSE editor to their multipage editor. Well, this can best be controled through the use of Contexts. Currently the SSE editor sets a StructuredTextEditorScope contenxt which is perfect for working with the XML editor and being able to check the enablement. We should easily be able to add a check on the content-type as well to make sure that the XML editor functions are only enabled through the WTP editor. A clarification on how it works is that it relies on commands and handlers. The commands being the generic definition, and then the handlers being the implementation. Only 0 to 1 handlers may be active at any given time, so by controlling when the handlers are active will allow adopters to add their own extensions for the existing commands, or reuse the commands/handlers we have already defined. > > > Questions in regards to the text Editor Popups. The current SourceEditor > > popup, doesn't make available all of the Source editing commands. The Java > > Editor makes these available under a Submenu called Source. For consistency > > sake, should we implement these there as well? With the org.eclipse.ui.menu > > extension point this is a fairly simple matter of adding them to the > > appropriate popup, and there wouldn't need to be additional java code to > > implement. > > My hesitation with this would be that Format & Cleanup would have to be moved > into that submenu, making them more hidden. But I guess that wouldn't be too > bad. I believe the items missing are the shift & comment commands. As long as > the commands still work on the current selection, it sounds fine. So sure, > create a Source submenu. > The JDT already has the Format and Cleanup in the Source content submenu of the popup so it wouldn't be that big of a change, and the Short cut key bindings could still be used to access the methods. I'll prototype it out so that we can see what it would look like, but I think it brings more consistency to where the items are, especially with the possibility of the new refactorMenuId menu that I've added for the SSE for refactoring commands to be contributed too. Currently there are no menu options besides what is in the XSD design view for refactoring support. Again, modeling off of what JDT has done, this is available through a menu, and follows accessibility guidelines as well. Keith mentioned renaming the XML, XSD, and WSDL menus to Design as this is what they really represent and are active for the Design views typically. I'll look at incorporating that as well in the next set of patches.
Currently you can't use the Search menu with the org.eclipse.ui.menus extension point as it is created with an actionSet. I've opened bug 213385 asking for it to be migrated to the new ui extension.
Created attachment 85510 [details] Replaces XML menu with Design This patch renames the XML menu to design. This was suggested by Keith Chong, but is implemented here for feedback, and to show how a menu can be controlled based on the tabs within a MultiPageEditor and menuing extension points with handlers and Core Expressions checking. This does not yet implement the Expand and Collapse commands. Reload and partial Toggle Grammar Constraints are implemented.
Added a dependency on bug 213385 being fixed. Also supplied a patch to 213385 to make it extensible so that we can contribute commands to the Search menu.
Created attachment 85614 [details] Adds Source and Refactor menus to Structured Text editor popup This patch builds on the previous patch submissions and adds a Source and future Refactor menu to the Structured Text Editors popup menu. This uses the org.eclipse.ui.menus popup: locationurl to create the new menus. The new menus are created in the SSE UI plugin.xml file, and the appropriate source editing commands are contributed to the menu from the XML UI plugin.xml. These are the same commands that are added to the main menu's Source menu. No additional handlers or extra coding was necessary to get this commands added, just needed to create the appropriate menu contributions in the extension point. Also, this leverages the Search menu patch from bug 213385, which allows the Find Occurrences to be placed into the Search menu. Previously it was in the Source menu. Refactor will not show up until there are active commands contributed to it. Items left to do: 1. Refactor Collapse and Expand to the "Expand Selection To" menu options (suggested by Nitin in IRC). 2. figure out how to change a commands icon from an handler. For the toggle grammar constraints option in the Design menu (previously the XML menu).
Created attachment 85623 [details] Additional refactoring to migrate Generate->XML popup contribution Continues the changes, and migrates the Generate->XML popup contribution to the new menu. This shows an example of how to do check when a command should be enabled based on the contentType of the file being selected.
Created attachment 85671 [details] Implements toggle ability for Grammar Constraints from Handler This implements the ability for the "Toggle Grammar Constraints" design/xml menu option to be controled and updated correctly from a Handler. This requires that the handler implement IElementUpdater and the Command needs to have a refresh event sent to it. Items left to do: 1. Toolbar contribution - Expand, Collapse, Reload Dependencies, Grammar Constraints 2. Structured Selection options that were in the Edit menu, need to re-add these. I've cleaned up the plugins so these projects shouldn't display any more errors in the plugin.xml files in sseui and xmlui.
Created attachment 85691 [details] Migrates Expand All and Collapse all menu and tool bar options This may be the final patch. This patch uses the org.eclipse.ui.menus locationurl for toolbar contributions to contribute to the main tool bar the expand, collapse, reload dependencies, and grammar constraints. It also sets up handlers to handle these. It adds the options to the Design menu, and the same handler handles both the toolbar contributions as well as the Design menu contributions. So activing either one kicks off the same handler. This pretty much completes this Proof of Concept patch. There are a few items that I may have missed, but in general, everything that was being done through the legacy menu, popups, and toolbar contribution api, seems like it can now be handled in a more extensible way with the new org.eclipse.ui.menus ui. All handlers have been created in a package called org.eclipse.wst.xml.ui.internal.handlers. The existing ActionDelegates are there as well. Enablement, Activation, and general control of the commands and handlers is done with the Core Command Expression syntax. All major functionality is now in the plugin.xml file instead of hard coded into the various frameworks. From an adopters point of view, I think it is worth the time to investigate migrating other wst ui aspects to the new framework. Additional work will need to be done as well as tests to make sure that there aren't any hidden gotchas, but it does allow for an easier way for plugins to contribute and extend the available menus and options that wst provides.
Thanks again, Dave, for your hard work on this. I'm going to target this to be fixed in wtp 3.0 m5.
We should start this early in the process in case there is anything that needs to be changed on this. Might be best to put it in it's own branch, just until we verified that we got everything and didn't loose any functionality. It might need some fine tuning. Let me know if you need me to check anything, my time is kind of limited at the moment so I'll help out where I can.
Okay, I've looked through the patch and it looks good for the most part, but I have some suggestions: 1. Both the plugin.xml in the sse.ui & xml.ui plugins were formatted in a different way making it a bit harder to spot code changes versus whitespace changes. From what I can tell, I believe the real changes were commenting out any use of the org.eclipse.ui.editorActions & org.eclipse.ui.popupMenus extension points and the addition of the org.eclipse.ui.menus extension point. Is that right? Is there any way you could resubmit the patch with only the code changes? 2. In sse.ui/plugin.xml, it looks like the Source->Format Document popup menu item that's added to the Navigator/Package Explorer is commented out. This should be added back in in some way since it's a useful way for users to format multiple documents at once. 3. In sse.ui ActionBarContributor, I noticed a lot of code was commented out, which is good. But ExtendedEditorActionBuilder and debug status line code should be uncommented back in. 4. In xml.ui/plugin.xml, I believe the Expand All & Collapse All menu items were active at all times. They should only be active when in the Design page of the XML editor. 5. In xml.ui/plugin.xml, expression definition org.eclipse.wst.xml.ui.generate.xml is not used anywhere (I'm guessing you just forgot to delete it) 6. The biggest issue I see is with what we touched on in comment #15 and comment #16 regarding tying the command handlers to the XML editor rather than to a Source Editor with XML content type. I see in xml.ui/plugin.xml that most of the handlers are associated with the XML editor id. I also noticed in the Handler classes that they are doing an instanceof check to make sure the editor associated with the handler is XMLMultiPageEditor type. This limits these handlers to specifically this one XML editor, when really the xml handlers should be able to be reused easily by other Source editors with a type of XML content type (like the source page of the WSDL & XSD editors) You had mentioned in a previous comment that the key may be in the active contexts, and I agree with this. I was thinking what we could do is in StructuredTextEditor, activate content type contexts. This way, you could have handlers be active when there's a Source Editor context & XML content type context active. And to take it a step further, not only should the current content type context be active, but inherited content type context should be also active. This way, for example, when we're in the WSDL editor, which contains the WSDL content type, which extends the XML content type, the WSDL editor picks up the XML content type handlers automatically since in the WSDL editor, the WSDL content type context as well as the XML content type context would be active. The tricky part with this is what happens if the WSDL editor wants to provide a better handler implementation. I'm not sure what type of context hierarchy there is. And going back to the Handler implementations, rather than checking if the editor is instanceof XMLMultipageEditor, what we typically do in our code is: ITextEditor fTextEditor = null; if (editor instanceof ITextEditor) fTextEditor = (ITextEditor)editor; else { Object o = editor.getAdapter(ITextEditor.class); if (o != null) fTextEditor = (ITextEditor)o; } This will get you the text editor which is what is mainly needed in the handlers. Okay, I think that's about all the suggests I had for now. I'll keep looking through the patch and testing it out. Do you think you'll be able to implement the stuff I suggested so far?
Oh, I forgot, for the final patch, the copyright statements for all the files you touched will need to be updated to 2008 and your name will need to be added to the list of contributors (I think I saw it already done on some files, but not all)
(In reply to comment #26) > Okay, I've looked through the patch and it looks good for the most part, but I > have some suggestions: > > 1. Both the plugin.xml in the sse.ui & xml.ui plugins were formatted in a > different way making it a bit harder to spot code changes versus whitespace > changes. From what I can tell, I believe the real changes were commenting out > any use of the org.eclipse.ui.editorActions & org.eclipse.ui.popupMenus > extension points and the addition of the org.eclipse.ui.menus extension point. > Is that right? Is there any way you could resubmit the patch with only the code > changes? Yeah, I can do this, but I probably won't get to it until next week, as I've got to go out of town for business. > > 2. In sse.ui/plugin.xml, it looks like the Source->Format Document popup menu > item that's added to the Navigator/Package Explorer is commented out. This > should be added back in in some way since it's a useful way for users to format > multiple documents at once. We just need to make sure that we contribute the command to the appropriate popup: url for those particular views popup menus. > > 3. In sse.ui ActionBarContributor, I noticed a lot of code was commented out, > which is good. But ExtendedEditorActionBuilder and debug status line code > should be uncommented back in. Okay, noted. > > 4. In xml.ui/plugin.xml, I believe the Expand All & Collapse All menu items > were active at all times. They should only be active when in the Design page of > the XML editor. We can probably control these better once the appropriate content-type activators are added (see your comment below). > > 5. In xml.ui/plugin.xml, expression definition > org.eclipse.wst.xml.ui.generate.xml is not used anywhere (I'm guessing you just > forgot to delete it) Yes, this needed to be removed, as it really is contributed by the DTD and XSD portions. > > You had mentioned in a previous comment that the key may be in the active > contexts, and I agree with this. I was thinking what we could do is in > StructuredTextEditor, activate content type contexts. This way, you could have > handlers be active when there's a Source Editor context & XML content type > context active. And to take it a step further, not only should the current > content type context be active, but inherited content type context should be > also active. This way, for example, when we're in the WSDL editor, which > contains the WSDL content type, which extends the XML content type, the WSDL > editor picks up the XML content type handlers automatically since in the WSDL > editor, the WSDL content type context as well as the XML content type context > would be active. The tricky part with this is what happens if the WSDL editor > wants to provide a better handler implementation. I'm not sure what type of > context hierarchy there is. If we can get a active context for the when the Structured Source Editor is the active context, this will make it much easier to handle the context and when they are active or not active. > > And going back to the Handler implementations, rather than checking if the > editor is instanceof XMLMultipageEditor, what we typically do in our code is: > ITextEditor fTextEditor = null; > if (editor instanceof ITextEditor) > fTextEditor = (ITextEditor)editor; > else { > Object o = editor.getAdapter(ITextEditor.class); > if (o != null) > fTextEditor = (ITextEditor)o; > } > > This will get you the text editor which is what is mainly needed in the > handlers. I think I tried that, and for some reason it wasn't working. The only way I was able to get it working was with specifically checking for the XMLEditor implementation. One thing, we need to figure out is...what is common Editor functionality, and what is XML specific functionality that may depend on a specific xml editor implementation. > > Okay, I think that's about all the suggests I had for now. I'll keep looking > through the patch and testing it out. Do you think you'll be able to implement > the stuff I suggested so far? Should be able to too. I will need that SSE editor context so that I can key off that and the file's content-type. Having that active context will help, with determining when items should be active and when they shouldn't. However, as I said I probably won't be able to get to this until at the earliest Thursday or Friday next week.
(In reply to comment #28) > One thing, we need to figure out is...what is common Editor functionality, and > what is XML specific functionality that may depend on a specific xml editor > implementation. I believe most of the actions (at least the ones included in this bug) should work as long as there's a Source editor with a StructuredModel. The only ones that seem specific to this editor are the design page actions like Expand & Collapse All. > I will need that SSE editor context so that I can key > off that and the file's content-type. Having that active context will help, > with determining when items should be active and when they shouldn't. I could try taking a stab at this (maybe this even deserves its own bug) The issue I'm still not sure about is if there's some sort of hierarchy for contexts and what happens if there is more than one context active and a handler for each context. Since we're not going to be able to fix this by tonight, I'm going to retarget this bug for M6.
(In reply to comment #29) > > I could try taking a stab at this (maybe this even deserves its own bug) The > issue I'm still not sure about is if there's some sort of hierarchy for > contexts and what happens if there is more than one context active and a > handler for each context. > > Since we're not going to be able to fix this by tonight, I'm going to retarget > this bug for M6. I think the best person to talk about the behaviour of the contexts and how the commands are handled, is Paul Webb.
Amy, I'm back from my business trip. I'll clean up the existing patch against the latest M5 build hopefully this weekend.
Created attachment 89897 [details] Cleaned up patch to remove plugin.xml formatting This is a cleaned up patch to remove the plugin.xml formatting that was previously in the earlier patches. This has the same functionality of the prior patches.
Note for myself to remember what UI items need to be cleaned up: (In reply to comment #28) > > > > 2. In sse.ui/plugin.xml, it looks like the Source->Format Document popup menu > > item that's added to the Navigator/Package Explorer is commented out. This > > should be added back in in some way since it's a useful way for users to format > > multiple documents at once. > > We just need to make sure that we contribute the command to the appropriate > popup: url for those particular views popup menus. > > > > > > 3. In sse.ui ActionBarContributor, I noticed a lot of code was commented out, > > which is good. But ExtendedEditorActionBuilder and debug status line code > > should be uncommented back in. > > Okay, noted. Fixed...this is will be in the next patch with item #2. > > > > > > 4. In xml.ui/plugin.xml, I believe the Expand All & Collapse All menu items > > were active at all times. They should only be active when in the Design page of > > the XML editor. These are only active when the Design Editor is active, if you switch to the source page view, they still show in the tool bar and Design menu, but aren't enabled. > > > > 5. In xml.ui/plugin.xml, expression definition > > org.eclipse.wst.xml.ui.generate.xml is not used anywhere (I'm guessing you just > > forgot to delete it) > > Yes, this needed to be removed, as it really is contributed by the DTD and XSD > portions. > Removed extraneous expression definition. I'll post an updated patch once I get the Format Document added back to the XML content types in the various navigator views (package explorer, project explorer).
Created attachment 90283 [details] Implements Source->Format on Navigator Menus This patch implements the Source->Format for IFile and IContainer resources. It also includes the fixes that were previously suggested. This should be ready for review.
Here are my notes on the latest patch: 1. sse.ui ActionBarContributor still has ExtendedEditorActionBuilder & debug status line code commented out (see #3 in comment #26) I will attach a patch I made that should add that code back in. 2. xml.ui XMLTableTreeActionBarContributor was modified recently, so your patch didn't apply cleanly. I will attach a patch that should fix that. 3. copyright comments still need to be updated for all the files you touched (see comment #27) 4. xml.ui/plugin.xml Expand All & Collapse All are still active in the menu at all times (see #4 in comment #26) I believe all that's needed is to add a visibleWhen check when you add the command in the main menu. 5. Source->Format is not working in the context menu. 6. sse.ui/plugin.xml The menuContribution code that adds sourceBegin, additions, sourceEnd, and refactorPopupMenuId to the "org.eclipse.core.runtime.xml.source.EditorContext" popup menu should be removed. Right now, the editor context menu id is generated programatically in the StructuredSourceEditor, and I think I will keep it that way. What I can do instead is in that same area of code, add those additional separators as well. 7. xml.ui/plugin.xml The org.eclipse.core.expressions.definitions, the sseActiveContext definition should be moved to the sse.ui plugin. I like the idea of putting the expressions in definitions so that adopters can just refer to the definition id and we can tweak the actual definitions later (like using content type context instead of editor id to identify the xml editor once I figure out how to do that) Since we're partway through M6 and I believe this is the last milestone for UI changes, once you've made the changes mentioned above, I'll put this bug up for legal review so we can get the bulk of the code in as soon as possible. Then I'll open up new bugs to polish this up.
Created attachment 92198 [details] ActionContributor.patch
Created attachment 92199 [details] XMLTableTreeActionBarContributor.patch
I'll get these added in and do a combined patch against the latest code base. I should have something for you tomorrow, the latest will be Friday.
(In reply to comment #35) > Here are my notes on the latest patch: > 1. sse.ui ActionBarContributor still has ExtendedEditorActionBuilder & debug > status line code commented out (see #3 in comment #26) I will attach a patch I > made that should add that code back in. > > 2. xml.ui XMLTableTreeActionBarContributor was modified recently, so your patch > didn't apply cleanly. I will attach a patch that should fix that. I applied both patches. > > 3. copyright comments still need to be updated for all the files you touched > (see comment #27) I believe I've got all of these now. > > 4. xml.ui/plugin.xml Expand All & Collapse All are still active in the menu at > all times (see #4 in comment #26) I believe all that's needed is to add a > visibleWhen check when you add the command in the main menu. Added the visibleWhen check. This now only shows the Design view menu when in the Design tab of the XML Multipage Editor. The tool bar is still available to allow reloading of the grammars in the source editor and to disable grammar validation. > > 5. Source->Format is not working in the context menu. I tested this out, and it seems to correctly kick off the appropriate background job for formatting files. The new handler is FormatHandler in org.eclipse.wst.sse.ui if you need to debug it further, but from what I could tell it is working the same as before. > > 6. sse.ui/plugin.xml The menuContribution code that adds sourceBegin, additions, > sourceEnd, and refactorPopupMenuId to the > "org.eclipse.core.runtime.xml.source.EditorContext" popup menu should be > removed. Right now, the editor context menu id is generated programatically in > the StructuredSourceEditor, and I think I will keep it that way. What I can do > instead is in that same area of code, add those additional separators as well. The problem here is that it may not be able to be seen by the other plugins. If you add it through api, I would suggest making sure you use the corresponding API from the org.eclipse.ui.menus plugins. This way we are guaranteed for these spots to show up by adopters looking for them to be there to add their own content. Intermixing functionality from the old API and the new API can cause some weird things to happen, and the purpose of the new api is to not hide the id's as they are needed by adopters to know where to contribute. Having the id's added by API defeats that purpose and makes adopters have to search multiple places in code to find the ids. We should allow adopters to add easily to our menus instead of hiding the content. Is there a particular reason why you want to do this through code instead of in the plugin.xml? > > 7. xml.ui/plugin.xml The org.eclipse.core.expressions.definitions, the > sseActiveContext definition should be moved to the sse.ui plugin. I moved this to the sse.ui.plugin. It does look like you can reference other definitions defined in other plugins.
Created attachment 92436 [details] Combined Patch with Additonal Fixes This is a combined patch that can be applied to the latest code in the HEAD branch.
1. I'm getting the following error with the latest patch: !ENTRY org.eclipse.ui 4 0 2008-03-14 16:15:06.488 !MESSAGE Unable to create editor ID org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorPart: An unexpected exception was thrown. !STACK 0 java.lang.NullPointerException at org.eclipse.wst.xml.ui.internal.actions.ActionContributorXML.setViewerSpecificContributionsEnabled(ActionContributorXML.java:215) at org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorActionBarContributor.activateDesignPage(XMLMultiPageEditorActionBarContributor.java:48) at org.eclipse.wst.xml.ui.internal.tabletree.SourceEditorActionBarContributor.setActivePage(SourceEditorActionBarContributor.java:185) at org.eclipse.ui.part.MultiPageEditorActionBarContributor.setActiveEditor(MultiPageEditorActionBarContributor.java:50) It looks like this is due to 3 actions that were added recently, go to matching bracket, go to previous sibling, go to next sibling. I'll take a look at how these actions are being contributed, and hopefully fixing these 3 actions can be spun off into another bug to not hold this bug up any longer. 2. I noticed you added copyright comments to the plugin.xml files. I should've clarified that those files don't have copyright comments in them. (I don't know why, but they dont) 3. Now that "org.eclipse.wst.xml.ui.sseActiveContext.definition" as been moved to the sse.ui plugin, the definition id should be "org.eclipse.wst.sse.ui.sseActiveContext.definition" 4. I meant the Format action was not working in the editor's context menu. But now that I'm looking at the editor context menu again, I can't seem to find the Source submenu in the editor context menu at all. I see that you've defined it in the plugin.xmls, but they're not showing up. 5. And while I'm on the subject of the editor popup menu, I think a better compromise for what we both wanted (you wanting menu ids defined in plugin.xml, me wanting to programmatically define content type-related ids) is rather than using the xml editor popup menu id, use the source editor popup menu id. So instead of <menuContribution locationURI="popup:org.eclipse.core.runtime.xml.source.EditorContext?before=additions"> ... </menuContribution> use <menuContribution locationURI="popup:org.eclipse.wst.sse.ui.StructuredTextEditor.EditorContext?before=additions"> ... </menuContribution> The SSE editor always registers the source editor popup menu id for the source editor, and then it programatically also registers a popup menu id for the same menu based on the current content type in the editor. I'm thinking the content type popup menu id code may be eventually taken out once I get content type active context set up. Then menu items can always just associate themselves with the generic source editor menu id and then handlers can specify enablement based on a content type specific active context.
I think we need to get this patch in soon, and start to encourage people to use the new UI and extension points. The new annotation actions, can actually be done using commands and handlers instead. I can discuss more at the WTP BOF event on the 17th.
(In reply to comment #41) > 1. I'm getting the following error with the latest patch: > !ENTRY org.eclipse.ui 4 0 2008-03-14 16:15:06.488 > !MESSAGE Unable to create editor ID > org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorPart: An unexpected > exception was thrown. > !STACK 0 > java.lang.NullPointerException > at > org.eclipse.wst.xml.ui.internal.actions.ActionContributorXML.setViewerSpecificContributionsEnabled(ActionContributorXML.java:215) > at > org.eclipse.wst.xml.ui.internal.tabletree.XMLMultiPageEditorActionBarContributor.activateDesignPage(XMLMultiPageEditorActionBarContributor.java:48) > at > org.eclipse.wst.xml.ui.internal.tabletree.SourceEditorActionBarContributor.setActivePage(SourceEditorActionBarContributor.java:185) > at > org.eclipse.ui.part.MultiPageEditorActionBarContributor.setActiveEditor(MultiPageEditorActionBarContributor.java:50) > > > It looks like this is due to 3 actions that were added recently, go to matching > bracket, go to previous sibling, go to next sibling. I'll take a look at how > these actions are being contributed, and hopefully fixing these 3 actions can > be spun off into another bug to not hold this bug up any longer. Amy, I found the problem for this. In the sse.ui.internal.actions.ActionContributor class the following lines need to be uncommented from the patch. fGotoMatchingBracketAction = new RetargetTextEditorAction(resourceBundle, StructuredTextEditorActionConstants.ACTION_NAME_GOTO_MATCHING_BRACKET + StructuredTextEditorActionConstants.UNDERSCORE); fGotoMatchingBracketAction.setActionDefinitionId(ActionDefinitionIds.GOTO_MATCHING_BRACKET); > 2. I noticed you added copyright comments to the plugin.xml files. I should've > clarified that those files don't have copyright comments in them. (I don't > know why, but they dont) I'll remove these. > > 3. Now that "org.eclipse.wst.xml.ui.sseActiveContext.definition" as been moved > to the sse.ui plugin, the definition id should be > "org.eclipse.wst.sse.ui.sseActiveContext.definition" Renamed. > > 4. I meant the Format action was not working in the editor's context menu. But > now that I'm looking at the editor context menu again, I can't seem to find the > Source submenu in the editor context menu at all. I see that you've defined it > in the plugin.xmls, but they're not showing up. It's showing up in my patched version correctly (I applied the patch that is listed here). Source->Format is there. > > 5. And while I'm on the subject of the editor popup menu, I think a better > compromise for what we both wanted (you wanting menu ids defined in plugin.xml, > me wanting to programmatically define content type-related ids) is rather than > using the xml editor popup menu id, use the source editor popup menu id. So > instead of > <menuContribution > locationURI="popup:org.eclipse.core.runtime.xml.source.EditorContext?before=additions"> > ... > </menuContribution> > > use > <menuContribution > locationURI="popup:org.eclipse.wst.sse.ui.StructuredTextEditor.EditorContext?before=additions"> > ... > </menuContribution> > > The SSE editor always registers the source editor popup menu id for the source > editor, and then it programatically also registers a popup menu id for the same > menu based on the current content type in the editor. I'm thinking the content > type popup menu id code may be eventually taken out once I get content type > active context set up. Then menu items can always just associate themselves > with the generic source editor menu id and then handlers can specify enablement > based on a content type specific active context. > Okay, I've changed this, and will attach a new patch.
Created attachment 92731 [details] Null Pointer Fix and changed Source Popup Menu ID contribution This fixes the null pointer exception, and renames the ids mentioned. The bracket matching and annotation items should be refactored to the toolbar contribution using org.eclipse.ui.menus and handlers.
Just to clarify, isn't it always referred to as Smart Insert and never as "Smart Auto Insert"? The latest patch uses "Smart Auto Insert".
(In reply to comment #45) > Just to clarify, isn't it always referred to as Smart Insert and never as > "Smart Auto Insert"? The latest patch uses "Smart Auto Insert". > Honestly, I'm not sure. In JDT, it looks like they call it, "Smart Insert Mode". Probably need to change it so that at least the Menu item is "Smart Insert Mode". I tried to keep all the names the same as the way JDT had them for consistency.
I agree we need to get this code in asap. I've gone ahead and opened an ip request: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=2193 I not sure if it's necessary or not since you are a wtp committer, but not in the sourceediting component. But I might as well follow the process just in case. Hopefully I'll get a quick reply and I'll commit & release the patch soonafter.
The patch has been ip approved. I've checked in and released the fix. I've opened bug 224040 to track some outstanding issues. Thanks, Dave for all your work on this bug.
mass change to add 'contributed' keyword based on bugzilla query, please correct if that's not accurate (by marking patches as obsolete and removing the 'contributed' keyword.
