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

Adapt code introspection, autocompletion and linting to comply with the Language Server Protocol #4742

Closed
andfoy opened this issue Jul 13, 2017 · 23 comments · Fixed by #4751
Closed

Comments

@andfoy
Copy link
Member

andfoy commented Jul 13, 2017

We should write a full-compliant Microsoft/language-server-protocol client. This should enable us to support editing capabilities such as Linting and Autocompletion on Python and other languages only by installing the correct server.

This reform also aims to simplify the current editor client-server architecture by only using a single client/server that performs all editing and code introspection tasks.

@ccordoba12
Copy link
Member

I think there's no need to write a client for this because there's already one and it should be easier for us to contribute to it.

The main problem is to make it work with two completion libraries (Rope and Jedi).

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

@ccordoba12 Actually, palantir/python-language-server implements its autocompletion services using Jedi

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

We need to write a client that links the server with the respective Spyder functions, using the language-server protocol

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

We need to implement the tool side of this Diagram, as the server is already implemented
Language-server-protocol

@goanpeca
Copy link
Member

The server and the tool are the same thing in the python case afaik

@ccordoba12
Copy link
Member

@andfoy, @goanpeca is right. You can instantiate a new server and use it as a client.

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

I've just tested the server side, can I see an example of the server acting as a client?

@ccordoba12
Copy link
Member

From the Spyder side, I mean :-)

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

I must say I got confused here! From the language-server perspective we have our python-language-server and we're missing the tool side. From the Spyder side we have a server that communicates with Qt? If this is the case, our Spyder server would act as a client for the python-language-server? @ccordoba12

@ccordoba12
Copy link
Member

can I see an example of the server acting as a client?

Please take a look at

https://github.com/palantir/python-language-server/blob/develop/test/test_language_server.py#L14

From the Spyder side we have a server that communicates with Qt?

We have a server that acts as a client, makes requests to the real server, gets its results and pass them to our completion widget and linting facilities.

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

https://github.com/palantir/python-language-server/blob/develop/test/test_language_server.py#L14

In this case they initialize another JSONRPC server to perform the requests, this doesn't mean that the client and the ls-server are a single entity. This could serve as an alternative, However I was thinking of communicating both sides using TCP sockets instead of system pipes, just in case another server does not support pipe communication.

We have a server that acts as a client, makes requests to the real server, gets its results and pass them to our completion widget and linting facilities.

Now everything is more clear, thanks :-)

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

@ccordoba12 Do we prefer plain TCP communication or system pipes? Using TCP will allow us to communicate with remote l-s servers

@ccordoba12
Copy link
Member

However I was thinking of communicating both sides using TCP sockets instead of system pipes, just in case another server does not support pipe communication.

Ok, I agree with that. Then we would need to develop a new client for that, right?

Now everything is more clear, thanks :-)

Sorry for the lack of info, I thought this was clear when we talked about it during the sprint.

@andfoy
Copy link
Member Author

andfoy commented Jul 13, 2017

Then we would need to develop a new client for that, right?

Yes, but is not more difficult than serializing JSON and sending it through a socket :-)

Sorry for the lack of info, I thought this was clear when we talked about it during the sprint.

Don't worry, it was my fault, I got confused by the ambiguity of the word server, I was thinking only from the l-s perspective

@goanpeca
Copy link
Member

Yes, but is not more difficult than serializing JSON and send it through a socket :-)

Agreed :-p

@ccordoba12
Copy link
Member

Ok, great! Please start to work on that then :-)

@andfoy
Copy link
Member Author

andfoy commented Jul 14, 2017

Just a minor question, all current introspection requests are handled by the IntrospectionManager, given that, then we would redirect requests directed to the Manager to the ls-server.

@andfoy
Copy link
Member Author

andfoy commented Jul 14, 2017

With respect to the Linting functionalities, is it possible to know where are they declared?

@ccordoba12
Copy link
Member

we would redirect requests directed to the Manager to the ls-server.

Yes, I think that's the idea.

where are they declared?

Pep8 and Pyflakes analysis run in their own threads. They are declared here

https://github.com/spyder-ide/spyder/blob/master/spyder/widgets/editor.py#L214-L221

and the thread manager is declared here

https://github.com/spyder-ide/spyder/blob/master/spyder/widgets/editor.py#L76-L156

@goanpeca
Copy link
Member

@ccordoba12 but the idea is to stop using the current logic, right? it is too confusing.

@ccordoba12
Copy link
Member

Yeah, the idea is to remove all our code for code completion and replace it for ls-protocol. But we could probably need to preserve the IntrospectionManager to handle calls to several completion engines.

@andfoy
Copy link
Member Author

andfoy commented Jul 14, 2017

Yeah, the idea is to remove all our code for code completion

This means that all IntrospectionPlugin implementations are to be removed with this change?

@ccordoba12
Copy link
Member

Yes, I think so. We need to rely only on ls-protocol.

@ccordoba12 ccordoba12 changed the title Adapt Code Introspection, Autocompletion and Linting editor functionalities to comply with MS/VSCode Language Server specification Adapt code introspection, autocompletion and linting to comply with the Language Server Protocol May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment