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

Workspace configuration that is registered by a VSCode plugin sometimes doesn't take effect #6046

Closed
a1994846931931 opened this issue Aug 26, 2019 · 6 comments · Fixed by #6090
Labels
bug bugs found in the application help wanted issues meant to be picked up, require help preferences issues related to preferences vscode issues related to VSCode compatibility

Comments

@a1994846931931
Copy link
Contributor

a1994846931931 commented Aug 26, 2019

Description

Sometimes, VSCode plugin registered configurations doesn't work in workspace (user configuration does work, though). No error or warning is shown in the setting widget and everything seem to work nicely expect that the workspace configuration is actually ignored. It seems that these configuration is wrongly filtered.

Reproduction Steps

This issue is kind of hard to reproduce. For one thing, it only happens in some VSCode plugins, for example, cpptool. For another, it doesn't always happen to them. So probably you need to try several times before it happens.

  1. Launch theia with VSCode's cpptool plugin (attention: due to issue Language server of VSCode's C/C++ plugin does not stop when theia exit #5899 , you need to clean /tmp/vscode-unpacked/cpptool* before restarting the background) or other VSCode plugins you want to test;
  2. Play around with the configurations that is registered by the plugin, and make sure they work in user configuration;
  3. Now refresh your browser and play around with the workspace configuration and see if they still take effect;
  4. You may need to repeat step 3 several times;

There is another thing that I found interesting (or, weird) - the issue does not happen if all editors are closed. That is, it happens only when one or more files are opened.

Updated Reproduction Steps

Try reproduce by
#6046 (comment)

OS and Theia version:

Theia: 0.9.0
OS: Ubuntu

Diagnostics:
None.

@akosyakov akosyakov added bug bugs found in the application preferences issues related to preferences vscode issues related to VSCode compatibility help wanted issues meant to be picked up, require help labels Aug 27, 2019
@a1994846931931
Copy link
Contributor Author

a1994846931931 commented Sep 2, 2019

It seems that it only happens for the first time the configuration is read. So if you make some changes to the workspace configuration and the plugin has a preference-changed listener, the re-read configuration would be valid.

PS. The preference system is just so complicated and very difficult to diagnose. And the fact that plugin-ext is impossible to debug (am I wrong?) even adds to the difficulty.

@akosyakov
Copy link
Member

And the fact that plugin-ext is impossible to debug (am I wrong?) even adds to the difficulty.

@a1994846931931 it is possible, see #3251 (comment) let me know if you need a help to get it running

@a1994846931931
Copy link
Contributor Author

And the fact that plugin-ext is impossible to debug (am I wrong?) even adds to the difficulty.

@a1994846931931 it is possible, see #3251 (comment) let me know if you need a help to get it running

@akosyakov Sorry, I mean debugging source code under packages/plugin-ext, not a VSCode extension.

@akosyakov
Copy link
Member

akosyakov commented Sep 2, 2019

@akosyakov Sorry, I mean debugging source code under packages/plugin-ext, not a VSCode extension.

Which file? These source code runs in 3 different processes:

@a1994846931931
Copy link
Contributor Author

a1994846931931 commented Sep 3, 2019

Diagnostic Update:

For workspace configurations, they are provided by .theia/settings.json and, will be validated and filtered by a schema. The schema will be updated when a plugin is loaded, through a series of events. However in some cases, the VSCode plugin requires reading of preferences as soon as it's activated, and the preferences are read before it's refreshed, some configurations will then be filtered by the old schema.

This explains why this issue happens only:

  1. For the very first time the workspace configuration is read;
  2. When package.json of the VSCode plugin is very complicated (this requires a long time to parse the metadata);
  3. For workspace configuration that is provided by .theia/settings.json (the validation process only works for settings.json under .theia folder). The logic behind it comes from the following lines:
    https://github.com/theia-ide/theia/blob/1e67e74498a369c4665b28afd64e91ec5f6e0133/packages/preferences/src/browser/abstract-resource-preference-provider.ts#L171-L174

Another interesting phenomenon is that, although user configuration is provided by ~/.theia/settings.json, who is also under a folder named .theia, its values are not validated because the uri of this provider is settings.json, and in line
https://github.com/theia-ide/theia/blob/1e67e74498a369c4665b28afd64e91ec5f6e0133/packages/preferences/src/browser/abstract-resource-preference-provider.ts#L173
this.configurations.getPath(this.getUri()) would actually return settings.json, rather than expected .theia and, thus, https://github.com/theia-ide/theia/blob/1e67e74498a369c4665b28afd64e91ec5f6e0133/packages/preferences/src/browser/abstract-resource-preference-provider.ts#L171
will return true directly.

Proposal

Now that the cause of the problem is clear, the question of, how we should solve it, remains. I've attempted to solve it by adding schema-changed events this.schemaProvider.onDidPreferenceSchemaChanged(() => this.readPreferences()); after https://github.com/theia-ide/theia/blob/1e67e74498a369c4665b28afd64e91ec5f6e0133/packages/preferences/src/browser/abstract-resource-preference-provider.ts#L54-L56
but it just doesn't work for it still loses the race between refreshing of preference and preference reading from VSCode plugins.

So my proposal would be that, just remove https://github.com/theia-ide/theia/blob/1e67e74498a369c4665b28afd64e91ec5f6e0133/packages/preferences/src/browser/abstract-resource-preference-provider.ts#L154-L157. After all, it only works for setting.json under workspace's .theia folder, which is strange because it behaves differently from configurations contributed by ${workspaceFolder}/.vscode/settings.json or ~/.theia/settings.json. So removing the process of validity should not cause a problem, or, will it? Have to admit that I'm not very confident of this modification but, nevertheless, a PR will be posted so you can check it out.

a1994846931931 added a commit to a1994846931931/theia that referenced this issue Sep 3, 2019
… extension and provided by .theia/settins.json is sometimes wrongly filtered out

Signed-off-by: Cai Xuye <[email protected]>
@a1994846931931
Copy link
Contributor Author

a1994846931931 commented Sep 3, 2019

Updated Reproduction Steps

  1. Download and deploy this simple test plugin on theia
    test-configuration-contribution-0.0.1.zip
  2. Add configuration "test.testConfiguration": "new configuration" to workspace configuration (you may need to create .theia/settings.json by your own;
  3. Refresh the browser and watch the notification that says the read configuration from the workspace. It's expected to be new configuration, but it should probably be "test.testConfiguration": "empty" instead. You can also run Show value of test.testConfiguration command after that and the value should be correct then.

svenefftinge pushed a commit that referenced this issue Sep 6, 2019
…d provided by .theia/settins.json is sometimes wrongly filtered out

Signed-off-by: Cai Xuye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application help wanted issues meant to be picked up, require help preferences issues related to preferences vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants