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

Account for non-API method signature change in LSP4E #334

Merged
merged 1 commit into from
May 30, 2024

Conversation

jonahgraham
Copy link
Member

The signature of the LanguageServerWrapper restart method changed between the last two snapshots. This means we needed to remove our catch a few commits ago. This also means we need to use a version of LSP4E >= the version where the non-API changed, hence why we care about patch version in the MANIFEST.MF.

We probably need to have LS4E expose needed methods as API so we don't have problems mixing and matching versions going forward.

See the corresponding LSP4E commit:

eclipse/lsp4e@a27b7a9

And the corresponding CDT-LSP commit that fixed the compilation issue:

6d6209b

The signature of the LanguageServerWrapper restart method
changed between the last two snapshots. This means we
needed to remove our catch a few commits ago. This also
means we need to use a version of LSP4E >= the version
where the non-API changed, hence why we care about patch
version in the MANIFEST.MF.

We probably need to have LS4E expose needed methods as API
so we don't have problems mixing and matching versions going
forward.

See the corresponding LSP4E commit:

eclipse/lsp4e@a27b7a9

And the corresponding CDT-LSP commit that fixed the compilation issue:

eclipse-cdt@6d6209b
@jonahgraham
Copy link
Member Author

Follow-up for #322

@ghentschke let me know what you think. This is slightly less important in SimRel, but quite important for people consuming the cdt-lsp repo directly to make sure they end up with a working pair of LSP4E and cdt-lsp.

@ghentschke
Copy link
Contributor

We probably need to have LS4E expose needed methods as API so we don't have problems mixing and matching versions going forward.

I fully agree. We have to discuss this with the LSP4E team to find a solution for this. We use several classes which are not public API. That's not very nice.

@ghentschke ghentschke merged commit 87c6142 into eclipse-cdt:master May 30, 2024
3 checks passed
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.

None yet

2 participants