eclipse-cdt / cdt-lsp

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

ToggleSourceAndHeaderCommandHandler seems to use wrong URIs in TextDocumentIdentifier #255

Open travkin79 opened 7 months ago

travkin79 commented 7 months ago

While debugging for another feature I looked deeper into the ToggleSourceAndHeaderCommandHandler and found out that the URI looks different compared to other URIs used in LSP4E. I think, that might lead to various problems.

I didn't find yet any issue that a cdt lsp user would experience because of that. But I had a problem with another LS request when I created the document identifier the same way as in ToggleSourceAndHeaderCommandHandler. In my case, the LS responded that it did not find my document. After fixing the document identifier creation, the LS found my document. Thus, I think, there might be a similar problem with the ToggleSourceAndHeaderCommandHandler, too.

Maybe someone should check that.

Details

The ToggleSourceAndHeaderCommandHandler creates document URIs with the following code new TextDocumentIdentifier(fileUri.toString()) which leads to URIs like file:/workspaces/.../src/instrumentmodel/ConsistencyCheckErrorReporter.cpp. LSP4E uses the utility method LSPEclipseUtils.toTextDocumentIdentifier(IDocument) to create such URIs and these look a little different. For the above example we get the URI file:///workspaces/.../src/instrumentmodel/ConsistencyCheckErrorReporter.cpp. Please note that the first URI begins with file:/while the second begins with file:///.

ddscharfe commented 7 months ago

Hi @travkin79,

you are right, the first URI lacks the (empty) authority, the second one seems correct. I guess I didn't think about this when I wrote the initial implementation.