GLSP-974: Use interface-injection for all subclasses of ModelState #199
Conversation
|
Nice! Maybe is eclipse-glsp/glsp#896 or is/could be addressed in this go? |
I'll take a look 👍 |
|
I believe this is fixed by this PR, as we now bind the class only once. Extra bindings only specify additional interface-bindings for the same class. However I'm not really sure how to test it. Is there an example where I could try it? |
That's what I also was thinking looking at the code.
Well, I think it should be sufficient if you'd remove the override of bindGModelState() in your subclass of EMFDiagramModule. The use case was basically having a concrete DiagramModule that extends EMFDiagramModule but uses the EMFModelState instead of explicitly binding a specialization of EMFModelState. |
- Make sure a single instance of the ModelState will be used, regardless of which (sub-)type is used for injecting it refs eclipse-glsp/glsp#974
There was a problem hiding this comment.
Thanks a lot @CamilleLetavernier!
Changes look great to me, I did not really test it, but it will be easier to test it with the already opened PR in the modelserver-glsp-integration.
Besides that I only have a few very minor comments, mostly regarding the year ranges in the file headers.
plugins/org.eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/EMFModelState.java
Show resolved
Hide resolved
plugins/org.eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/EMFModelStateImpl.java
Outdated
Show resolved
Hide resolved
....eclipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/notation/EMFNotationModelState.java
Outdated
Show resolved
Hide resolved
...lipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/notation/EMFNotationDiagramModule.java
Outdated
Show resolved
Hide resolved
...ipse.glsp.server.emf/src/org/eclipse/glsp/server/emf/notation/EMFNotationModelStateImpl.java
Outdated
Show resolved
Hide resolved
plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/model/DefaultGModelState.java
Outdated
Show resolved
Hide resolved
plugins/org.eclipse.glsp.server/src/org/eclipse/glsp/server/model/DefaultGModelState.java
Show resolved
Hide resolved
- Also remove accidental debug statements refs eclipse-glsp/glsp#974
CamilleLetavernier
force-pushed
the
issues/974
branch
from
41cdf65 to
41c6970
Compare
- Update Changelog to describe the changes to GModelState dependency-injection refs eclipse-glsp/glsp#974
|
Commit a0350bf updates the Changelog for 2.x, as this PR includes breaking changes related to EMFModelState and EMFNotationModelState |
…clipse-glsp#199) GLSP-974: Use interface-injection for all subclasses of ModelState - Convert subclasses of GModelState to interface and add corresponding class implementation - Make all ModelState classes singletons - Make sure a single instance of the ModelState will be used, regardless of which (sub-)type is used for injecting it - Update Changelog to describe the changes to GModelState dependency-injection refs eclipse-glsp/glsp#974
refs eclipse-glsp/glsp#974