eclipsesource / graphical-lsp

Graphical language server platform for building web-based diagram editors
https://www.eclipse.org/glsp
Eclipse Public License 2.0
37 stars 8 forks source link

Migrate to injectable commands and views #174

Closed planger closed 5 years ago

planger commented 5 years ago

The API for creating commands and views has been improved in Sprotty with https://github.com/eclipse/sprotty/issues/45. This issue is to track the modifications needed in GLSP to take advantage of this new API enabling injectable commands and views.

planger commented 5 years ago

Currently we have a bunch of action handler in GLSP that handle all sorts of actions, mostly "control"-like actions, such as showing and hiding the UI extensions, handling actions sent from the server, etc. Looking at the code a bit more and having migrated a few of them, I'm wondering whether we should actually migrate them to injectable commands after all rather than handling them in dedicated action handlers as we have now.

What do you think @tortmayr?

Having them as commands seems a bit more lightweight. However, they are also not really typical commands, as commands are sort-of designed to modify the SModel in some way, which most of our action handlers don't.

I currently tend to keep them and establish as a design guideline:

This way we have a clear distinction and also avoid having those commands as system commands on the command stack, etc. Let me know what you think?

tortmayr commented 5 years ago

As you already mentioned, most of these action handlers are not really representing a typical sprotty command ( undo-/ redoable modification of the SModel). IMHO keeping them as is and use your proposed design guidelines seems like the best option.

planger commented 5 years ago

Agreed, let's close this issue then.