Add convenient static factory methods to create JFace Actions

[HannesWell]

Creating JFace actions is often cumbersome, especially for simple implementations of the run method, because one has to sub-class the Action class and implement at least the run method. Very often this is done using a anonymous class whose run() method just calls the desired method.

Similar like org.eclipse.core.runtime.jobs.Job.create() jface' Action could provide a static factory method that accepts the runnable to execute (and optionally a text and image). And does the anonymous implementation for the caller.

I also added overloads that uses a Consumer and overwrites runWithEvent().
Additionally an overload to accepts a text and the style could be added to reflect all Action constructor overloads.

Instead of providing the methods in the Action class, they could also be added to the IAction interface.

There would be many use-cases in the SDK. I already used it for a few in the second commit, but that could be extended.

[github-actions]

Test Results

     814 files  ±0       814 suites  ±0   50m 18s ⏱️ + 11m 58s
  5 667 tests ±0    5 524 ✔️ ±0  143 💤 ±0  0 ❌ ±0 
18 105 runs  ±0  17 653 ✔️ ±0  452 💤 ±0  0 ❌ ±0 

Results for commit eb89e98. ± Comparison against base commit 8929f49.

♻️ This comment has been updated with latest results.

[BeckerWdf]

I like it. Makes consumer code much more readable.

[laeubi]

I think for actions a builderpattern would be much more suitable e.g.

IAction theAction = IAction.create("My Action").withImage(iconDescriptor).onEvent(event -> doSomethingWith(event));

IAction theAction = IAction.create("My Action").asCheckBox().onRun(() -> doSomething());

[HannesWell]

I think for actions a builderpattern would be much more suitable e.g.

IAction theAction = IAction.create("My Action").withImage(iconDescriptor).onEvent(event -> doSomethingWith(event));

IAction theAction = IAction.create("My Action").asCheckBox().onRun(() -> doSomething());

Sounds good. I was looking for that as well.

The only change I think we have to do, is to provide the runnable/event-consumer as mandatory argument of the create method. An action without a runnable does not make sense so it should be enforced by the type-system.

I can add those builder methods in a second commit of this PR.

[laeubi]

The only change I think we have to do, is to provide the runnable/event-consumer as mandatory argument of the create method. An action without a runnable does not make sense so it should be enforced by the type-system.

This is ensured by the terminal operation that transform the builder into an action.

[HannesWell]

The only change I think we have to do, is to provide the runnable/event-consumer as mandatory argument of the create method. An action without a runnable does not make sense so it should be enforced by the type-system.

This is ensured by the terminal operation that transform the builder into an action.

Ok. With your proposal initially I was actually thinking about fluent setters, so that the create method already has build the actions and the fluent setters just call the setter that have return type void:
IAction theAction = IAction.create(event -> doSomethingWith(event)).withText("My Action").withImage(iconDescriptor);

IAction theAction = IAction.create(() -> doSomething()).withText("My Action").asCheckBox();

This would have the advantage that the terminal step is not necessary. Furthermore an Action does not necessarily need a text. Therfore the initial method should not require a text. So in that case IAction.create(() -> doSomething()) is sufficient with fluent setters, while with a full builder-pattern it would be a bit longer: IAction.create().onRun(() -> doSomething()).
The advantage of using fluent setters would be that there is no extra IActionBuilder type necessary, which has at the same time the weakness that the fluent setters would clutter the IAction interface.
If we continue with the full builder the initial method name should indicate that better. Something like
IAction.builder(). Furthermore I would like to prefix the terminal method with build...() so that it is more clear that this is the terminal operation. But buildRunOn() buildRunOnEvent() does not sound so well to me.

[laeubi]

I don't think fluent setters add anything above the current way of calling a setter.
Also note that some things can not be reliable set because they are passed as constructor parameters usually (e.g. the Type of action) and a builder might be used to build many similar actions that only diverge in some aspects.

[BeckerWdf]

What's the state of this PR?

[HannesWell]

What's the state of this PR?

It is currently stalled. I plan to continue on this further discussing Christoph's suggestions, but I don't expect to find time for that to be in time for the September release.

I don't think fluent setters add anything above the current way of calling a setter. Also note that some things can not be reliable set because they are passed as constructor parameters usually (e.g. the Type of action) and a builder might be used to build many similar actions that only diverge in some aspects.

The question I ask myself here, if in usual use-cases so many aspects are shared for different actions. And at the same time I would like to make this simple in simple use-cases where one only wants an action with a runnable, without more setup.