Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#122 #123 Centralize configuration in the server #90

Conversation

CamilleLetavernier
Copy link
Member

  • Merge ServerLayoutConfiguration into DiagramConfiguration
  • Centralize model update protocol in Model Submission Handler

This addresses both eclipse-glsp/glsp/issues/122 and eclipse-glsp/glsp/issues/123, as the changes to needsClientLayout affects the way the ModelSubmissionHandler is implemented.

- Merge ServerConfiguration and DiagramConfiguration
- Centralize model update protocol in Model Submission Handler

fixes eclipse-glsp/glsp/issues/122
fixes eclipse-glsp/glsp/issues/123

Signed-off-by: Camille Letavernier <cletavernier@eclipsesource.com>
@CamilleLetavernier
Copy link
Member Author

Note: I also have the corresponding client-side changes (Which simply removes the needsClientLayout parameter from the client); I'll create a separate PR if these changes are fine. The client-side changes aren't actually required (strictly speaking), as the extra parameter would simply be ignored by the server.

Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I only have to minor change requests 😉

default ServerLayoutKind getLayoutKind() { return ServerLayoutKind.NONE; }

default boolean needsClientLayout() {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default should be true. Unless label sizes are computed explicitly on the server side (which isn't the case in any of our current usecases) we always need client layouting

* The {@link GModelState}
* @return
* A list of Actions to be processed in order to submit the model
*/
public List<Action> doSubmitModel(final boolean update, final GModelState modelState) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just call it submitModel?

@planger
Copy link
Member

planger commented Dec 14, 2020

This looks very useful to me! Can we get this merged soon?
I'd like to address eclipse-glsp/glsp#119 based on this change too, because the model submission handler could just invoke an extended version of the model factory to translate model state (with the custom model source) into the new version of the gmodel before submitting it to the client.
This would then also centralize the translation from original model source to gmodel.

@planger
Copy link
Member

planger commented Dec 16, 2020

Closed in favor of #95

@planger planger closed this Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants