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

Bump monaco version #199

Merged
merged 11 commits into from
Mar 19, 2020
Merged

Bump monaco version #199

merged 11 commits into from
Mar 19, 2020

Conversation

RomanNikitenko
Copy link
Collaborator

@RomanNikitenko RomanNikitenko commented Jan 24, 2020

Bump Monaco to 0.19.3

I created the branch based on #198 and tried to fix the compilation errors.

NLueg and others added 7 commits December 6, 2019 12:15
A new enum has been added to the vscode compatibility layer (as
vscode-languageserver rely on it). Also the severity enum converter
use an internal version of the enum to avoid relying on the vscode
module.
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@gitpod-io-legacy-app
Copy link

Open in Gitpod - starts a development workspace for this pull request in code review mode and opens it in a browser IDE.

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@akosyakov
Copy link
Contributor

@RomanNikitenko ping me if you have any Qs

@RomanNikitenko
Copy link
Collaborator Author

RomanNikitenko commented Jan 27, 2020

@akosyakov
thank you very much for your support!

At the moment I have the problems with dependencies.
vscode-ws-jsonrpc brings dependency "vscode-jsonrpc": "^4.1.0-next" version

vscode-languageclient@^6.0.0 brings vscode-languageserver-protocol "^3.15.0" => vscode-jsonrpc "^5.0.0" version.

MessageConnection of 4.1.0-next is not compatible with the object of 5.0.0 version.

As result I get errors like:
[compile] src/browser/messaging/ws-connection-provider.ts(112,34): error TS2345: Argument of type 'import("/home/rnikitenko/projects/theia/node_modules/vscode-ws-jsonrpc/node_modules/vscode-jsonrpc/lib/main").MessageConnection' is not assignable to parameter of type 'import("/home/rnikitenko/projects/theia/node_modules/vscode-jsonrpc/lib/main").MessageConnection'.

So at the current step I'm looking for correct way to fix this problem.

@akosyakov
Copy link
Contributor

akosyakov commented Jan 28, 2020

@RomanNikitenko we need a new version of vscode-ws-jsonrpc then with 5.0.0. We will have the same issue in Theia.

Added you as an admin there: https://github.com/TypeFox/vscode-ws-jsonrpc just ping me when i have to make a new release

@akosyakov
Copy link
Contributor

@RomanNikitenko published vscode-ws-jsonrpc@0.2.0, you should be able to use it now

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@akosyakov
Copy link
Contributor

@RomanNikitenko Let me know if you need any other support. Usually we will do a dev release from the PR, open a PR for Theia against dev version and test it there.

@RomanNikitenko
Copy link
Collaborator Author

@akosyakov
About dev release - is it OK if within testing dev version I find something that we have to fix in monaco-languageclient repo and prepare another dev version?

I could test it locally first: add for theia repo dependency on these changes like:
"monaco-languageclient": "file:../../../monaco-languageclient/client",

Also for testing it for theia repo I need:

Upgrade TypeFox/monaco-editor-core to the latest Microsoft/monaco-editor-core

@akosyakov
Copy link
Contributor

About dev release - is it OK if within testing dev version I find something that we have to fix in monaco-languageclient repo and prepare another dev version

@RomanNikitenko yes, that's an idea. I will publish it today then?

Upgrade TypeFox/monaco-editor-core to the latest Microsoft/monaco-editor-core

I will do it for you today.

@RomanNikitenko
Copy link
Collaborator Author

@akosyakov

@RomanNikitenko yes, that's an idea. I will publish it today then?

I would like to pick up the changes from the PR #191 to have ability to test them as well on dev version for theia.
WDYT?

If you don't mind I can merge the changes from the specified PR to my draft PR.

@akosyakov
Copy link
Contributor

@RomanNikitenko yes, it is fine

@RomanNikitenko
Copy link
Collaborator Author

RomanNikitenko commented Feb 7, 2020

@akosyakov
I merged the specified PR changes to the current PR.

I think this one for testing goals only and we can streamline the commits when we are ready to merge these changes to master.
Please, let me know if we need to streamline the commits right now.

@akosyakov
Copy link
Contributor

akosyakov commented Feb 7, 2020

there's going to be Monaco 0.20.x soon 🙈 https://github.com/microsoft/vscode/tree/standalone/0.20.x

Should we rather go for it instead?

@akosyakov
Copy link
Contributor

Let's stick with 0.19.

@akosyakov
Copy link
Contributor

Published dev tag from this branch:

Successfully published:

  • monaco-languageclient@0.11.1-dev.4+6590ed7

Published 0.19.3 monaco from https://github.com/theia-ide/vscode/tree/standalone/0.19.x

  • @theia/monaco-editor-core@0.19.3

@RomanNikitenko
Copy link
Collaborator Author

@akosyakov
thank you very much!

I'll apply these versions for theia repo and prepare the corresponding PR for testing.

@akosyakov
Copy link
Contributor

@RomanNikitenko just open it is as a draft, we can collaborate on it, i.e. i can help writing integration tests while you are fixing others :) more of such we have less work it would be to migrate next time

@CGNonofr
Copy link
Collaborator

CGNonofr commented Mar 2, 2020

It seems we have an issue in the vscode-compatibility (vscode-api.ts): the workspaceFolders getter returns a new list containing a new Uri every time.

The consequence is that a DidChangeWorkspaceFoldersNotification is sent at startup, with an identical added and removed (same Uri but a different instance) and some LSP implementations don't like that at all!

@akosyakov
Copy link
Contributor

@RomanNikitenko I want to merge it and make a new release. Could you open for a review it please?

@RomanNikitenko RomanNikitenko marked this pull request as ready for review March 19, 2020 15:45
@RomanNikitenko RomanNikitenko changed the title [DRAFT] Bump monaco version Bump monaco version Mar 19, 2020
Copy link
Contributor

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @RomanNikitenko

@akosyakov akosyakov merged commit 95c9925 into master Mar 19, 2020
@akosyakov akosyakov deleted the bumpMonacoVersion branch March 19, 2020 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants