[preferences] use text models to update content #7110

akosyakov · 2020-02-09T11:43:11Z

What it does

fix Update of preferences from VSCode plugin triggers multiple overwrite popup confirmation #6845, fix [theming] changing themes quickly results in a dialog #6786: use text models to update content. It resolves following issues:
- edits are applied in the thread-safe fashion;
- dirty editors are respected -> actually it is not good, it is better to way till a model is saved, otherwise intermediate values are applied.
  - apply only saved values
fix ERROR TypeError: Cannot read property 'startLineNumber' of null when editing the preferences #7125, see the explanation: ERROR TypeError: Cannot read property 'startLineNumber' of null when editing the preferences #7125 (comment)
fix can't activate vscode python plugin when opening an ipynb file #7111, base language activation on monaco created modes
activate textmate languages only if a grammar is registered, otherwise activation can be triggered too early and lead to no coloring at all
don't swallow added file if it was deleted during debouncing, file resource rely on it to notify clients that is not valid anymore

How to test

over 800 vscode compatibility preference tests are migrated to be integration tests
(theming) test that changing themes does not lead to the conflict dialog anymore
(vscode) use gitlens welcome screen to change different configuration and check that it does not lead to the conflict dialog anymore
test 2 above steps with dirty editor on and off
modify preferences and test that only saved preferences are applied
check that formatting is respected when preference values are changed
check that files with jsonc language are properly colored on first load and on reload with preserved editor

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

packages/monaco/src/browser/monaco-editor-model.ts

examples/api-tests/src/launch-preferences.spec.js

akosyakov · 2020-02-10T12:06:28Z

@vince-fugnitto Could you check please whether you can reproduce the file conflict dialog while switching themes?

vince-fugnitto · 2020-02-10T12:10:52Z

@vince-fugnitto Could you check please whether you can reproduce the file conflict dialog while switching themes?

Sure, I'll verify if we still get the dialog when switching themes 👍

vince-fugnitto · 2020-02-10T12:23:39Z

@vince-fugnitto Could you check please whether you can reproduce the file conflict dialog while switching themes?

Sure, I'll verify if we still get the dialog when switching themes

I was unable to reproduce the problem 👍 It looks good to me in that regard :)

akosyakov · 2020-02-10T20:20:58Z

Still struggling to get fs events right, but the rest should be ready for the review.

akosyakov · 2020-02-11T17:00:03Z

Oh, now jsonc files don't have coloring, because language is contributed by @theia/preferences extension and it is not tracked, so grammars get ignored. It is the similar cause as for #7112 (comment)

akosyakov · 2020-02-11T20:35:42Z

@vince-fugnitto Could someone try it? I need it to continue working on git and gitlens extensions support in #6921

spoenemann · 2020-02-12T08:48:32Z

(vscode) use gitlens welcome screen to change different configuration and check that it does not lead to the conflict dialog anymore

How to test that?

Everything else works as described for me 👍

AlexTugarev

Going to test the changes now.

json coloring
ipynb loading
preferences in general
monaco workspace edits

packages/filesystem/src/node/file-change-collection.ts

packages/monaco/src/browser/monaco-editor-model.ts

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

packages/plugin-ext/src/main/browser/documents-main.ts

packages/plugin-ext/src/main/browser/text-editor-main.ts

packages/preferences/src/browser/abstract-resource-preference-provider.ts

akosyakov · 2020-02-12T09:11:07Z

@spoenemann that's a bit involving :) You need:

