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

GLSP-1116 Revise model loading #211

Merged
merged 3 commits into from
Sep 15, 2023
Merged

GLSP-1116 Revise model loading #211

merged 3 commits into from
Sep 15, 2023

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Sep 12, 2023

GLSP-1116 Revise model loading

GLSP-1117: Remove need for explicit definition of client actions

Refactor the base GLSP protocol to allow the client to tell the server which actions it is going to handle i.e. which actions should be forwarded to the client

  • Add clientActions array to InitializeClientSessionParams. This means the client now has to pass the action kinds it wants to handle as part of the initalize request
  • Refactor ClientSessionManager API directly use the InitializeClientSessionParams object for creating new sessions. This means that the ClientSessionManager can also access the generic args properties that might have been passed with the initialize request.
  • Replace ClientActionHandler with ClientActionForwader a separate component that is not part of the server-side action handlers.
  • Remove configureClientActions method from DiagramModule as the explicit configuration is no longer needed
  • Refactor ClientIdModule to ClietnSessionModule responsible for injection session specific configuration like the clientId and the clientActions

Part of eclipse-glsp/glsp/issues/1117

GLSP-1071: Rename ServerStatus/ServerMessage action

Also remove unused ServerStatus class
Part of eclipse-glsp/glsp#1071

Copy link
Member

@planger planger left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! 👍 Just a few minor doc suggestions inline.

@tortmayr
Copy link
Contributor Author

@planger I have also added change for GLSP-1117 and GLSP-1071 to this PR. Since these are also changes that require updatse across all integration projects I would rather do this all in one go instead of having to create multiple PRs per repository.

Could you please re-review?

- Refactor `ModelSubmissionHandler` to support proper handling of the `RequestModelAction`
as real request action 
- Add support for rejecting request actions
Part-of: eclipse-glsp/glsp#1116
Part-of: eclipse-glsp/glsp#606
Refactor the base GLSP protocol to allow the client to tell the server which actions it is going to handle i.e. which actions should be forwarded to the client
- Add `clientActions` array to `InitializeClientSessionParams`. This means the client now has to pass the action kinds it wants to handle as part of the initalize request
- Refactor `ClientSessionManager` API directly use the `InitializeClientSessionParams` object for creating new sessions. This means that the `ClientSessionManager` can also access the generic `args` properties that might have been passed with the initialize request.
- Replace `ClientActionHandler` with `ClientActionForwader` a separate component that is not part of the server-side action handlers. 
- Remove `configureClientActions` method from `DiagramModule` as the explicit configuration is no longer needed
- Refactor `ClientIdModule` to `ClietnSessionModule` responsible for injection session specific configuration like the clientId and the clientActions

Part of eclipse-glsp/glsp/issues/1117
Copy link
Member

@planger planger left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! 👍

Also remove unused `ServerStatus` class
Part of eclipse-glsp/glsp#1071
@tortmayr tortmayr merged commit 1323e76 into master Sep 15, 2023
5 checks passed
@tortmayr tortmayr deleted the glps-1116 branch September 15, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants