search

MopeSWTP-SS21 / MopeSWTP

MIT License
1 stars 0 forks source link

Find better Place for OMC.Connect #78

Open manuEbg opened 3 years ago

manuEbg commented 3 years ago

We need to find a better place for this Line .

Reasons:

Propsal: Create new RPC-Method connectCompiler, which obviously connects to the compiler, and maybe sets some kind of variable that allows other functions like checkModel to report an error if compiler is not connected instead of failing....

CSchoel commented 3 years ago

Objection! :speaking_head: :point_right:

Please think about your users, in this case the developers that have to implement the client. Everything you do inside the LSP framework comes almost free of cost at the client side, because you can use standard LSP libraries. Custom JSON RPC commands, however, require custom code at the client side. Therefore, there should be a very good justification for each JSON RPC call. The whole point of using LSP is to make the implementation of clients as easy as possible.

In this case, I see no justification at all for a custom call. When the client asks the server to initialize, it is reasonable to expect that the server will do everything required to be able to receive any other command. If this causes issues with multiple clients, you have to a) start a separate OMC instance for each client or b) check whether the OMC is already running and only issue a new connection if this is not the case.

Also as a side note I think your link leads to the wrong line of code. In the title you talk about OMC:connect, but the link shows the line associated with #74 .

manuEbg commented 3 years ago

Correct the Link was wrong... not sure how this happened:) Thanks for pointing this out!

I see the Problem you pointed out regarding another RPC command... But i thought it might be useful to allow a client to trigger a "reconnect" to the compiler.

Also I thought that the InitializeMethod intended to be more kind of a handshake and capability exchange than "Initialization".

CSchoel commented 3 years ago

I think the idea of the initialize method is twofold: The first part is a handshake, exactly as you said, and the second part why it is useful is that it provides a time window where client and server know about each other, but do not actively send messages. That time window is exactly what we need to connect to the OMC. Otherwise, as you have already noted, you would have to implement error handling procedures for virtually any other request that can be made by the client, because it would fail if the OMC is not connected.

Nothing stops you from implementing a separate reset/reconnect command, but again: Think from the user's point of view. If you have the option, do you want to send two commands to the server in order to start working on your project, or rather just one? KISS.

manuEbg commented 3 years ago

Nothing stops you from implementing a separate reset/reconnect command, but again: Think from the user's point of view. If you have the option, do you want to send two commands to the server in order to start working on your project, or rather just one?

I don't want to send two commands 😊