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

#421 Improve implementation of DefaultGLSPServer #133

Merged
merged 2 commits into from
Oct 18, 2021

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Oct 17, 2021

#421 Improve implementation of DefaultGLSPServer

  • Update DefaultGLSPServer implementation so that request methods throw ResponseErrorExceptions instead of GLSPServerExceptions (see also LSP4J core concepts
    -Refactor and rename createClientSession method of ClientSessionManager to getOrCreateClientSession
  • Update behavior of initialize and initializeClientSession requests to enable multiple invocations with the same parameters.

#423 Rework "ServerConnectionListener"
Refactor ServerConnectionListener to GLSPServerListener and allow listening to the following events:

  • GLSPClient proxy connection
  • Server initialization
  • Server shutdown

This also means that the server no longer needs to new which resources it should dispose on shutdown. Instead classes that should be disposed can listen on the shutdown-event and dispose themselves (e.g. ClientSessionManager)

Also: Improve general extensibility by changing the visibility of private fields of classes in the websocket package to protected
Improve interface documentation and mention when methods should notify a listener.

Fixes eclipse-glsp/glsp/issues/421
Fixes eclipse-glsp/glsp/issues/423

Required for eclipse-glsp/glsp/issues/418

#421 Improve implementation of DefaultGLSPServer
- Update `DefaultGLSPServer` implementation so that request methods throw `ResponseErrorExceptions` instead of `GLSPServerExceptions` (see also [LSP4J core concepts](https://github.com/eclipse/lsp4j/blob/main/documentation/jsonrpc.md#response-errors)
-Refactor and rename `createClientSession` method of `ClientSessionManager` to `getOrCreateClientSession`
- Update behavior of `initialize` and `initializeClientSession` requests to enable multiple invocations with the same parameters.

#423 Rework "ServerConnectionListener" 
Refactor `ServerConnectionListener` to `GLSPServerListener` and allow listening to the following events:
- GLSPClient proxy connection
- Server initialization
- Server shutdown 

This also means that the server no longer needs to new which resources it should dispose on shutdown. Instead classes that should be disposed can listen on the shutdown-event and dispose themselves (e.g. ClientSessionManager)

Also: Improve general extensibility by changing the visibility of private fields of classes in the websocket package to `protected`
Improve interface documentation and mention when methods should notify a listener.

Fixes eclipse-glsp/glsp/issues/421
Fixes eclipse-glsp/glsp/issues/423

Required for eclipse-glsp/glsp/issues/418
@planger planger self-requested a review October 18, 2021 07:15
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 great and still seems to work fine!
I appended a commit with minor suggestions for improvements.

@tortmayr tortmayr merged commit 03f418b into master Oct 18, 2021
@tortmayr tortmayr deleted the tortmayr/multi-client branch October 18, 2021 09:24
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.

Rework "ServerConnectionListener" Improve implementation of DefaultGLSPServer
2 participants