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

[#242] make API of org.eclipse.cdt.lsp.clangd package public #243

Closed
wants to merge 1 commit into from

Conversation

ghentschke
Copy link
Contributor

fixes #242

@iloveeclipse
Copy link

Any reason why this PR hangs for 3 weeks? Is there a chance to be merged soon?

@ghentschke
Copy link
Contributor Author

ghentschke commented Feb 21, 2024

Any reason why this PR hangs for 3 weeks? Is there a chance to be merged soon?

Yes. I would like to refactor the API of the cdt-lsp feature as described here.

@ruspl-afed
Copy link
Member

Any reason why this PR hangs for 3 weeks? Is there a chance to be merged soon?

If you have any specific requests for API, it is good time to formulate them @iloveeclipse

@iloveeclipse
Copy link

I was askimg because we needed something that was supposed to be aded as API with this PR, but I have no idea what exactly was needed. @travkin79 should know.

@ghentschke
Copy link
Contributor Author

ghentschke commented Feb 26, 2024

My Plan is to refactor the API of the org.eclipse.cdt.lsp bundle first. See #277. Then we should refactor the org.eclipse.cdt.lsp.clangd bundle API and lifting the bundle version to 2.0.0

@travkin79
Copy link
Contributor

If you have any specific requests for API, it is good time to formulate them

Our requirements come from #227. The PR #229 offers the opportunity to adapt the org.eclipse.cdt.lsp.clangd.ClangdConfigurationFileManager by overriding methods like enableSetCompilationDatabasePath(IProject). Doing so requires to implement a sub-class of ClangdConfigurationFileManager and registering it as a higher priority OSGi service for the service type org.eclipse.cdt.lsp.clangd.ClangdCProjectDescriptionListener (we use that type in a @Component annotation). In our custom ClangdConfigurationFileManager sub-class we get discouraged access warnings for these two types, since the API is not public, yet.

Another idea was being able to provide a custom ICLanguageServerProvider. We don't use that at the moment, but I tried to implement a custom ICLanguageServerProvider and needed access to the following types that are not part of the public API either: org.eclipse.cdt.lsp.clangd.ClangdConfiguration, org.eclipse.cdt.lsp.clangd.ClangdFallbackFlags. We either need these types in the public API or a class that we could subclass, for example, in order to override the method org.eclipse.cdt.lsp.internal.clangd.ClangdLanguageServerProvider#isEnabledFor(IProject). I would prefer the latter to avoid redundant implementation of getInitializationOptions(URI) and getCommands(URI).

@ghentschke
Copy link
Contributor Author

ghentschke commented Mar 5, 2024

If you have any specific requests for API, it is good time to formulate them

Our requirements come from #227. The PR #229 offers the opportunity to adapt the org.eclipse.cdt.lsp.clangd.ClangdConfigurationFileManager by overriding methods like enableSetCompilationDatabasePath(IProject). Doing so requires to implement a sub-class of ClangdConfigurationFileManager and registering it as a higher priority OSGi service for the service type org.eclipse.cdt.lsp.clangd.ClangdCProjectDescriptionListener (we use that type in a @Component annotation). In our custom ClangdConfigurationFileManager sub-class we get discouraged access warnings for these two types, since the API is not public, yet.

These files are with the PR #285 and #286 part of the public clangd API

Another idea was being able to provide a custom ICLanguageServerProvider. We don't use that at the moment, but I tried to implement a custom ICLanguageServerProvider and needed access to the following types that are not part of the public API either: org.eclipse.cdt.lsp.clangd.ClangdConfiguration, org.eclipse.cdt.lsp.clangd.ClangdFallbackFlags. We either need these types in the public API or a class that we could subclass, for example, in order to override the method org.eclipse.cdt.lsp.internal.clangd.ClangdLanguageServerProvider#isEnabledFor(IProject).

It should not be necessary for vendors to implement the ICLanguageServerProvider. To provide a custom enable the org.eclipse.cdt.lsp.editor.LanguageServerEnable interface has to be implemented as OSGi service with a service-ranking > 0.
E.g.:

@Component(property = {"service.ranking:Integer=1"})
public class ClangdLanguageServerEnable implements LanguageServerEnable {
        @Override
	public boolean isEnabledFor(IProject project) {
             // your custom enable
         }
}

@ruspl-afed
Copy link
Member

ruspl-afed commented May 29, 2024

@ghentschke is it still relevant?

@ghentschke
Copy link
Contributor Author

@ghentschke is it still relevant?

No, became outdated.

@ghentschke ghentschke closed this May 29, 2024
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.

Make org.eclipse.cdt.lsp.clangd API public
4 participants