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

Monaco integration #88

Closed
akosyakov opened this issue Sep 3, 2016 · 17 comments
Closed

Monaco integration #88

akosyakov opened this issue Sep 3, 2016 · 17 comments

Comments

@akosyakov
Copy link
Contributor

I want to connect monaco editor with a language server by means of this project.

I've managed to make use of jsonrpc module to create a client connection and hoped to reuse code and protocol converters, but it seems that types expected by vscode and monaco are not compatible, e.g. monaco.IRange vs vscode.Range.

I'm aware that monaco is assembled from vscode. Would it be possible to eliminate the difference?

@dbaeumer dbaeumer added this to the Backlog milestone Sep 8, 2016
@dbaeumer
Copy link
Member

dbaeumer commented Sep 8, 2016

@akosyakov in principal yes, however this is quite some work. Besides different names there are semantic differences as well (that is why the names are different at the end). For example in Monaco position informations are one based, whereas in the VS Code API they are zero based.

@georgewfraser
Copy link

It looks like sourcegraph is using the language server protocol with monaco.

@mbana
Copy link

mbana commented Oct 26, 2016

would be awesome if web sockets were used. it might then be easy to make a pure brower-based client without much trouble, i hope?

@akosyakov
Copy link
Contributor Author

@mbana i would prefer that transport should be interchangeable, as actually it is a case with vscode integration.

@dbaeumer
Copy link
Member

OK. The right approach would be to make the converters pluggable into the language client. There is already an interface for this in the corresponding file (e.g. Converter interface type). The converters also expose old legacy functions. When we make this pluggable we should remove the old function and break (e.g. version 3.0).

@mbana
Copy link

mbana commented Dec 8, 2016

@dbaeumer and @akosyakov,

so i tried out another approach, which although quite painful requires little code change in modules within vscode-languageserver-node. At the moment it's making a connection, see below, I just need to keep stubbing out more code.

