Summary: | [Commands] misc: Provide new API to hook actions to commands | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Michael Valenta <Michael.Valenta> | ||||||
Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||||
Status: | ASSIGNED --- | QA Contact: | |||||||
Severity: | enhancement | ||||||||
Priority: | P5 | CC: | douglas.pollock | ||||||
Version: | 3.0 | Keywords: | helpwanted | ||||||
Target Milestone: | --- | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Michael Valenta
2003-09-04 12:04:54 EDT
doug, please look at this for mike. i spoke to him yesterday about it. This is proving to be interesting. First of all, it looks like only the text editor is using the delete provided through the global key filter. All the other tables and trees are coding their own key handlers: TaskList, MarkerView, BookmarkNavigator, and RefactorActionGroup. In TaskList, for example, this use of SWT.DEL has been in there since before September 24, 2002 (when a big refactoring took place). This seems to indicate that this isn't a regression, but that this has been like this for quite some time. The problem is that the CommandManager is never made aware of the command. Created attachment 6082 [details]
Patch to RefactorActionGroup, SyncViewerActions, and SynchronizeView
This removes the unnecessary code, and adds code to register the action with
the global key binding service. Only code has been added to register the
delete action. The move and rename have been left as an exercise for the
reader. ;) Plus, some decision might need to be made about where best to get
the command ID constants from.
This has been tested on I20030910 with the HEAD for platform-ui and
org.eclipse.team.ui. I bound the delete key to 'F9' to make sure that the
native widget wasn't doing any work. 'F9' also worked in the text editor with
these changes.
As always, Chris could provide more insight into how the key binding architecture should work. Hope this helps. Oh, one other thing. The command "org.eclipse.ui.edit.delete" is not bound to any key by default. If you want it bound to the "Delete" key, then you should file a separate enhancement request. (or just ask someone on platform-ui with commit rights). I have applied the patch that Doug supplied. However, he raises some issues above that are still open. Reassigning to UI to consider. The issue I'm most concerned with is the hard-coded string "org.eclipse.ui.edit.delete" which is in out code. Does UI have a constant for this? If not, it should. Created attachment 6135 [details]
Patch migrating to new architecture
This patch does a few things. It marks getKeyBindingService() as deprecated in
IWorkbenchPartSite, and replaces it with a new method getActionService().
MultiPageEditorSite is update to reflect these changes.
And, more importantly, IWorkbenchCommandConstants is added. This declares as
constants all of the command identifiers that the UI plugin defines (in
plugin.xml). It also gives as an explanatory note in the class comment which
explains how to hook both old-style and new-style actions as handlers for
global commands.
Chris: look over this patch carefully. This deprecates the old getKeyBindingService() API -- providing direct access to IActionService. If it looks good, then I'll add comments for each of the declared command identifiers. Also I'm not sure about the comment for IEditorActionBarContributor; it's copied from IWorkbenchActionConstants. Don't apply the patch yet. Doug what is the status of this? Not for 3.0. Lowering priority to P2 (P1 means that this is a "stop-ship" bug report) Reassigning to Platform-UI-Inbox (I left IBM 18 months ago..) Moving Dougs bugs There are currently no plans to work on this feature. PW Changes requested on bug 193523 We now have the IHandlerService to hook up actions, and an IWorkbenchCommandConstants to describe the commands we provide. PW This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. |