Description Nick Edgar 2001-10-10 22:22:19 EDT

The accelerator and text of an action cannot be set independently.

This is problematic for global actions in the Eclipse UI, since handlers can change the text, but the workbench
should define the accelerator.  Currently handlers must re-specify the accelerator in the text.
There is also no way of getting the textual representation of the accelerator, only the SWT keycode.

The accelerator should be a separately settable property, with conveniences for converting from
the textual representation to the SWT keycode, and vice-versa.

Customer's original request:
RH:
I am updating the Undo Action's label, for example, "undo delete".
If I do this, it resets the accelerator.  How do I avoid resetting the acclerator?
And, In the case of Undo, when our action is actually wrapped by the real UndoAction,
is our accelerator being used, or just that of the Global UndoAction?

NE:
Action.setText() is used to set the accelerator as well.  Since global
action handlers simply propagate their setText() to the "real" action, the
accelerator (and mnemonic) are passed on (or lost if not specified by the handler).
By default, undo and redo use "&Undo@Ctrl+Z" and "&Redo@Ctrl+Y" for their text.

You should include both the mnemonic and the accelerator in the text, as
follows:

undoAction.setText("&Undo Delete@Ctrl+Z");
redoAction.setText("&Redo Delete@Ctrl+Y");

You might also want to set the tool tip text.

RH:
Why is the accelerator not a separate proprety from the label?
Anyway, we are trying to parse the old label's accelerator and restore it.

If I call getText, will I get the "@Ctrl+Z" portion, or just Undo?

NE:
Unfortunately, the accelerator portion is stripped when it is set, so getText() would return "&Undo", not "&Undo@Ctrl+Z".
There is no way of getting the accelerator for a global action.

Note that there is a getAccelerator() method on Action, but this returns the SWT key code, and there is no API to convert this to the textual representation.
However, getting the accelerator on your handler will not return the accelerator of the actual global action.

I have filed two PRs for these issues: 
1GCDU59: JFUIF:ALL - Actions should have accelerator as separate property
1GCDW98: ITPUI:ALL - Need way of getting accelerators for global actions

For now, I recommend just hard-coding "@Ctrl-Z" and "@Ctrl-Y" for your undo and redo handlers.


NOTES:

NE (4/18/01 10:25:33 AM)
	Should add setAccelerator(int) to IAction and Action.
	Should change setText() to not change accelerator if not specified.
	Should add static methods on Action:
		int acceleratorTextToKey(String acceleratorText)
		String acceleratorKeyToText(int)

NE (4/18/01 12:35:47 PM)
	RH proposes:

Action.setText(String) and Action.getText() are not symmetrical.
"@" it being converted to a TAB, and "&" is getting lost.

We are implementing UndoAction.getTooltipText() to return getText(), but
then the
Tooltip has the accelerator on it.  This doesn't make any sense to me.

I would prefer that  "text" be an entirely separate property from
accelerator
(and possibly mnemonic).  getText() might return just the Text, no
accelerator.

It is probably too late to make this type of change, but this could be
simulated
by changing the semantics of setText().  setText() could parse the
accelerator
if present, but leave it alone if not present.

So, the accelerator would not be changed when calling:
     CASE 1: setText("Undo delete")

To really set the accelerator to null, I would call:
     CASE 2: setText("Undo delete@")

One could argue that the mnemonic should always be set with the text, so I
think having to call:
     CASE 3: setText("&Undo delete")
is perfectly acceptable.

To be considered separately, getText() could return just the text, without
the accelerator at the end.
ActionContributionItem would have to be changed accordingly when it updates
a MenuItem.

NE (4/18/01 1:13:02 PM)
	Made suggested workarounds.  See: 
		1GCE23H: JFUIF:ALL - Accelerator should not be reset if not specified in text
		1GCE25G: JFUIF:ALL - Action.getText() should return what was set in setText()

NE (5/2/01 10:32:52 PM)
	Should make accelerator separate property.
	Should have way of converting to/from printable representation.
	However, too late to make these API changes.
	Workaround is to have all global action handlers specify accelerator text.
	Moved to inactive.