to uninstall @theia/git extension and rebuild the browser example app
install built-in git extension by uploading vsix file (https://github.com/theia-ide/vscode-builtin-extensions/releases/download/v1.39.1-prel/git-1.39.1-prel.vsix) into plugins folder
install gitlens vscode extension the same way
then you start Theia in Gitpod pass --hostname 0.0.0.0 to let webviews work
you should see welcome gitlens webview if not use command Gitlens: Welcome
then try to change configurations in this webview, you should not get any conflict dialogs and then see that preferences are actually applied in the preferences view

AlexTugarev · 2020-02-12T09:16:12Z

@akosyakov when saving changes in preference editor I see:

bundle.js:134921 root ERROR onWillSave listeners should provide edits, not directly alter the document.

akosyakov · 2020-02-12T09:18:07Z

bundle.js:134921 root ERROR onWillSave listeners should provide edits, not directly alter the document.

@AlexTugarev it is expected, it does not break save control flow, we just notify that there some save participants which don't provide edits, but mutate documents, the same on master

AlexTugarev · 2020-02-12T09:20:16Z

The alert is shown twice.

akosyakov · 2020-02-12T09:22:55Z

The alert is shown twice.

@AlexTugarev It is another issue with the preference widget. I did not intend to fix it. I guess it is because workspace and use preference files are changed.

AlexTugarev

I checked that preferences are working fine, and save actions in editors are ok. It's good to see the color-theme-changes are reflected nicely 👍
Well done!

akosyakov · 2020-02-12T10:24:49Z

@spoenemann @AlexTugarev thanks for testing and thorough review, i will apply discussed changes and merge if the build is green

RomanNikitenko

I checked that my use case was fixed, also I tried to modify the user preferences - I don't see any errors

vince-fugnitto · 2020-02-12T13:13:03Z

The alert is shown twice.

@AlexTugarev the issue is a pre-existing problem with the preferences: #2780

It resolve following issues: - dirty editors are respected - edits are applied in thread safe fashion Signed-off-by: Anton Kosyakov <[email protected]>

Signed-off-by: Anton Kosyakov <[email protected]>

Before we will activate grammars for all languages which is bogus since grammar is not necessary registered yet. Now we will trigger grammar activation only if it was registered. Signed-off-by: Anton Kosyakov <[email protected]>

akosyakov · 2020-02-12T13:52:20Z

packages/preferences/src/browser/abstract-resource-preference-provider.ts

-        } catch {
-            return undefined;
+            let preferences;
+            if (model.valid) {


@AlexTugarev since we don't reset model to empty value when resource is gone, i have to add such guard here that preferences don't get created from invalid model

akosyakov · 2020-02-12T14:21:04Z

ok, it looks good now, merging, if we have some tests instabilities because of fs event reporting, i will look into it as my top priority

akosyakov · 2020-02-12T14:53:03Z

so first build on master hit it 😬 looking into the fix

akosyakov commented Feb 9, 2020

View reviewed changes

packages/monaco/src/browser/monaco-editor-model.ts Outdated Show resolved Hide resolved

akosyakov commented Feb 9, 2020

View reviewed changes

examples/api-tests/src/launch-preferences.spec.js Outdated Show resolved Hide resolved

akosyakov self-assigned this Feb 9, 2020

akosyakov added monaco issues related to monaco preferences issues related to preferences vscode issues related to VSCode compatibility labels Feb 9, 2020

This was referenced Feb 9, 2020

Update of preferences from VSCode plugin triggers multiple overwrite popup confirmation #6845

Closed

[theming] changing themes quickly results in a dialog #6786

Closed

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from debe513 to 1b80e5f Compare February 10, 2020 12:06

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch 3 times, most recently from b5a5893 to 337f1a2 Compare February 10, 2020 17:08

akosyakov changed the title ~~WIP [preferences] use text models to update content~~ [preferences] use text models to update content Feb 10, 2020

akosyakov requested review from vince-fugnitto, kittaakos and svenefftinge February 10, 2020 20:15

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from 337f1a2 to 8daa99f Compare February 10, 2020 20:43

akosyakov mentioned this pull request Feb 11, 2020

ERROR TypeError: Cannot read property 'startLineNumber' of null when editing the preferences #7125

Closed

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from 8daa99f to 1cd13e6 Compare February 11, 2020 14:10

akosyakov requested a review from RomanNikitenko February 11, 2020 14:50

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from 1cd13e6 to 830838c Compare February 11, 2020 16:33

akosyakov mentioned this pull request Feb 11, 2020

listen to activate all languages even witout grammar contributions #7112

Closed

1 task

akosyakov requested review from AlexTugarev and spoenemann February 12, 2020 07:51

AlexTugarev reviewed Feb 12, 2020

View reviewed changes

AlexTugarev approved these changes Feb 12, 2020

View reviewed changes

RomanNikitenko approved these changes Feb 12, 2020

View reviewed changes

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from 17792f8 to 2b039bf Compare February 12, 2020 10:42

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from 73bebf2 to a8da83d Compare February 12, 2020 13:30

akosyakov added 3 commits February 12, 2020 13:50

[preferences] fix #6845: use text models to update content

aefea04

It resolve following issues: - dirty editors are respected - edits are applied in thread safe fashion Signed-off-by: Anton Kosyakov <[email protected]>

[vscode] fix #7111: activate by language based on created mode

4b3f78e

Signed-off-by: Anton Kosyakov <[email protected]>

akosyakov force-pushed the akosyakov/update-of-preferences-6845 branch from a8da83d to 00535be Compare February 12, 2020 13:50

akosyakov commented Feb 12, 2020

View reviewed changes

akosyakov mentioned this pull request Feb 12, 2020

various fixes to make git and gitlens vscode extension work #6921

Merged

15 tasks

akosyakov merged commit e1ec660 into master Feb 12, 2020

akosyakov deleted the akosyakov/update-of-preferences-6845 branch February 12, 2020 14:23

This was referenced Feb 13, 2020

Editors Sprint 179 eclipse-che/che#15902

Closed

Theia Color theme switching works improperly eclipse-che/che#15764

Closed

crawr mentioned this pull request Feb 25, 2020

Overwrite File Message - Do you want to overwrite changes made on file system? zowe/zowe-explorer-vscode#317

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[preferences] use text models to update content #7110

[preferences] use text models to update content #7110

akosyakov commented Feb 9, 2020 •

edited

Loading

akosyakov commented Feb 10, 2020

vince-fugnitto commented Feb 10, 2020

vince-fugnitto commented Feb 10, 2020

akosyakov commented Feb 10, 2020

akosyakov commented Feb 11, 2020

akosyakov commented Feb 11, 2020

spoenemann commented Feb 12, 2020

AlexTugarev left a comment •

edited

Loading

akosyakov commented Feb 12, 2020

AlexTugarev commented Feb 12, 2020

akosyakov commented Feb 12, 2020

AlexTugarev commented Feb 12, 2020

akosyakov commented Feb 12, 2020 •

edited

Loading

AlexTugarev left a comment

akosyakov commented Feb 12, 2020

RomanNikitenko left a comment •

edited

Loading

vince-fugnitto commented Feb 12, 2020

akosyakov Feb 12, 2020

akosyakov commented Feb 12, 2020

akosyakov commented Feb 12, 2020

[preferences] use text models to update content #7110

[preferences] use text models to update content #7110

Conversation

akosyakov commented Feb 9, 2020 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

akosyakov commented Feb 10, 2020

vince-fugnitto commented Feb 10, 2020

vince-fugnitto commented Feb 10, 2020

akosyakov commented Feb 10, 2020

akosyakov commented Feb 11, 2020

akosyakov commented Feb 11, 2020

spoenemann commented Feb 12, 2020

AlexTugarev left a comment • edited Loading

Choose a reason for hiding this comment

akosyakov commented Feb 12, 2020

AlexTugarev commented Feb 12, 2020

akosyakov commented Feb 12, 2020

AlexTugarev commented Feb 12, 2020

akosyakov commented Feb 12, 2020 • edited Loading

AlexTugarev left a comment

Choose a reason for hiding this comment

akosyakov commented Feb 12, 2020

RomanNikitenko left a comment • edited Loading

Choose a reason for hiding this comment

vince-fugnitto commented Feb 12, 2020

akosyakov Feb 12, 2020

Choose a reason for hiding this comment

akosyakov commented Feb 12, 2020

akosyakov commented Feb 12, 2020

akosyakov commented Feb 9, 2020 •

edited

Loading

AlexTugarev left a comment •

edited

Loading

akosyakov commented Feb 12, 2020 •

edited

Loading

RomanNikitenko left a comment •

edited

Loading