-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Upgrade to Monaco 0.20.0 #8010
Upgrade to Monaco 0.20.0 #8010
Conversation
c2fab83
to
e9a1640
Compare
@RomanNikitenko let me know if you need any input to move it forward, I'm going to start looking into dropping monaco-languageclient later this week, but it would be good to have this PR in first. |
I'm working on |
I had to pause my work on the issue because of some urgent issues on our side. I'm really sorry for the delay. |
It is fine. I think we will land removing monaco-langaugeclient only in 1.5.0 (in beginning of August). |
@RomanNikitenko I will spend this week adding more automated tests. Maybe also doing a follow-up PR for #7608 based on your PR. |
e9a1640
to
c6b3989
Compare
I'm working on |
c6b3989
to
16b96e0
Compare
Working on editor configurations alignment. |
I provided a separate PR #8185 as possible solution for compareEntries function problem as we have the issue for that #7899 |
977c516
to
fd9f111
Compare
Tested
|
fd9f111
to
0ed046b
Compare
Tested Tasks related functionality:
|
d5b2fda
to
201dedc
Compare
tests sometimes are failed, sometimes are successful on ci for my PR. @akosyakov |
@RomanNikitenko I will look tomorrow! |
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
90c8c3e
to
1dde8d9
Compare
I've rebased the PR and resolved #8010 (comment). I would like to merge it (if tests are green) and move to implement semantic highligting support in the plugin system. Does anyone has objections or would you like to test it more? @eclipse-theia/ecd-theia-committers I can open a PR against this PR if so. |
merge it as soon as possible so it can fit in upcoming release and it let some time to fix some bugs before cutting the release |
Unfortunately tests are not passing with Node.js 12. I will check what is wrong. |
@RomanNikitenko semantic of That's unfortunately won't fix other tests. They seem to be affected by rename test but not because of failure. A rather a fix which did last week that models without editors should be auto saved not opened. I will try to disable auto saving before tests to see whether it helps. |
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Also-by: Roman Nikitenko <rnikiten@redhat.com> Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
6727fb3
to
2a8a22f
Compare
Disabling auto save of dirty models without editors for tests helped. I will wait for builds and if they are green will merge. (Besides #8360, since it was failing before as well). |
Nope tests are still failing. I will debug more on Monday why models get persisted implicitly by tests. |
I was able to track down the bug in how we apply bulk edits now. There are 7 edits during rename for variable and references. We don't group them and apply atomically to the same model, but each in own tick, so revert get canceled and consequent tests are using the wrong model. |
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
2a8a22f
to
8ce6e71
Compare
I fixed bulk edits in the last commit and adjusted test expectations to await a single edit operation on rename or fail. |
Thank you @akosyakov for fixing it! |
What it does
TODO
ConfigurationChangeEvent
with VS CodefontInfo
was changed only, see for detailsfuzzySort: true
throws an error #7899 for more detailsfuzzySort: true
throws an error #7899 (comment)How to test
edit
menu with editorselection
menu with editor- done Upgrade to Monaco 0.20.0 #8010 (comment)
Review checklist
Reminder for reviewers