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

remove dependency to monaco-languageclient #7100

Closed
akosyakov opened this issue Feb 7, 2020 · 28 comments · Fixed by #8131
Closed

remove dependency to monaco-languageclient #7100

akosyakov opened this issue Feb 7, 2020 · 28 comments · Fixed by #8131
Assignees
Labels
lsp language server protocol monaco issues related to monaco vscode issues related to VSCode compatibility

Comments

@akosyakov
Copy link
Member

akosyakov commented Feb 7, 2020

Right now it is quire hard to catch up with Monaco because we have to upgrade monaco-languageclient each time and make sure that translation between vscode api and lsp works in the browser context as well as vscode-languageclient (which actually written for Node.js).

Since we have possibility to run language servers as part of vscode extensions. I suggest that we simplify and get rid of monaco-languageclient. A consequence is that existing Theia extensions contributing language features then should be rewritten as VS Code extensions.

After that process of upgrading to latest Monaco will look like:

  • publish not-tree shaken version of monaco from theia-ide/vscode
  • bump up a version in Theia and test it

I hope we will be able to add integration tests for all exposed internal Monaco apis and ideally move to nightly builds and testing of not-tree shaken monaco against Theia soon.

cc @svenefftinge @marcdumais-work @eclipse-theia/ecd-theia-committers

@RomanNikitenko @azatsarynnyy it is not something to work on right now.

@akosyakov akosyakov added monaco issues related to monaco lsp language server protocol vscode issues related to VSCode compatibility proposal feature proposals (potential future features) labels Feb 7, 2020
@akosyakov
Copy link
Member Author

Added to next dev meeting.

@spoenemann
Copy link
Contributor

A consequence is that existing Theia extensions contributing language features then should be rewritten as VS Code extensions.

We should make sure we have a clear story for packaging VS Code extensions with Theia Electron apps first. AFAIK this has not been properly tested yet.

@akosyakov
Copy link
Member Author

akosyakov commented Feb 7, 2020

@spoenemann there is a PR in theia-apps already for it: theia-ide/theia-apps#299

@spoenemann
Copy link
Contributor

Just because it works with the example app by running yarn start does not mean it will automatically work after running electron-builder. That's what I mean by "not properly tested yet".

@akosyakov
Copy link
Member Author

Just because it works with the example app by running yarn start does not mean it will automatically work after running electron-builder. That's what I mean by "not properly tested yet".

@spoenemann it should be a goal of theia-ide/theia-apps#299 cc @marechal-p

@marcdumais-work
Copy link
Contributor

@akosyakov I assume this proposal also impacts debug extensions the same way? Do Theia plugins already have the ability to run language extensions without our version of monaco-languageclient?

@akosyakov
Copy link
Member Author

@marcdumais-work debugging and plugin system don’t depend on monaco-languageclient if there is some code reuse we will move it to this repo. I don’t expect much.

@akosyakov akosyakov removed the proposal feature proposals (potential future features) label Feb 11, 2020
@akosyakov
Copy link
Member Author

It was discussed that we do it, but first Theia extensions which we maintain should be migrated to VS Code extensions, particularly cpp and yang extensions.

@tsmaeder
Copy link
Contributor

tsmaeder commented Feb 12, 2020

@akosyakov note that the cpp extension from microsoft has a problematic license: https://marketplace.visualstudio.com/items/ms-vscode.cpptools/license
You're only allowed to use it with VS products.

@tsmaeder
Copy link
Contributor

There is a Eclipse CDT-based extensions, but can't find it on the marketplace, right now.

@akosyakov
Copy link
Member Author

@tsmaeder yes, i meant https://github.com/eclipse-theia/theia-cpp-extensions/tree/master/packages/cpp should become VS Code extension instead

@akosyakov
Copy link
Member Author

We probably should start with a PR which prove that we can get rid of monaco-languageclient and only after that migrate to VS Code extensions. It would be bad to migrate everything and learn afterwards that we cannot drop it for some reasons.

@marcdumais-work
Copy link
Contributor

There is a Eclipse CDT-based extensions, but can't find it on the marketplace, right now.

I had in mind that I heard of such an extension. Looking for it, I think it's this one: https://github.com/eclipse-cdt/cdt-vscode

I do not think it was ever published. Looking at the code, it looks like work-in-progress.

@tsmaeder
Copy link
Contributor

@marcdumais yeah, not the most active. Do we have volunteers to maintain a VS Code-CPP alternative? Problem is: VS Code users won't care about the license because it works for them.

@vince-fugnitto
Copy link
Member

@marcdumais yeah, not the most active. Do we have volunteers to maintain a VS Code-CPP alternative? Problem is: VS Code users won't care about the license because it works for them.

I think we (Ericsson) and ARM have been the main maintainers of the theia-cpp-extensions repository (due to internal needs), and will likely be the ones to migrate the extension and help maintain it. The extension itself will be useful for anyone who requires C/C++ support in a Theia-based application (since they cannot distribute the vscode counterpart).

@tsmaeder
Copy link
Contributor

Added a topic to the next dev-meeting to discuss where and how vscode-cpp-alt should live.

@benoitf
Copy link
Contributor

benoitf commented Feb 13, 2020

I think it was already discussed at the previous meeting

@tsmaeder
Copy link
Contributor

There is nothing in the minutes of the last two meetings 🤷‍♂

@vince-fugnitto
Copy link
Member

I think internally we might verify if the vscode-clangd might be a suitable replacement for the C/C++ language features as it already has a strong user-base, and is supported by LLVM.

Additional Information:

@tsmaeder
Copy link
Contributor

My Cpp-wrangling days are long over. My concern is how well it works for non-clang compilers.

@marcdumais-work
Copy link
Contributor

My Cpp-wrangling days are long over. My concern is how well it works for non-clang compilers.

Our existing @theia/cpp also uses clangd, so I think it's a wash from that PoV.

From what I understand, clangd is supposed to work well with any ANSI C compiler, so long as a "compilation database" is provided.

There are tools specialised in generating this "database", e.g. cmake can generate it, else something like bear can intercept commands sent to the compiler and output the compile database.

@Livven
Copy link
Contributor

Livven commented Feb 18, 2020

@tsmaeder

note that the cpp extension from microsoft has a problematic license: https://marketplace.visualstudio.com/items/ms-vscode.cpptools/license
You're only allowed to use it with VS products.

Seems like that only applies to the Marketplace distributed version, not to the source code which is MIT?

@tsmaeder
Copy link
Contributor

tsmaeder commented Feb 18, 2020

As I remember it's the runtime bits (debugger) that are the problem, no the VS Code extension.

@marcdumais-work
Copy link
Contributor

As I remember it's the runtime bits (debugger) that are the problem, no the VS Code extension.

There is as well another license file under the Extension folder: https://github.com/microsoft/vscode-cpptools/blob/master/Extension/LICENSE.txt :

[...]
6. SCOPE OF LICENSE. The software is licensed, not sold. This
   agreement only gives you some rights to use the software. Microsoft
   reserves all other rights. Unless applicable law gives you more rights
   despite this limitation, you may use the software only as expressly
   permitted in this agreement. In doing so, you must comply with any
   technical limitations in the software that only allow you to use it in
   certain ways. You may not
      *  work around any technical limitations in the software;
      *  reverse engineer, decompile or disassemble the software, or attempt
         to derive the source code for the software, except and to the extent
         required by third party licensing terms governing use of certain open
         source components that may be included with the software;
      *  remove, minimize, block or modify any notices of Microsoft or its
         suppliers in the software;
      *  use the software in any way that is against the law; or
      *  share, publish, rent, or lease the software, or provide the software
         as a stand-alone hosted solution for others to use.

@akosyakov
Copy link
Member Author

It depends on #3985 as well as replacing Theia json extension with VS Code built-in.

@akosyakov
Copy link
Member Author

akosyakov commented Jul 2, 2020

@kittaakos brought up that removing @theia/languages will cause loose of semantic highlighting functionality, since we don't support it yet via Theia plugin system. So landing #7697 is a prerequisite as well.

akosyakov added a commit that referenced this issue Jul 2, 2020
It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@kittaakos
Copy link
Contributor

will cause loose of semantic highlighting functionality,

@akosyakov, what about the call- and type-hierarchy features?

@akosyakov
Copy link
Member Author

@kittaakos We try to preserve it based on VS Code APIs or Monaco internals. If not we drop them and file issues to support in the upstream first.

akosyakov added a commit that referenced this issue Jul 5, 2020
It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Jul 5, 2020
These APIs will be removed in next release via #8131

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Jul 8, 2020
These APIs will be removed in next release via #8131

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
RomanNikitenko pushed a commit that referenced this issue Jul 30, 2020
It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
RomanNikitenko pushed a commit that referenced this issue Aug 3, 2020
It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue Aug 3, 2020
It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
minyoungyang pushed a commit to minyoungyang/theia that referenced this issue Aug 4, 2020
…eclient`

It allows to consume Monaco directly without trying to run vscode-languageclient in the browser. One should contributed language smartness via VS Code extensions instead.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp language server protocol monaco issues related to monaco vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants