(110/212) 303 - EditAction.Wrapper - Extend to support javax.swing.action

Looks like I need to do this myself in order to unify HelperLauncher/ProjectViewer.

I will also need to support full sets of properties, including dynamic labels which can be chosen at the time the menu is created.

- **assigned_to**: nobody --> ezust
- **summary**: JEditAbstractEditAction: is/setEnabled() --> EditAction.Wrapper - Extend to support javax.swing.action

Post by Francois Rey on jEdit-devel which I think is relevant, so I am going to paste it here:
--- cut here ---
Over a month ago while writing the Launcher plugin I started a thread because
I needed help in clarifying how to best use Action, ActionSet, etc. I got some
answers but nothing gave me a clear sense of direction, I was more confused.
Back then I wrote a reply while carrying out my own investigation. It didn't
look so contructive, so I just kept the mail in my draft folder.
However recent discussions on how to improve Launcher before release showed me
how 'hot' this topic is. The need for some refactoring in this area is shared,
as well as the understanding this would be an involved work.

I'm unable carry that work (other priorities require my attention) however the
draft email I wrote, while not really constructive, could serve as my 2cts and
give a nice overview of what's confusing with present situation.

To summarize, here's what's the most confusing for me:

1. Actions are both stateful and stateless without clear indication when they
are and when they're not
2. When stateful, state is not necessarily used in action invoke() logic
3. ActionContext has more to do with managing Action than giving context
4. Top level classes make strong assumptions that Action are implemented with
beanshell code

Here's the longer version (draft email with minor edits):


On Saturday 16 January 2010 01:57:05 Alan Ezust wrote:
> Yes, PV has its own hierarchy of actions, which is kinda annoying. But
> that's because jedit's built-in actions were not sufficient for the job at
> the time, I suppose.
Probably, yes, and I realize this Action/ActionSet/ActionContext has a long
history. These classes have grown organically and that's part of my confusion:
I don't seem to be able to see a clear intent in them, or if there is/was, it
seems diluted at various places and the names only help a little.

For example, at first glance the framework seems to indicate that Actions
should not have state so the same instance can be reused and applied several
times. So you think the context is what changes across invocations. But when
you look at the the Action api, it never references any ActionContext, because
ActionContext are just manager of ActionSet (it even says so in the javadoc).
It's only through the indirection of its own invoke() method that an
ActionContext can prepare some contextual information before calling
action.invoke(), as it is the case for BrowserActionContext defined in
VFSBrowser.java. In other words the mechanism by which an action logic knows
its contextual information is outside the scope of the action framework api.
In fact, as far as Actions are concerned, ActionContext does not exist. If you
look at FSB code here's what's happening: context information is given by the
\*global\* beanshell namespace to beanshell actions. The same ActionContext
instance is used for managing sets of stateless Actions, so it has nothing
specific to the action about to be invoked since it's the same context for all
actions. So in terms of name it has nothing to do with context, and more to do
with an <ActionSet>Manager that only deals with context if one cares to use it
as intermediates for invoking actions.

While FSB actions are somewhate stateless, PV Actions are clearly stateful and
make no use of ActionContext. The PV method Action.prepareForNode() is a clear
illustration of that. So the two existing components that already do something
similar to what the Launcher plugin does are far appart from each other in the
way they use Action and ActionContext.

Moreover, when looking at top level framework class JEditAbstractEditAction
you realize actions aren't meant to be stateless after all. There is a
protected instance variable of type Object\[\] which is first initialized in the
contructor, and then modified by the invoke method that has an additional
Object\[\] parameter. However the latter does not seem to be aimed at containing
the actual logic of the action: it just memorize the array and call the other
invoke method which is abstract.
So what are these arguments for? The subclass EditAction.getLabel() uses them
to compute the label, which as nothing to do with action parameters.
LoadPerspectiveAction, private class of DockingLayoutManager, uses it in its
invoke() logic, so in that case they're really contextual information for the
invoke logic.

Another thing that makes me wonder how to best use current framework classes:
the abstract method JEditActionSet.createBeanShellAction() implies that the
semantic of ActionSet is dependent of BeanShell. In fact, ActionSet,
GUIUtilities, EnhancedMenu, all have strong assumptions that EditActions logic
are implemented with BeanShell scripting. So the framework is sending strong
signals that Action must be implemented with BeanShell so you should think
twice if you don't. However not all actions are (best) implemented in
beanshell and I guess that's part of the reason why not all actions can be
recorded as macros. I do however understand why we would prefer beanshell
actions: they're obviously stateless, and they can be easily recorded\! One way
to make things clearer in that respect would be to have a getReplayCode() in a
ReplayableAction interface (or RecordableAction?). This would not imply that
all actions have to have beanshell code, and would offer a chance for any
action to provide a replay beanshell code even if they're not beanshell
actions.

So overall I feel like the framework make strong statement here and there that
are not always holding true everywhere. Hence all the head scratching when
determining what's the best API for a new plugin like Launcher.

- **assigned_to**: ezust --> nobody