eclipse-lsp4j / lsp4j

A Java implementation of the language server protocol intended to be consumed by tools and language servers implemented in Java.
https://eclipse.org/lsp4j
Other
581 stars 141 forks source link

Provide FileOperationRegistrationOptions class #826

Closed angelozerr closed 1 month ago

angelozerr commented 3 months ago

The LSP specification defines the FileOperationRegistrationOptions which is helpful to support dynamic registration for file operations.

It would be nice if LSP4J could provide it.

Thanks!

pisv commented 3 months ago

Looks like LSP4J already provides it: https://github.com/eclipse-lsp4j/lsp4j/blob/cc20058d5b3d58596d49b71ca3c20308cc451844/org.eclipse.lsp4j/src/main/java/org/eclipse/lsp4j/Protocol.xtend#L8107-L8126

angelozerr commented 3 months ago

Thanks @pisv!

I wonder why FileOperationOptionsname is used instead of FileOperationRegistrationOptions which respects the spec?

pisv commented 3 months ago

I wonder why FileOperationOptions name is used instead of FileOperationRegistrationOptions which respects the spec?

Good question :-)

The following is my educated guess...

There are some inconsistencies regarding registration options in the spec. But usually there is a basic *Options interface, and there is a corresponding *RegistrationOptions interface the extends the basic *Options interface and also extends StaticRegistrationOptions and/or TextDocumentRegistrationOptions.

For example, there is SemanticTokensOptions, and there is SemanticTokensRegistrationOptions, which extends SemanticTokensOptions, TextDocumentRegistrationOptions, StaticRegistrationOptions.

The corresponding server capability semanticTokensProvider is typed as SemanticTokensOptions | SemanticTokensRegistrationOptions to allow for static registration of SemanticTokensRegistrationOptions in response to the initialize request.

SemanticTokensRegistrationOptions can also be used for dynamic registration of the textDocument/semanticTokens feature via the client/registerCapability request.

Clearly, FileOperationRegistrationOptions does not match this pattern. According to the spec, there are no dynamic registration options supported for WillCreateFiles and similar requests/notifications. There is no FileOperationOptions that FileOperationRegistrationOptions would extend, and FileOperationRegistrationOptions also does not extend StaticRegistrationOptions.

So, despite its name, FileOperationRegistrationOptions has nothing to do with static or dynamic registration options, as it now stands.

Also, while type names don't mean much in TypeScript with its structural typing, changing a type name in Java would be a breaking change.

Therefore, I guess it was decided to reserve the name FileOperationRegistrationOptions in LSP4J to be able to differentiate between FileOperationOptions and FileOperationRegistrationOptions according to the usual pattern for registration options, when/if such a need arises.

jonahgraham commented 1 month ago

Thank you @pisv for providing such a detailed analysis. I don't think there is anything left to do here so I am closing this issue. Please comment/reopen if I have missed something.