Community
Participate
Working Groups
Build ID: I20070717-1020 We have some common actions in the editors: Delete, Cut, Copy and Paste. If you look at the context menu in the Package Explorer or in the "Edit" toolbar, these actions have associated icons and keybindings. PDE editors are still lacking both icon and keybinding for these actions. Each of the four actions I mentioned should use the standard Enabled icon, Disabled icon, and keybinding used in other Eclipse projects.
I looked into this and i detected some inconsistencies. I assume that "PDE editor" is the "Plug-in Manifest Editor". There is no unique naming standard for these operations. "Delete" is sometimes called "remove". In most of the views "Remove, Cut, Copy, Paste" are in a single group (separated from the rest of the context menu by a line) For example in "Overview" in "Execution Environments" and in "Dependencies" (all three tree views) But "Runtime", "Package Visibilities" has a "remove" button but no "remove" operation in the context menu. In "Extensions" there is a "Delete" in the context menu, but it is separated from the "Copy, Cut and Paste" operations. Is this a separate issue ?
All good points. The inconsistencies are captured by bug 197828.
I think i found the place to change: org.eclipse.pde.internal.ui.editor.PDEFormEditorContributor. The image descriptors will be taken from the workbench ISharedImages. The keybind is set with setActionDefinitionId. ISharedImages sharedImages = getPage().getWorkbenchWindow().getWorkbench().getSharedImages(); setImageDescriptor(sharedImages.getImageDescriptor(ISharedImages.IMG_TOOL_COPY)); setDisabledImageDescriptor(sharedImages.getImageDescriptor(ISharedImages.IMG_TOOL_COPY_DISABLED)); setActionDefinitionId("org.eclipse.ui.edit.copy"); //$NON-NLS-1$ I took the class org.eclipse.ui.actions.ActionFactory as the prototype. Does this sound ok ? I will provide a patch.
Now it works for cut, copy and paste. NOT for "remove". "remove" is handled differently in the class PDEFormEditorContributor. I think there is a special "remove" action and that it is not the DELETE-action used in PDEFormEditorContributor addGlobalAction(ActionFactory.CUT.getId(), fCutAction); addGlobalAction(ActionFactory.COPY.getId(), fCopyAction); addGlobalAction(ActionFactory.PASTE.getId(), fPasteAction); addGlobalAction(ActionFactory.DELETE.getId()); I will attach a patch for cut, copy and paste.
Created attachment 74810 [details] Patch for cut, copy and paste operations
Brian to review patch.
Created attachment 74821 [details] Patch with an additional delete operation (with a red cross icon)
It is also possible to provide a second "consistent" delete operation (that has a red cross). Now the remaining task is to remove the "remove" operation (which i haven't found in the source code). I attached the patch.
Joern, you are definitely well on your way with this patch. It is looking good. The reason you can't find the Remove action off hand, is because each section implements their own remove action. This seems extremely inefficent. Unfortunately, to remove those we will need to go to each section that has a remove action and maunally remove it. I was looking through many of those sections and how the section is used. I think the wording "Remove" is more descriptive instead of "Delete" in those cases. For example, in the feature editor, when you select an included plug-in and select "Remove" you know the item will get removed from the file. Whereas if you selected "Delete", one could possibly expect you to delete the plug-in itself since that was what was selected. Therefore, lets try to rename the DeleteAction to "&Remove" instead of "&Delete". I also wanted to try to stay as consistent with the ordering we currently have. In order to do this, we should move the Remove action to the top of the list (above cut, copy and paste). When removing each individual remove action you may notice some sections have a remove all action. Make sure we run the PDEFormEditorContributor, and then you can call the manager.insertAfter(id, Action). Insert the removeAllAction after the DeleteAction (whose id should be ActionFactory.DELETE.getId()). If you need help trying to find instances of the existing remove actions, let me know.
I have been known to get carried away (especially with a crying baby in the background). Wassim reminded me that this bug was just to make the Actions look more shiny, not to worry about reordering (bug 197828). For simplicity, I am going to commit your first patch. When we work on reordering, we can then utilize the Delete Action you created. Not changing a thing from your patch (minus adding your name to the copyright). Thanks alot for the very solid patch and going above and beyond with the creation of DeleteAction. We definitely appreciate the help! Be prepared to see the new and improved, shinier PDE in 3.4M1 ;-)
Joern, we added your name to the contributors page. We will need a 200x280 picture to go along with it. A leafy background preferred. In the meantime, we will use a placeholder: http://www.eclipse.org/pde/pde-ui/committers/committers.php
