Let OperationHandlers return an optional command to control execution #187
Conversation
martin-fleck-at
force-pushed
the
issue-864
branch
10 times, most recently
from
2b5003c
to
6bdf2f3
Compare
There was a problem hiding this comment.
Thanks Martin! Create change and even nicer API refactoring that cleans up the convoluted OperationHandler type hierarchy without introducing too many API breaks.
Apart from a couple of inline suggestions I noticed the following:
Part of the example operation handlers still use the (now) deprecated handlers which results in a couple of warnings:
ec6ac70cb0b6.png)
We should clean that up.
Also, since we are touching the Operationhandler API anyways, I was wondering wether we also should refactor and deprecate the CreateEdgeOperationHandler and CreateNodeOperationHandler sub interfaces and base implementations.
From what I can see, their only purpose right now is to enable code sharing of utility functions for the CreateNode use case. The CreateEdgeOperationHandler even is a completely empty marker interface. Maybe it would be better to move the utility functions in question into a util class and remove the unnecessary subtypes of CreateOperationHandler ?
|
|
||
| private String taskId; | ||
| private String expression; | ||
|
|
||
| public ApplyTaskEditOperation() { | ||
| public ApplyTaskEditAction() { |
There was a problem hiding this comment.
I guess the client counterpart needs to be updated as well?
...mple.workflow/src/org/eclipse/glsp/example/workflow/taskedit/ApplyTaskEditActionHandler.java
Outdated
Show resolved
Hide resolved
| * @param operation operation | ||
| * @return the matching command for the given operation | ||
| */ | ||
| static Optional<Command> getExecutableCommand(final OperationHandlerRegistry registry, final Operation operation) { |
There was a problem hiding this comment.
Whats the purpose of this utility method? Wouldn't it be better to implement this as a direct method of the OperationHandlerRegistry ?
martin-fleck-at
force-pushed
the
issue-864
branch
from
6bdf2f3
to
f08e170
Compare
martin-fleck-at
force-pushed
the
issue-864
branch
from
f08e170
to
fca929e
Compare
Make OperationHandler take a generic type - Deprecate Abstract base classes and provide Basic*, EMF*, GModel* - Adapt handlers to use new base class - Elevate concept of command to OperationHandler for more control -- Move command handling from EMF to general operation handler - Let abstract handlers use recording command for compatibility -- Add new 'preExecute' method to abort execution - Minor fixes -- Convert ApplyTaskEditActionHandler from operation to action handler Fixes eclipse-glsp/glsp#864
martin-fleck-at
force-pushed
the
issue-864
branch
from
fca929e
to
0e09959
Compare
|
Awesome! Changes look good to me. 👍🏼 |
- Refactoring of OperationHandler type hierarchy API, see also eclipse-glsp/glsp-server#187 - Stay with executeOperation method as we manage command stack via Model Server - Merge interfaces and abstract operation handler classes - Update GLSP and Model Server dependencies Part of #47
…scribing (#49) * #48 Fix unsubscription on closing/reloading application Until now unsubscribe was only triggered if a diagram editor was closed but not on application close/refresh. Fixes #48 * #47 Update to latest GLSP version due to API breaks - Refactoring of OperationHandler type hierarchy API, see also eclipse-glsp/glsp-server#187 - Stay with executeOperation method as we manage command stack via Model Server - Merge interfaces and abstract operation handler classes - Update GLSP and Model Server dependencies Part of #47 * #43 Follow-up: Update maven settings and Jenkinsbuild - Remove maven config and only use custom maven settings if necessary (i.e. Jenkinsbuild) - Remove maven config and settings from project template - Update Jenkinsbuild (do not build project-template with base framework as we have a separate build job for this)
Elevate concept of command to OperationHandler for more control
-- Move command handling from EMF to general operation handler
Let abstract handlers use recording command for compatibility
-- Add new 'preExecute' method to abort execution
Rewrite compound operation handler to create compound command
Minor fixes
-- Convert ApplyTaskEditActionHandler from operation to action handler
Fixes eclipse-glsp/glsp#864