PR: Migrate introspection services to use the Language Server Protocol (LSP) #4751
Conversation
|
|
||
| # Spyder editor capabilities | ||
| EDITOR_CAPABILITES = { | ||
| "workspace": { |
There was a problem hiding this comment.
Does the editor supports all the following functionalities?
There was a problem hiding this comment.
I don't know what these terms refer to. Could you add a comment above each one to understand what they mean?
spyder/utils/code_analysis/client.py
Outdated
|
|
||
|
|
||
| parser = argparse.ArgumentParser( | ||
| description='ZMQ Python-based MS Language-Server v3.0 client for Spyder') |
There was a problem hiding this comment.
So do you plan to use zmq sockets? Why not regular sockets?
There was a problem hiding this comment.
Well I saw that the current implementation is based on ZMQ, but we could use also dedicated sockets per each editor connection, what do you prefer @ccordoba12 @goanpeca?
There was a problem hiding this comment.
Shall we continue using zmq? @ccordoba12 @goanpeca
There was a problem hiding this comment.
I have no opinion, if using zmq makes the core easier to read and write, then why not?
There was a problem hiding this comment.
Yes, please do. I think Zmq sockets are more robust than plain sockets.
| # WorkspaceClientCapabilities define capabilities the | ||
| # editor / tool provides on the workspace | ||
|
|
||
| WORKSPACE_CAPABILITIES = { |
There was a problem hiding this comment.
@ccordoba12 Are the capabilities descriptions more clear than the last time?
spyder/utils/code_analysis/client.py
Outdated
| pass | ||
| client.stop() | ||
| sig_manager.restore() | ||
| os.kill(os.getpid(), signal.SIGTERM) |
There was a problem hiding this comment.
This doesn't work on Windows, i.e. processes can't be killed this way there. If you want a cross-platform way of doing this, please take a look at psutil.kill.
andfoy
force-pushed
the
language_server_adapter
branch
from
c2685b0 to
db63b91
Compare
| # code lenses for a given text document. | ||
| # A code lens represents a command that should be shown along with | ||
| # source text, like the number of references, a way to run tests, etc. | ||
| "codeLens": { |
There was a problem hiding this comment.
I think Spyder doesn't support code lens, does it?
There was a problem hiding this comment.
ehhh not sure I understand what it does?
There was a problem hiding this comment.
For example, showing who was the last one that modified a line
There was a problem hiding this comment.
No, although It'll be a great addition, (and integration with git), I'll look forward to implement this after you finish this PR 😄
|
@andfoy, this is terrific! Finally all is green, yay!! However, this PR will add almost 3400 lines! What modules or files do you want us to review in depth? |
It would be good to review the changes done on codeeditor, as they represent the final achievement of this PR. Also, it would be nice to do some kind of beta testing for the feature replacement on a real scenario, such that we can test the actual performance of the client and to certify that it does not hang when performing several requests concurrently. |
|
@ccordoba12 Could you please give this PR a try? |
|
@ccordoba12 I've added an option to set LSP options as a JSON object on the LSP configuration panel. With this addition, the PR should be complete.
|
andfoy
force-pushed
the
language_server_adapter
branch
from
9636fe9 to
9b0b29f
Compare
|
Tasks before merging this PR:
|
|
Also:
|
andfoy
force-pushed
the
language_server_adapter
branch
from
d023e33 to
41b5563
Compare
|
@andfoy, the error in Circle seems valid, unless you prefer to skip testing |
andfoy
force-pushed
the
language_server_adapter
branch
from
d09306d to
41b5563
Compare
|
@ccordoba12 This one should be ready |
There was a problem hiding this comment.
Great job @andfoy!! Thanks a lot for helping us with this one!!
Fingers crossed, this is going in!
Fixes #4742
On this PR we will adapt editor code introspection and linting functionality to comply with Microsoft/language-server-protocol specifications. During this PR, the following tasks must be accomplished:
Start specific language LSP server when a file of that language is first openedClose specific language LSP server when there aren't any files of that language openedHandle hover callsIntegrate lsp-client to perform symbol searchingIntegrate lsp-client to handle code formattingAdd client/transport testsNote: Crossed out features of this list are to be implemented/discussed on subsequent PRs, please see: #4751 (comment)