Requests: Refactor implIntf and inferIntf parameters

[mattiasdrp]

Currently implIntf and inferIntf accept a DocumentUri while typedHoles accepts

{
  uri: DocumentUri
}

When looking at how vscode-ocaml-platform handles it:

• For implIntf and inferIntf:

  let source_uri = Uri.toString (TextDocument.uri document) ()
  

• For typedHoles

  let uri = TextDocument.uri doc
  

I don't see any reason to not have the three requests have the same type of parameter

[mattiasdrp]

Fixes #1330

[voodoos]

I don't see any reason to not have the three requests have the same type of parameter

That's quite right, but is that a reason sufficient-enough to introduce a breaking change in the implIntf and inferIntf custom request ? I personally don't think that it is worth-it.

[mattiasdrp]

Hi @voodoos ;-)

It's not yet widely used and it may introduce some technical debt once people start adding new custom requests picking code from implIntf or typedHoles. Having to already play with inconsistencies when there are only 3 different requests justifies a change before we have to deal with way more, in my opinion :-)

[nemethf]

There is a way to ease the transition from the current API to the new one. A new experimental server capability like, for example, experimental.ocamllsp.handleSwitchImplAndIntf (with 'And') could let the LSP clients know to use the new API. Clients relying on the old API search for experimental.ocamllsp.handleSwitchImplIntf (without 'And'), and therefore they could gracefully fall back to a more basic operation.

[mattiasdrp]

I like this idea @nemethf, thanks for it! I'll improve my PR tomorrow!

[voodoos]

Having to already play with inconsistencies when there are only 3 different requests

That cost is not high enough to justify breaking older clients or start a complex migration process. There is nothing wrong in the current queries behavior. They don't enforce the same style practices and that's a shame. I agree that we should decide on which style is the best to make sure we enforce it in the future. This can be done by documenting the current implementations.

[xvw]

I open a RFC https://github.com/ocaml/ocaml-lsp/issues/1503 to discuss about custom request migration. FMPOV, I think that we can probably change the behaviour of existing current request to deal with uri | {uri: DocumentUri} in order to be more lax and to not have to compose with client migration ATM.