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

Use environment-safe platform check in preferences #11888

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

thegecko
Copy link
Member

Signed-off-by: thegecko [email protected]

What it does

The PR: #11393

Introduced a dependency on the node os module in common plugin source. This had the effect that the preferences system doesn't load for web-based extensions.

This PR fixes the issue by using the environment-safe checks instead.

How to test

Load a web extension which accesses user configuration and check it is possible.

Review checklist

Reminder for reviewers

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me 👍

I'm wondering whether it makes sense to grab the backend OS type instead since we are dealing with URIs. I saw that we never set the backend OS type in the web-based plugin host, so it will always be populated with the frontend OS type. We might want to address this in a follow-up PR.

@thegecko thegecko merged commit 571e301 into eclipse-theia:master Nov 21, 2022
@thegecko thegecko deleted the fix-web-config branch November 21, 2022 12:06
@colin-grant-work
Copy link
Contributor

Apologies for introducing the bad dependency, and thanks for the fix!

@vince-fugnitto vince-fugnitto added this to the 1.32.0 milestone Nov 24, 2022
erezmus pushed a commit to ARMmbed/theia that referenced this pull request May 15, 2023
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Jun 12, 2023
CareyJWilliams pushed a commit to ARMmbed/theia that referenced this pull request Jun 22, 2023
CareyJWilliams pushed a commit to ARMmbed/theia that referenced this pull request Jun 23, 2023
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Aug 2, 2023
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Sep 4, 2023
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Sep 12, 2023
erezmus pushed a commit to ARMmbed/theia that referenced this pull request Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants