eclipse-cdt / cdt-lsp

Eclipse CDT™ LSP Extensions for CDT
Eclipse Public License 2.0
26 stars 11 forks source link

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

Closed jonahgraham closed 4 months ago

jonahgraham commented 4 months ago

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:

https://github.com/eclipse/lsp4e/commit/a27b7a93ad74b3edc95c11d4eaf29342a04489d9

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

https://github.com/eclipse-cdt/cdt-lsp/commit/6d6209bfeeeed817c6c4a321f564e0f377552b3d

jonahgraham commented 4 months ago

Follow-up for https://github.com/eclipse-cdt/cdt-lsp/pull/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 commented 4 months ago

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.