update monaco editor to 0.12.x #1681

AlexTugarev · 2018-04-10T14:17:23Z

I've started to update monaco, also because this includes updates of libraries and fixes issues like the broken java model sync. The work is suspended now due to some critical monaco issues, cf [1]. Especially the broken find references [2] is a show stopper.

[1] https://github.com/Microsoft/monaco-editor/milestone/11
[2] microsoft/monaco-editor#779

test exposed APIs (diff navigator, etc.)
fix keybinding conflicts, see update monaco editor to 0.12.x #1681 (comment)
fix JDT integration with latest monaco-languageclient, see update monaco editor to 0.12.x #1681 (comment)

AlexTugarev · 2018-04-11T11:46:35Z

There is some movement in the upstream, microsoft/monaco-editor#779 was fixed. Let's see if there is a new release on its way.

akosyakov · 2018-04-11T12:17:40Z

packages/monaco/package.json

@@ -1,58 +1,51 @@
 {
-  "name": "@theia/monaco",


@AlexTugarev please could you fix formatting in json files, should be 2 spaces

AlexTugarev · 2018-04-11T15:22:43Z

[email protected] was released. I'll give it a try.

kittaakos · 2018-04-11T15:53:55Z

packages/monaco/src/browser/monaco-loader.ts

                    global.monaco.modes = modes;
                    global.monaco.cancellation = cancellation;
+                    debugger;


Out of curiosity; what's that and what does this line do?

debugger is a leftover here. it's a breakpoint in code. bad habit, I know, but it was helpful, as chrome was not willing to install breakpoint here.
I'll remove it.

bad habit

Not at all, it was in WIP anyway. I wanted to squeeze some knowledge. Thanks!

kittaakos · 2018-04-11T15:58:09Z

packages/editor/src/browser/editor-decorations-service.ts

@@ -209,7 +209,7 @@ export namespace EditorDecorationStyle {
        const index = styleSheet.insertRule('.' + selector + '{}', 0);
        const rules = styleSheet.cssRules || styleSheet.rules;
        const rule = rules.item(index);
-        if (rule.type === CSSRule.STYLE_RULE) {
+        if (rule && rule.type === CSSRule.STYLE_RULE) {


What is your tsc --version? Is this related: #1621?

it's 2.7.2, but code-insiders is ahead and sometimes it fails to pick ts from node_modules. that change would still be ok with 2.7.x, I'll check that.

Sure, and please, ignore me; I was curious. Thanks!

ishitatsuyuki · 2018-04-12T04:53:13Z

dev-packages/application-manager/src/generator/webpack-generator.ts

@@ -39,7 +39,7 @@ const { mode }  = yargs.option('mode', {
 const development = mode === 'development';${this.ifMonaco(() => `

 const monacoEditorPath = development ? '${this.resolve('monaco-editor-core', 'dev/vs')}' : '${this.resolve('monaco-editor-core', 'min/vs')}';


It would be good to take a look at #1542 and the referenced microsoft/monaco-editor#18. Basically, this means we no longer need to copy all the monaco files separately, but bundle it into our main artefact.

yes, but it is out of the scope of this PR

How out of scope? This is only available on Monaco 0.11+ and we should do it upon the upgrade. I'm aware that this is not a blocker, but this is not out of scope either.

We do it with another PR, there are things to consider before switching from AMD to ESM, like the build speed. We had bad experience in monaco-languageclient. We want to consider it separately without blocking usage of the latest Monaco.

AlexTugarev · 2018-04-19T06:59:36Z

@kittaakos, have dealt with keybindings before? I think editor and diffEditor should be different contexts.

    Collided keybinding is ignored;  [ '{"command":"monaco.editor.action.wordHighlight.next","keybinding":"f7","context":"editorTextFocus"}',
      ' collided with ',
      '{"command":"monaco.editor.action.diffReview.next","keybinding":"f7","context":"editorTextFocus"}' ]

    Collided keybinding is ignored;  [ '{"command":"monaco.editor.action.wordHighlight.prev","keybinding":"shift+f7","context":"editorTextFocus"}',
      ' collided with ',
      '{"command":"monaco.editor.action.diffReview.prev","keybinding":"shift+f7","context":"editorTextFocus"}' ]

AlexTugarev · 2018-04-19T07:15:19Z

Did we consider inDiffEditor before? There are no keybinding conflicts with previous versions of monaco.

cc. @akosyakov

AlexTugarev · 2018-04-19T07:36:55Z

JDT LS integration is broken.

akosyakov · 2018-04-19T10:10:34Z

@AlexTugarev logging warning instead of throwing is fine with me

AlexTugarev · 2018-04-19T10:57:04Z

@akosyakov, I did not get that.

logging warning instead of throwing is fine with me

which error do you refer to?

AlexTugarev · 2018-04-24T10:28:00Z

created PR TypeFox/monaco-languageclient#74 to resolve issues with suggest widget.

svenefftinge

Great work, Alex! Thank you :)

akosyakov · 2018-04-27T05:59:13Z

packages/monaco/src/typings/monaco/index.d.ts

 /// <reference types='monaco-editor-core/monaco'/>

 declare module monaco.instantiation {
    export interface IInstantiationService {
    }
 }

-declare module monaco.editor {
+declare namespace monaco.editor {


@AlexTugarev Could you explain why it should be changed? (just out of curiosity)

I think it's a left over. I just tried out something while fixing typings. The monaco.editor is just a global spread and it should not be necessary to declare it a module. We also never import it, right?

@akosyakov maybe we need to change it with the transition to ESM version of monaco. I've reverted it here.

monaco is module, not namespace, it is a way how to augment modules.

@akosyakov it used to be declare module monaco but they changed it to declare namespace monaco.

it was changed sometimes between 0.11.1 and 0.11.6, I don't remember

ok, then it makes sense :)

I reverted it, but it won't be a problem. We'll revise it when it comes to using ESM.

Signed-off-by: Alex Tugarev <[email protected]>

svenefftinge · 2018-04-30T15:16:31Z

Tested again. Looks still good to me.

akosyakov reviewed Apr 11, 2018

View reviewed changes

AlexTugarev force-pushed the at/update-monaco branch from 3cfc723 to b62caff Compare April 11, 2018 12:29

AlexTugarev changed the title ~~WIP update monaco editor to 0.11.x~~ WIP update monaco editor to 0.12.x Apr 11, 2018

kittaakos reviewed Apr 11, 2018

View reviewed changes

ishitatsuyuki reviewed Apr 12, 2018

View reviewed changes

AlexTugarev force-pushed the at/update-monaco branch 3 times, most recently from 54bfb15 to 04b983a Compare April 19, 2018 06:52

This was referenced Apr 19, 2018

[Markers] No marker icon in the gutter next to the marker's line #1720

Closed

Remove jdt.ls from repository #1732

Merged

AlexTugarev force-pushed the at/update-monaco branch 4 times, most recently from 96a0a52 to a85ef4a Compare April 26, 2018 10:40

AlexTugarev changed the title ~~WIP update monaco editor to 0.12.x~~ update monaco editor to 0.12.x Apr 26, 2018

svenefftinge approved these changes Apr 26, 2018

View reviewed changes

AlexTugarev force-pushed the at/update-monaco branch from f40627a to b88e2db Compare April 26, 2018 13:26

akosyakov reviewed Apr 27, 2018

View reviewed changes

AlexTugarev force-pushed the at/update-monaco branch 3 times, most recently from f684585 to 14bdac0 Compare April 27, 2018 14:45

AlexTugarev force-pushed the at/update-monaco branch from 2f6f158 to 8ac3dd6 Compare April 30, 2018 08:51

update monaco to v0.12.0

54cf48f

Signed-off-by: Alex Tugarev <[email protected]>

AlexTugarev force-pushed the at/update-monaco branch from 3d822bf to 27550b4 Compare April 30, 2018 11:58

AlexTugarev added 4 commits April 30, 2018 15:51

[browser/test] pin wdio versions

3f554e7

Signed-off-by: Alex Tugarev <[email protected]>

[browser/test] enforce version of chrome driver

c13482e

Signed-off-by: Alex Tugarev <[email protected]>

[travis] increase max_old_space_size to avoid OOM errors

27d9313

Signed-off-by: Alex Tugarev <[email protected]>

[ci] bump yarn version to 1.6.0

2ad6d75

Signed-off-by: Alex Tugarev <[email protected]>

AlexTugarev force-pushed the at/update-monaco branch from b12a682 to 2ad6d75 Compare April 30, 2018 13:52

[ci] try stablize tests

b69576d

Signed-off-by: Alex Tugarev <[email protected]>

AlexTugarev merged commit a34acb8 into master Apr 30, 2018

AlexTugarev deleted the at/update-monaco branch April 30, 2018 15:31

This was referenced Apr 30, 2018

fix typo in test:theia task #1820

Merged

[monaco] fix quick open #1821

Merged

AlexTugarev mentioned this pull request May 8, 2018

Test against latest monaco and monaco-languageclient #1539

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update monaco editor to 0.12.x #1681

update monaco editor to 0.12.x #1681

AlexTugarev commented Apr 10, 2018 •

edited

Loading

AlexTugarev commented Apr 11, 2018

akosyakov Apr 11, 2018

AlexTugarev commented Apr 11, 2018

kittaakos Apr 11, 2018

AlexTugarev Apr 12, 2018

kittaakos Apr 12, 2018

kittaakos Apr 11, 2018

AlexTugarev Apr 12, 2018

kittaakos Apr 12, 2018

ishitatsuyuki Apr 12, 2018

akosyakov Apr 12, 2018

ishitatsuyuki Apr 12, 2018

akosyakov Apr 12, 2018 •

edited

Loading

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

akosyakov commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 24, 2018

svenefftinge left a comment

akosyakov Apr 27, 2018

AlexTugarev Apr 27, 2018

AlexTugarev Apr 27, 2018

akosyakov Apr 27, 2018

AlexTugarev Apr 27, 2018

AlexTugarev Apr 27, 2018

akosyakov Apr 27, 2018

AlexTugarev Apr 27, 2018

svenefftinge commented Apr 30, 2018

		@@ -39,7 +39,7 @@ const { mode } = yargs.option('mode', {
		const development = mode === 'development';${this.ifMonaco(() => `

		const monacoEditorPath = development ? '${this.resolve('monaco-editor-core', 'dev/vs')}' : '${this.resolve('monaco-editor-core', 'min/vs')}';

update monaco editor to 0.12.x #1681

update monaco editor to 0.12.x #1681

Conversation

AlexTugarev commented Apr 10, 2018 • edited Loading

AlexTugarev commented Apr 11, 2018

Choose a reason for hiding this comment

AlexTugarev commented Apr 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akosyakov Apr 12, 2018 • edited Loading

Choose a reason for hiding this comment

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

akosyakov commented Apr 19, 2018

AlexTugarev commented Apr 19, 2018

AlexTugarev commented Apr 24, 2018

svenefftinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenefftinge commented Apr 30, 2018

AlexTugarev commented Apr 10, 2018 •

edited

Loading

akosyakov Apr 12, 2018 •

edited

Loading