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

Allow to define VS Code extensions preferences in Che plugin registry #17975

Closed
gattytto opened this issue Sep 25, 2020 · 20 comments
Closed

Allow to define VS Code extensions preferences in Che plugin registry #17975

gattytto opened this issue Sep 25, 2020 · 20 comments
Assignees
Labels
area/plugin-registry kind/enhancement A feature request - must adhere to the feature request template. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P2 Has a minor but important impact to the usage or development of the system. sprint/current
Milestone

Comments

@gattytto
Copy link

gattytto commented Sep 25, 2020

this feature will allow 2 things:

  1. set default static configurations for extensions that don't implement the use of ENV_VARS from within plugin's meta.yaml and avoid having to set them in the devfile scope.

  2. fix things that need to be set in a specific way when extension A needs to change the behavior of extension B, example:
    set java engine from lombok extension's meta.yaml without the 2 extensions having to be mashed up in the same plugin (producing a duplicate of references to vsix that need to be updated separately).

as a consequence this feature will prevent:

  1. extensions from downloading content from internet that can be previously populated in the sidecar

  2. having to "edit" extensions code to adapt them for sidecars.

example cases of this problem:
kotlin
lombok

@gattytto gattytto added the kind/enhancement A feature request - must adhere to the feature request template. label Sep 25, 2020
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Sep 25, 2020
@tomgeorge tomgeorge added area/editors area/languages Issues related to Language extensions or plugins integration. severity/P2 Has a minor but important impact to the usage or development of the system. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. area/editors labels Sep 28, 2020
@ericwill
Copy link
Contributor

I think this one is a good candidate for a bump in priority -- we are seeing this use needed for multiple extensions:

  • kotlin
  • haskell
  • plantuml

@gattytto
Copy link
Author

gattytto commented Sep 30, 2020

this may be related

@gattytto
Copy link
Author

I think this one is a good candidate for a bump in priority -- we are seeing this use needed for multiple extensions:

  • kotlin
  • haskell
  • plantuml

maybe add lombok to the list :P

@tsmaeder
Copy link
Contributor

And the "official" vale plugin: https://github.com/errata-ai/vale-vscode

@tsmaeder tsmaeder added area/plugin-broker area/plugin-registry and removed area/languages Issues related to Language extensions or plugins integration. labels Jan 13, 2021
@sunix
Copy link
Contributor

sunix commented Jan 13, 2021

There is a hack/workaround to do that: add the preference at the sidecar entrypoint level:
For instance:
https://github.com/eclipse/che-plugin-registry/blob/master/sidecars/asciidoc/etc/entrypoint.sh#L31-L35

@tsmaeder
Copy link
Contributor

@sunix that hack will not work for multi-root workspaces which should be coming real soon.

@tsmaeder
Copy link
Contributor

But we could do this: write some code in che-theia that reads preferences files from a well known location (/projects/.initprefs) and applies those preferences properly at workspace creation time.
Plugins then could write their desired preferences into a uniquely named file in that folder.

@sunix
Copy link
Contributor

sunix commented Jan 24, 2021

@tsmaeder agree that eventually that won't be valid. But we don't have multi-root yet. It works at the moment, that's why it is called a hack/workaround :)
I prefer to have something that works than plugins that fails to starts. This current issue is about doing it properly.

@tsmaeder
Copy link
Contributor

@sunix it's a workaround because the exact mechanism of storing workspace settings are an undocumented implementation detail of Theia. If we used that proper API, everything would just keep working.

@sunix
Copy link
Contributor

sunix commented Feb 12, 2021

@tsmaeder we have use the workspace settings in a workspace file like

{
   "folders": [
      {
         "path": "file:///tmp/theiadev_projects"
      },
      {
         "path": "file:///tmp/theiadev_projects/console-java-simple"
      }
   ],
"settings": {
    "java.semanticHighlighting.enabled": false
}

keeping it in /projects, telling theia to use the sameone and we'll all be fine

@ericwill
Copy link
Contributor

For devfile v2:

  • specify settings in che-theia-plugins.yaml
  • consume these settings on the che-theia side

@sunix
Copy link
Contributor

sunix commented Mar 19, 2021

@benoitf @ericwill If it's too hard to implement that for devfile 2.0 + multiroot, I think we could apply the workaround above and patch /projects/che.theia-workspace instead of /projects/.theia/settings.json
What do you think ?
It would give us flexibility not having to think about an implementation that would have to work with both devfile v1 and v2.
We would just work on implementing that properly for devfile 2

@benoitf
Copy link
Contributor

benoitf commented Mar 19, 2021

I still don't like the patch solution as proper solution.

AFAIK, patch is only possible today when we have sidecars (as you're updating the entrypoint.sh)

So why can't we:

  • Add the definition of every setting in the che-theia-plugins.yaml
  • generated devfile.yaml has these attributes set
    like
    attributes:
    che-theia-settings:
    - asciidoc.use_asciidoctorpdf: true
    for meta.yaml (che-server/devfile v1 we generate/convert this information into an ENV variable like CHE_THEIA_EXTRA_SETTING="{'asciidoc.use_asciidoctorpdf':true}" (if there is no sidecar then we don't do anything but it's like today as we only patch through entrypoint)

Then, when the sidecar is booting, if there ENV var CHE_THEIA_EXTRA_SETTING it could submit these settings

in devfile 2, we fetch all attributes with che-theia-settings and apply these properties

so the logic of 'how to provide settings' is not in the sidecar entrypoint, but on the consumer, che-theia)
and the definition is properly exposed in the che-theia-plugins.yaml

@sunix
Copy link
Contributor

sunix commented Mar 19, 2021

No, it's not about a proper solution but temporary solution for devfile v1 + multi root. it is a trivial change while we work on your proposal for devfile v2. I am -1 to have multiroot on by default if it breaks that.
I am ok with the proposal with the env variable for devfile v1, but it sounds like overkill for a temporary solution.

@ericwill
Copy link
Contributor

I'm not sure about the CHE_THEIA_EXTRA_SETTING either if it's just going to be temporary. For the sake of a temporary fix we can do the sidecar hack since it's the quickest. At the same time we can add + document these settings in the che-theia-plugins.yaml.

@benoitf
Copy link
Contributor

benoitf commented Mar 19, 2021

Well it'll avoid to patch all entrypoint.sh ( also the suggested hack won't work with devworkspace engine)
And I don't see why it's quickest as it does not cover all mode ( multi-root and devworkspace) while doing it properly does not take more time

@sunix
Copy link
Contributor

sunix commented Mar 20, 2021

@benoitf I have just checked and the hack is for only 2 plugins (ruby and asciidoc).
The idea is to have an temporary fix so we can turn on multiroot without breaking existing features.

I don't mind what we choose but if the 2 plugins don't have the settings required to work properly in multi-root, I am strongly against turning on multi-root by default. Users will be impacted.

@ericwill
Copy link
Contributor

The agreed way forward is:

  1. Add preferences fields to the che-theia-plugins.yaml entries.
  2. @benoitf will make a PR to consume them on the che-theia side
  3. Remove existing sidecar hacks (asciidoc and ruby)
  4. Investigate any devfiles which have preferences in them that can be made default.

@gattytto gattytto changed the title FR: Set extension settings from plugins meta.yaml FR: Set extension settings from che-theia-plugins.yaml Mar 26, 2021
@benoitf
Copy link
Contributor

benoitf commented Mar 30, 2021

4 steps have been done.
in che-server mode, there is still a limitation that preferences will be added only if there is a custom sidecar

@l0rd
Copy link
Contributor

l0rd commented Mar 30, 2021

We have fixed that for devworkspace based workspaces. We are not going to fix it for che-server mode when the extension is not running on a sidecar since che server engine will be deprecated. Closing this issue.

@l0rd l0rd closed this as completed Mar 30, 2021
@l0rd l0rd changed the title FR: Set extension settings from che-theia-plugins.yaml Allow to define VS Code extensions preferences in che-theia-plugins.yaml Mar 30, 2021
@l0rd l0rd added the new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes label Mar 30, 2021
@l0rd l0rd changed the title Allow to define VS Code extensions preferences in che-theia-plugins.yaml Allow to define VS Code extensions preferences in Che plugin registry Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugin-registry kind/enhancement A feature request - must adhere to the feature request template. new&noteworthy For new and/or noteworthy issues that deserve a blog post, new docs, or emphasis in release notes severity/P2 Has a minor but important impact to the usage or development of the system. sprint/current
Projects
None yet
Development

No branches or pull requests

9 participants