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

Introduce universal configuration provider #835

Closed
wants to merge 1 commit into from

Conversation

zulus
Copy link
Contributor

@zulus zulus commented Oct 3, 2023

This pr implements:

  1. didChangeConfiguration auto registration
  2. Support for old style configuration watchers
  3. Basic API to implements settings provider on top of EclipsePreferences

@zulus
Copy link
Contributor Author

zulus commented Oct 3, 2023

See attached implementation for wildwebdeveloper. After this, for example vue language server (because support didChangeConfiguration auto-registration) without any other modification will be able to consume js/ts settings managed by eclipse

@zulus zulus marked this pull request as ready for review October 4, 2023 21:14
@zulus
Copy link
Contributor Author

zulus commented Oct 5, 2023

@angelozerr @mickaelistria any comments?

@mickaelistria
Copy link
Contributor

I will try to review it soon

@zulus
Copy link
Contributor Author

zulus commented Oct 15, 2023

knock knock ;)

@mickaelistria
Copy link
Contributor

I think the part adding "didChangeConfiguration auto registration" is good and due to properly support LSP.
However, I'm skeptical about adding a custom extension point and native support for preference-based configuration directly in LSP4E now, as there is only 1 LS that has expressed the need for it, and the mapping rules happens to be simple for this LS, but may not be in most cases (ie a configuration int from the LS could be turned into a string for the IDE preferences and other more complex mapping can take place, some of them than cannot be captured by LSP4E).
So can you please trim out that part the particular part about Eclipse Preferences and move it to client ( eclipse-wildwebdeveloper/wildwebdeveloper#1356 ) to see how things behave? I believe it would keep things simpler and more powerful if LSP4E only adds extensibility so consumers can push/fetch/react to configuration in their own code.

@zulus
Copy link
Contributor Author

zulus commented Oct 17, 2023

Sure, I had a plan to maybe in future implement default provider for "proxy.*" configs, but even they don't use IEclipsePreference ;)

@eclipsewebmaster eclipsewebmaster deleted the branch eclipse-lsp4e:master May 21, 2024 14:09
@mickaelistria
Copy link
Contributor

The initial target branch master was deleted and replaced by main, so this PR got closed automatically. If this is still relevant, please re-create this PR targetting the main branch.

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.

3 participants