LanguageClient:langserver-antha -  [Trace - 9:21:35 PM] Received response 'initialize - (0)' in 16ms. Request failed: invalid root path "": must be file:/// URI (0).
(index):137 LanguageClient:langserver-antha -  [Error - 9:21:35 PM] Server initialization failed.
(index):137 LanguageClient:langserver-antha -  Error: invalid root path "": must be file:/// URI
    at new ResponseError (http://127.0.0.1:8080/node_modules/vscode-languageclient/node_modules/vscode-jsonrpc/lib/main.js:371:32)
    at handleResponse (http://127.0.0.1:8080/node_modules/vscode-languageclient/node_modules/vscode-jsonrpc/lib/main.js:988:48)
    at WebSocketMessageReader.callback (http://127.0.0.1:8080/node_modules/vscode-languageclient/node_modules/vscode-jsonrpc/lib/main.js:1168:17)
    at WebSocketMessageReader.onMessage (http://127.0.0.1:8080/release/dev/goMode.js:524:18)
    at WebSocket.ws.onmessage (http://127.0.0.1:8080/release/dev/goMode.js:512:27)
goWorker.js:18createData:  Object
:8080/release/dev/goWorker.js:18 createData:
monaco: https://github.com/mbana/monaco-go/compare/languageclient?expand=1

effectively, it stubs or provides implementations for the things the LanguageClient requires. this part: https://github.com/mbana/monaco-go/compare/languageclient?expand=1#diff-eacf331f0ffc35d4b482f1d15a887d3b. then makes a WebSocket connection that implements StreamInfo, https://github.com/mbana/monaco-go/compare/languageclient?expand=1#diff-bc65e464774a8d1c47cdd321986e9c51R122.

vscode-languageserver-node: https://github.com/Microsoft/vscode-languageserver-node/compare/master...mbana:monaco?expand=1

main changes are:

  • move to AMD because requirejs isn't so easy to deal with.
  • make a single outFile for each lib.
  • export more definitions from client and jsonrpc.
  • IterableIterator doesn't play well with rjs optimizer so some small cleanup here.
  • jsonrpc/src/main.tsjsonrpc/src/vscode-jsonrpc.ts, again due to AMD not liking main.js.

basically, it has been painful due to rjs. or maybe i am just doing it wrong?


let me know what you think,

cheers

@mbana
Copy link

mbana commented Dec 15, 2016

@dbaeumer @akosyakov,
if anyone wants to give this a quick try, https://github.com/mbana/monaco-go.

ta,

@akosyakov
Copy link
Contributor Author

@dbaeumer I don't think that having converters is enough, it would be nice if LanguageClient does not have direct dependencies on vscode API. I would extract a base language client from LanguageClient with customization points for workspace, language, window and transport, so it does not have dependencies on node and vscode. These points should not use vscode APIs as well but could be modeled quite close. If you would like i can prototype a minimal change for it.

@mbana
Copy link

mbana commented Mar 10, 2017

@akosyakov makes a good point.
at least part of the code can be refactored to use vscode-uri instead of importing the whole of vscode. lines like the below can changed:

i believe monaco-editor already uses that above so you're certain it's going to work in the browser.

ref: https://github.com/Microsoft/vscode-languageserver-node/blob/master/client/src/codeConverter.ts

@dbaeumer
Copy link
Member

I agree to make this right we need to take the libraries apart in an VS Code independent part and a part the depends on VS Code. In addition we would need to factor out node dependencies which will hurt in the browser (Monaco) as well.

At the end of the day I am not sure if we should have this in one repository or only make json-rpc node independent and have a separate repository monaco-languageserver with the rest of the code. Introducing all these abstraction might not make the code easier to maintain.

@akosyakov
Copy link
Contributor Author

akosyakov commented Apr 10, 2017

I've been prototyping this approach in the fork: https://github.com/TypeFox/vscode-languageserver-node/tree/ak/vscode_independent_client

There are 2 ts modules introducing extensions:

  • services - for Languages, Workspace, Commands and Window APIs expressed in terms of the lsp
  • connection - for IConnection and its provider

then there are vscode implementation of services (not complete) and node implementation of connection, in our project we have also monaco implementation of services and web socket implementation of connection

regarding of maintenance:

interesting side effects:

  • one can use Language API to register editor agnostics features without any lang server
  • it could be used for other editors besides vscode and monaco

and there seem to be a bug:

  • feature handlers are not disposed when the client is stopped - TypeFox@90bab93

@dbaeumer
Copy link
Member

dbaeumer commented Apr 11, 2017

The API also aligned as some subtile differences. One major one is that the Monaco API is 1 based when taking to the editor and the VS Code is zero based. In addition that are difference in the document selector. So besides abstracting the client all converters need to consider the subtile differences. This is why I think it might be better to have a monaco-languageserver projects which might share some code with the vscode-languageserver project but not everything is mangled into one project.

Thanks for the bug report. I added:

for (let handler of this._registeredHandlers.values()) {
	handler.dispose();
}
this._registeredHandlers.clear();

@akosyakov
Copy link
Contributor Author

akosyakov commented Apr 28, 2017

We published monaco-languageclient, that is based on our fork of vscode-languageserver-node/client changed to be vscode and node independent as described above and published as vscode-base-languageclient. Ideally we would like to avoid having another module for vscode-languageserver-node, but use the original repository.

Regarding subtle differences:

@dbaeumer
Copy link
Member

dbaeumer commented May 8, 2017

@akosyakov I will look into providing the BaseLanguageClient this month so that you can get rid of the fork. Thanks for providing the monaco-languageclient

@dbaeumer
Copy link
Member

dbaeumer commented May 8, 2017

Objection to close this issue.

@akosyakov
Copy link
Contributor Author

We could keep it for discussions around BaseLanguageClient. If you think it is not necessary i am fine with closing it.

@dbaeumer
Copy link
Member

I pushed a commit to master that introduces the BaseLanguageClient. I will close this one. We should open specific issues if we encounter problems with the BaseLanguageClient.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 21, 2017
@dbaeumer dbaeumer removed this from the Backlog milestone Oct 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants